posix_spawn_file_actions_addopen: Fix possible use-after-free bug.

Reported at <https://sourceware.org/bugzilla/show_bug.cgi?id=17048>.

* lib/spawn_int.h (struct __spawn_action): Remove 'const' from path.
* lib/spawn_faction_addopen.c (posix_spawn_file_actions_addopen): Make
a copy of the path argument.
* lib/spawn_faction_destroy.c (posix_spawn_file_actions_destroy): Free
it.

4 files changed, 69 insertions, 19 deletions

diff --git a/ChangeLog b/ChangeLog
index 7bbf5fafc0..16f720d790 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2019-06-10  Bruno Haible  <bruno@clisp.org>
 
+	posix_spawn_file_actions_addopen: Fix possible use-after-free bug.
+	Reported at <https://sourceware.org/bugzilla/show_bug.cgi?id=17048>.
+	* lib/spawn_int.h (struct __spawn_action): Remove 'const' from path.
+	* lib/spawn_faction_addopen.c (posix_spawn_file_actions_addopen): Make
+	a copy of the path argument.
+	* lib/spawn_faction_destroy.c (posix_spawn_file_actions_destroy): Free
+	it.
+
+2019-06-10  Bruno Haible  <bruno@clisp.org>
+
 	posix_spawn_file_actions_addfchdir: Add tests.
 	* tests/test-posix_spawn_file_actions_addfchdir.c: New file.
 	* tests/test-posix_spawn5.c: New file.
diff --git a/lib/spawn_faction_addopen.c b/lib/spawn_faction_addopen.c
index 6b4422fd12..ab4f727f33 100644
--- a/lib/spawn_faction_addopen.c
+++ b/lib/spawn_faction_addopen.c
@@ -20,6 +20,8 @@
 #include <spawn.h>
 
 #include <errno.h>
+#include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
 
 #if !_LIBC
@@ -47,27 +49,38 @@ posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions,
 #if !REPLACE_POSIX_SPAWN
   return posix_spawn_file_actions_addopen (file_actions, fd, path, oflag, mode);
 #else
-  /* Allocate more memory if needed.  */
-  if (file_actions->_used == file_actions->_allocated
-      && __posix_spawn_file_actions_realloc (file_actions) != 0)
-    /* This can only mean we ran out of memory.  */
-    return ENOMEM;
-
   {
-    struct __spawn_action *rec;
+    /* Copy PATH, because the caller may free it before calling posix_spawn()
+       or posix_spawnp().  */
+    char *path_copy = strdup (path);
+    if (path_copy == NULL)
+      return ENOMEM;
+
+    /* Allocate more memory if needed.  */
+    if (file_actions->_used == file_actions->_allocated
+        && __posix_spawn_file_actions_realloc (file_actions) != 0)
+      {
+        /* This can only mean we ran out of memory.  */
+        free (path_copy);
+        return ENOMEM;
+      }
+
+    {
+      struct __spawn_action *rec;
 
-    /* Add the new value.  */
-    rec = &file_actions->_actions[file_actions->_used];
-    rec->tag = spawn_do_open;
-    rec->action.open_action.fd = fd;
-    rec->action.open_action.path = path;
-    rec->action.open_action.oflag = oflag;
-    rec->action.open_action.mode = mode;
+      /* Add the new value.  */
+      rec = &file_actions->_actions[file_actions->_used];
+      rec->tag = spawn_do_open;
+      rec->action.open_action.fd = fd;
+      rec->action.open_action.path = path_copy;
+      rec->action.open_action.oflag = oflag;
+      rec->action.open_action.mode = mode;
 
-    /* Account for the new entry.  */
-    ++file_actions->_used;
+      /* Account for the new entry.  */
+      ++file_actions->_used;
 
-    return 0;
+      return 0;
+    }
   }
 #endif
 }
diff --git a/lib/spawn_faction_destroy.c b/lib/spawn_faction_destroy.c
index 6d9ec800f2..0640da483f 100644
--- a/lib/spawn_faction_destroy.c
+++ b/lib/spawn_faction_destroy.c
@@ -21,11 +21,38 @@
 
 #include <stdlib.h>
 
+#if REPLACE_POSIX_SPAWN
+# include "spawn_int.h"
+#endif
+
 /* Initialize data structure for file attribute for 'spawn' call.  */
 int
 posix_spawn_file_actions_destroy (posix_spawn_file_actions_t *file_actions)
+#undef posix_spawn_file_actions_destroy
 {
-  /* Free the memory allocated.  */
+#if !REPLACE_POSIX_SPAWN
+  return posix_spawn_file_actions_destroy (file_actions);
+#else
+  int i;
+
+  /* Free the paths in the open actions.  */
+  for (i = 0; i < file_actions->_used; ++i)
+    {
+      struct __spawn_action *sa = &file_actions->_actions[i];
+      switch (sa->tag)
+        {
+        case spawn_do_open:
+          free (sa->action.open_action.path);
+          break;
+        default:
+          /* No cleanup required.  */
+          break;
+        }
+    }
+
+  /* Free the array of actions.  */
   free (file_actions->_actions);
+
   return 0;
+#endif
 }
diff --git a/lib/spawn_int.h b/lib/spawn_int.h
index 584c1bbcc3..08c477a9cb 100644
--- a/lib/spawn_int.h
+++ b/lib/spawn_int.h
@@ -42,7 +42,7 @@ struct __spawn_action
     struct
     {
       int fd;
-      const char *path;
+      char *path;
       int oflag;
       mode_t mode;
     } open_action;