This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix segfault in maybe_script_execute.


On 09/06/2018 10:50 AM, Andreas Schwab wrote:
On Sep 06 2018, Stefan Liebler <stli@linux.ibm.com> wrote:

On 09/05/2018 03:44 PM, Andreas Schwab wrote:
On Sep 05 2018, Stefan Liebler <stli@linux.ibm.com> wrote:

If argc > 1, the arguments inclusive the NULL termination of new_argv
is copied from argv with memcpy.

But it has the same bug, doesn't it?

Andreas.

Yes. It also has this bug.

This makes the description rather confusing, implying there is a
difference between argc == 1 and argc > 1, which isn't.

Andreas.

Okay. I've adjusted the description.

Thanks.
Stefan
commit 6359e1c59f856a1115fdbcfa233052ac62e32c83
Author: Stefan Liebler <stli@linux.ibm.com>
Date:   Thu Sep 6 11:03:38 2018 +0200

    Fix segfault in maybe_script_execute.
    
    If glibc is built with gcc 8 and -march=z900,
    the testcase posix/tst-spawn4-compat crashes with a segfault.
    
    In function maybe_script_execute, the new_argv array is dynamically
    initialized on stack with (argc + 1) elements.
    The function wants to add _PATH_BSHELL as the first argument
    and writes out of bounds of new_argv.
    
    In case of argc == 1, it writes three instead of two elements:
    new_argv[0] = (char *) _PATH_BSHELL;
    new_argv[1] = (char *) args->file;
    new_argv[2] = NULL;
    
    The latter write access writes to the same location where the
    pointer args is stored on stack.  Then it segfaults while accessing args:
    args->exec (new_argv[0], new_argv, args->envp);
    
    In case of argc > 1, new_argv[0] and new_argv[1] are set in the same
    way as in the case above and the arguments inclusive the NULL termination
    of new_argv is copied from argv with memcpy.
    Note: The last copied element (NULL termination) is written out of
    bounds of new_argv.
    
    The implementation assumes that argc == 0 will never happen!
    
    ChangeLog:
    
            * sysdeps/unix/sysv/linux/spawni.c (maybe_script_execute):
            Increment size of new_argv by one.

diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index cf0213ece5..85239cedbf 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -101,7 +101,7 @@ maybe_script_execute (struct posix_spawn_args *args)
       ptrdiff_t argc = args->argc;
 
       /* Construct an argument list for the shell.  */
-      char *new_argv[argc + 1];
+      char *new_argv[argc + 2];
       new_argv[0] = (char *) _PATH_BSHELL;
       new_argv[1] = (char *) args->file;
       if (argc > 1)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]