Skip to content

Commit

Permalink
Fix: security check that open is called basedd on previous lstat check
Browse files Browse the repository at this point in the history
Instead of using lstate before opening the file to later on check if it
was changed manually the file descriptor can also be open before and
just check if fstat is returning an error even after opening it.
  • Loading branch information
nichtsfrei authored and y0urself committed Sep 2, 2021
1 parent 7d182ac commit 9052172
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 40 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
[#811](https://github.com/greenbone/openvas/pull/811)

### Fixed
- Use fchmod to change file permission instead of on open to prevent race conditions [854](https://github.com/greenbone/openvas-scanner/pull/854)

### Removed
- Remove handling of source_iface related preferences. [#730](https://github.com/greenbone/openvas/pull/730)
Expand Down
52 changes: 12 additions & 40 deletions nasl/nasl_cmd_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ nasl_file_open (lex_ctxt *lexic)
{
tree_cell *retc;
char *fname, *mode;
struct stat lstat_info, fstat_info;
struct stat fstat_info;
int fd;
int imode = O_RDONLY;

Expand Down Expand Up @@ -458,48 +458,20 @@ nasl_file_open (lex_ctxt *lexic)
else if (strcmp (mode, "a+") == 0)
imode = O_RDWR | O_APPEND | O_CREAT;

if (lstat (fname, &lstat_info) == -1)
fd = open (fname, imode, 0600);
if (fd < 0)
{
if (errno != ENOENT)
{
nasl_perror (lexic, "file_open: %s: %s\n", fname, strerror (errno));
return NULL;
}
fd = open (fname, imode, 0600);
if (fd < 0)
{
nasl_perror (lexic, "file_open: %s: %s\n", fname, strerror (errno));
return NULL;
}
nasl_perror (lexic, "file_open: %s: possible symlink attack!?! %s\n",
fname, strerror (errno));
return NULL;
}
else

if (fstat (fd, &fstat_info) == -1)
{
fd = open (fname, imode, 0600);
if (fd < 0)
{
nasl_perror (lexic, "file_open: %s: possible symlink attack!?! %s\n",
fname, strerror (errno));
return NULL;
}
if (fstat (fd, &fstat_info) == -1)
{
close (fd);
nasl_perror (lexic, "fread: %s: possible symlink attack!?! %s\n",
fname, strerror (errno));
return NULL;
}
else
{
if (lstat_info.st_mode != fstat_info.st_mode
|| lstat_info.st_ino != fstat_info.st_ino
|| lstat_info.st_dev != fstat_info.st_dev)
{
close (fd);
nasl_perror (lexic, "fread: %s: possible symlink attack!?!\n",
fname);
return NULL;
}
}
close (fd);
nasl_perror (lexic, "fread: %s: possible symlink attack!?! %s\n", fname,
strerror (errno));
return NULL;
}

retc = alloc_typed_cell (CONST_INT);
Expand Down

0 comments on commit 9052172

Please sign in to comment.