diff options
author | Bruno Haible <bruno@clisp.org> | 2019-06-10 16:50:04 +0200 |
---|---|---|
committer | Bruno Haible <bruno@clisp.org> | 2019-06-10 17:24:58 +0200 |
commit | 765146c33361b46aa2c592e980b16069094c6000 (patch) | |
tree | b8cbb15b10cca39202340848c26b980ef838b068 | |
parent | fbb40ec10e333cff0b9845572065edd9e66eac79 (diff) | |
download | gnulib-765146c33361b46aa2c592e980b16069094c6000.tar.gz |
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.
-rw-r--r-- | ChangeLog | 10 | ||||
-rw-r--r-- | lib/spawn_faction_addopen.c | 47 | ||||
-rw-r--r-- | lib/spawn_faction_destroy.c | 29 | ||||
-rw-r--r-- | lib/spawn_int.h | 2 |
4 files changed, 69 insertions, 19 deletions
@@ -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; |