Fix issues found by CodeQL

CHANGELOG.md

@@ -25,6 +25,8 @@ 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)
+- Several minor potential security risks in different files, spotted by Code QL [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)

nasl/md4.c

@@ -147,7 +147,11 @@ mdfour64_ntlmssp (uint32 *M)
   B &= 0xFFFFFFFF;
   C &= 0xFFFFFFFF;
   D &= 0xFFFFFFFF;
-  memset (X, '\0', sizeof (X));
+
+  for (size_t i = 0; i < sizeof (X); i++)
+    {
+      *(volatile char *) (X + i) = '\0';
+    }
 }
 
 static void

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);

nasl/nasl_isotime.c

@@ -93,12 +93,12 @@ epoch2isotime (my_isotime_t timebuf, time_t atime)
     *timebuf = 0;
   else
     {
-      struct tm *tp;
+      struct tm tp;
 
-      tp = gmtime (&atime);
+      gmtime_r (&atime, &tp);
       if (snprintf (timebuf, ISOTIME_SIZE, "%04d%02d%02dT%02d%02d%02d",
-                    1900 + tp->tm_year, tp->tm_mon + 1, tp->tm_mday,
-                    tp->tm_hour, tp->tm_min, tp->tm_sec)
+                    1900 + tp.tm_year, tp.tm_mon + 1, tp.tm_mday, tp.tm_hour,
+                    tp.tm_min, tp.tm_sec)
           < 0)
         {
           *timebuf = '\0';

nasl/nasl_misc_funcs.c

@@ -41,6 +41,7 @@
 #include <glib.h>
 #include <gvm/util/compressutils.h> /* for gvm_uncompress */
 #include <gvm/util/kb.h>            /* for KB_TYPE_STR */
+#include <stdbool.h>                /* for boolean */
 #include <stdlib.h>                 /* for lrand48 */
 #include <string.h>                 /* for bzero */
 #include <sys/time.h>               /* for gettimeofday */
@@ -672,23 +673,35 @@ tree_cell *
 nasl_localtime (lex_ctxt *lexic)
 {
   tree_cell *retc;
-  struct tm *ptm;
+  struct tm ptm;
   time_t tictac;
   int utc;
   nasl_array *a;
   anon_nasl_var v;
+  bool success;
 
   tictac = get_int_var_by_num (lexic, 0, 0);
   if (tictac == 0)
     tictac = time (NULL);
   utc = get_int_var_by_name (lexic, "utc", 0);
 
+  success = true;
   if (utc)
-    ptm = gmtime (&tictac);
+    {
+      if (gmtime_r (&tictac, &ptm) == NULL)
+        {
+          success = false;
+        }
+    }
   else
-    ptm = localtime (&tictac);
+    {
+      if (localtime_r (&tictac, &ptm) == NULL)
+        {
+          success = false;
+        }
+    }
 
-  if (ptm == NULL)
+  if (!success)
     {
       nasl_perror (lexic, "localtime(%d,utc=%d): %s\n", tictac, utc,
                    strerror (errno));
@@ -700,23 +713,23 @@ nasl_localtime (lex_ctxt *lexic)
   memset (&v, 0, sizeof (v));
   v.var_type = VAR2_INT;
 
-  v.v.v_int = ptm->tm_sec;
+  v.v.v_int = ptm.tm_sec;
   add_var_to_array (a, "sec", &v); /* seconds */
-  v.v.v_int = ptm->tm_min;
+  v.v.v_int = ptm.tm_min;
   add_var_to_array (a, "min", &v); /* minutes */
-  v.v.v_int = ptm->tm_hour;
+  v.v.v_int = ptm.tm_hour;
   add_var_to_array (a, "hour", &v); /* hours */
-  v.v.v_int = ptm->tm_mday;
+  v.v.v_int = ptm.tm_mday;
   add_var_to_array (a, "mday", &v); /* day of the month */
-  v.v.v_int = ptm->tm_mon + 1;
+  v.v.v_int = ptm.tm_mon + 1;
   add_var_to_array (a, "mon", &v); /* month */
-  v.v.v_int = ptm->tm_year + 1900;
+  v.v.v_int = ptm.tm_year + 1900;
   add_var_to_array (a, "year", &v); /* year */
-  v.v.v_int = ptm->tm_wday;
+  v.v.v_int = ptm.tm_wday;
   add_var_to_array (a, "wday", &v); /* day of the week */
-  v.v.v_int = ptm->tm_yday + 1;
+  v.v.v_int = ptm.tm_yday + 1;
   add_var_to_array (a, "yday", &v); /* day in the year */
-  v.v.v_int = ptm->tm_isdst;
+  v.v.v_int = ptm.tm_isdst;
   add_var_to_array (a, "isdst", &v); /* daylight saving time */
 
   return retc;

nasl/nasl_var.c

@@ -689,7 +689,7 @@ get_variable_by_name (lex_ctxt *ctxt, const char *name)
      switch (t2)
        {
        case VAR2_INT:
-         nasl_trace (NULL, "NASL> %s <- %d\n", get_var_name (v1), v1->v.v_int);
+         nasl_trace (NULL, "NASL> %s <- %lu\n", get_var_name (v1), v1->v.v_int);
          break;
        case VAR2_STRING:
        case VAR2_DATA:
@@ -876,7 +876,8 @@ get_variable_by_name (lex_ctxt *ctxt, const char *name)
        ret->type = CONST_INT;
        ret->x.i_val = v->v.v_int;
        if (nasl_trace_enabled ())
-         nasl_trace (lexic, "NASL> %s -> %d\n", get_var_name (v), ret->x.i_val);
+         nasl_trace (lexic, "NASL> %s -> %lu\n", get_var_name (v),
+                     ret->x.i_val);
        return ret;
 
      case VAR2_STRING:

src/hosts.c

@@ -81,7 +81,10 @@ host_set_time (kb_t kb, char *ip, char *type)
   int len;
 
   t = time (NULL);
-  timestr = g_strdup (ctime (&t));
+  char ts[26];
+  char *ts_ptr = ts;
+  ctime_r (&t, ts_ptr);
+  timestr = g_strdup (ts_ptr);
   len = strlen (timestr);
   if (timestr[len - 1] == '\n')
     timestr[len - 1] = '\0';