From 90521724f052c9430ddc30d9dd3e535b2e21577a Mon Sep 17 00:00:00 2001
From: Philipp Eder <philipp.eder@greenbone.net>
Date: Thu, 2 Sep 2021 10:48:14 +0200
Subject: [PATCH] Fix: security check that open is called basedd on previous
 lstat check

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.
---
 CHANGELOG.md         |  1 +
 nasl/nasl_cmd_exec.c | 52 ++++++++++----------------------------------
 2 files changed, 13 insertions(+), 40 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f72ec25e0..e1ecf417b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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)
diff --git a/nasl/nasl_cmd_exec.c b/nasl/nasl_cmd_exec.c
index 3da83772b..de80ccf7f 100644
--- a/nasl/nasl_cmd_exec.c
+++ b/nasl/nasl_cmd_exec.c
@@ -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;
 
@@ -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);