Fix issues found by ASAN and Coverity

* tests/test_driver.pl: Preserve the LSAN_OPTIONS variable.
* tests/scripts/targets/ONESHELL: Don't set a local variable.
* tests/scripts/functions/let: Test empty let variable.
* src/posixos.c (osync_parse_mutex): Free existing osync_tmpfile.
* src/misc.c (get_tmpfd): Set umask() before invoking mkstemp().
* src/ar.c (ar_parse_name): Check invalid name (shouldn't happen).
* src/function.c (define_new_function): Free previous function entry
when replacing it with a new one.
* src/job.c (child_execute_job): Initialize pid for safety.
(construct_command_argv_internal): In oneshell mode ensure that the
returned argv has the right format (0th element is a pointer to the
entire buffer).

6 files changed, 40 insertions, 14 deletions

diff --git a/src/ar.c b/src/ar.c
index 8b02f047..7b607dcd 100644
--- a/src/ar.c
+++ b/src/ar.c
@@ -36,7 +36,7 @@ ar_name (const char *name)
   const char *p = strchr (name, '(');
   const char *end;
 
-  if (p == 0 || p == name)
+  if (p == NULL || p == name)
     return 0;
 
   end = p + strlen (p) - 1;
@@ -61,6 +61,9 @@ ar_parse_name (const char *name, char **arname_p, char **memname_p)
 
   *arname_p = xstrdup (name);
   p = strchr (*arname_p, '(');
+  /* This is never called unless ar_name() is true so p cannot be NULL.  */
+  if (!p)
+    OS (fatal, NILF, "Internal: ar_parse_name: bad name '%s'", *arname_p);
   *(p++) = '\0';
   p[strlen (p) - 1] = '\0';
   *memname_p = p;
diff --git a/src/dir.c b/src/dir.c
index 3577a12d..b47e94fe 100644
--- a/src/dir.c
+++ b/src/dir.c
@@ -601,7 +601,7 @@ find_directory (const char *name)
   /* Point the name-hashed entry for DIR at its contents data.  */
   dir->contents = dc;
 
-  /* If the contents have changed, we need to reseet.  */
+  /* If the contents have changed, we need to reseed.  */
   if (dc->counter != command_count)
     {
       if (dc->counter)
diff --git a/src/function.c b/src/function.c
index db5e1b51..f0ef3434 100644
--- a/src/function.c
+++ b/src/function.c
@@ -2811,7 +2811,8 @@ define_new_function (const floc *flocp, const char *name,
   ent->adds_command = 1;
   ent->fptr.alloc_func_ptr = func;
 
-  hash_insert (&function_table, ent);
+  ent = hash_insert (&function_table, ent);
+  free (ent);
 }
 
 void
diff --git a/src/job.c b/src/job.c
index ed951026..b78f279c 100644
--- a/src/job.c
+++ b/src/job.c
@@ -2271,7 +2271,7 @@ child_execute_job (struct childbase *child, int good_stdin, char **argv)
   const int fdin = good_stdin ? FD_STDIN : get_bad_stdin ();
   int fdout = FD_STDOUT;
   int fderr = FD_STDERR;
-  pid_t pid;
+  pid_t pid = -1;
   int r;
 #if defined(USE_POSIX_SPAWN)
   char *cmd;
@@ -3364,30 +3364,44 @@ construct_command_argv_internal (char *line, char **restp, const char *shell,
 #endif /* WINDOWS32 */
         /* Create an argv list for the shell command line.  */
         {
-          int n = 0;
+          int n = 1;
+          char *nextp;
 
           new_argv = xmalloc ((4 + sflags_len/2) * sizeof (char *));
-          new_argv[n++] = xstrdup (shell);
+
+          nextp = new_argv[0] = xmalloc (shell_len + sflags_len + line_len + 3);
+          nextp = mempcpy (nextp, shell, shell_len + 1);
 
           /* Chop up the shellflags (if any) and assign them.  */
           if (! shellflags)
-            new_argv[n++] = xstrdup ("");
+            {
+              new_argv[n++] = nextp;
+              *(nextp++) = '\0';
+            }
           else
             {
               /* Parse shellflags using construct_command_argv_internal to
                  handle quotes. */
-              char **argv, **a;
-              char *f;
-              f = alloca (sflags_len + 1); /* +1 for null terminator.  */
+              char **argv;
+              char *f = alloca (sflags_len + 1);
               memcpy (f, shellflags, sflags_len + 1);
               argv = construct_command_argv_internal (f, 0, 0, 0, 0, flags, 0);
-              for (a = argv; a && *a; ++a)
-                new_argv[n++] = *a;
-              free (argv);
+              if (argv)
+                {
+                  char **a;
+                  for (a = argv; *a; ++a)
+                    {
+                      new_argv[n++] = nextp;
+                      nextp = stpcpy (nextp, *a) + 1;
+                    }
+                  free (argv[0]);
+                  free (argv);
+                }
             }
 
           /* Set the command to invoke.  */
-          new_argv[n++] = line;
+          new_argv[n++] = nextp;
+          memcpy(nextp, line, line_len + 1);
           new_argv[n++] = NULL;
         }
         return new_argv;
diff --git a/src/misc.c b/src/misc.c
index bf6fe7ca..8264fe9f 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -668,6 +668,7 @@ get_tmpfd (char **name)
 {
   int fd = -1;
   char *tmpnm;
+  mode_t mask;
 
   /* If there's an os-specific way to get an anoymous temp file use it.  */
   if (!name)
@@ -677,6 +678,10 @@ get_tmpfd (char **name)
         return fd;
     }
 
+  /* Preserve the current umask, and set a restrictive one for temp files.
+     Only really needed for mkstemp() but won't hurt for the open method.  */
+  mask = umask (0077);
+
 #if defined(HAVE_MKSTEMP)
   tmpnm = get_tmptemplate ();
 
@@ -704,6 +709,8 @@ get_tmpfd (char **name)
       free (tmpnm);
     }
 
+  umask (mask);
+
   return fd;
 }
 
diff --git a/src/posixos.c b/src/posixos.c
index 63201b40..44aeb346 100644
--- a/src/posixos.c
+++ b/src/posixos.c
@@ -673,6 +673,7 @@ osync_parse_mutex (const char *mutex)
       return 0;
     }
 
+  free (osync_tmpfile);
   osync_tmpfile = xstrdup (mutex + CSTRLEN (MUTEX_PREFIX));
 
   EINTRLOOP (osync_handle, open (osync_tmpfile, O_WRONLY));