This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2] Fix writes past the allocated array bounds in execvpe (BZ#20847)
On 21/11/2016 18:10, Andreas Schwab wrote:
> On Nov 21 2016, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>
>> diff --git a/posix/execvpe.c b/posix/execvpe.c
>> index d933f9c..96a12bf5 100644
>> --- a/posix/execvpe.c
>> +++ b/posix/execvpe.c
>> @@ -41,19 +41,20 @@ maybe_script_execute (const char *file, char *const argv[], char *const envp[])
>> ptrdiff_t argc = 0;
>> while (argv[argc++] != NULL)
>> {
>> - if (argc == INT_MAX - 1)
>> + if (argc == INT_MAX - 2)
>> {
>> errno = E2BIG;
>> return;
>> }
>> }
>>
>> - /* Construct an argument list for the shell. */
>> - char *new_argv[argc + 1];
>> + /* Construct an argument list for the shell. It will contain at minimum 3
>> + arguments (current shell, script, and an ending NULL. */
>> + char *new_argv[argc + 2];
>
> The array is now always one element too big, unless execvpe was called
> with argv[0] == NULL.
You are right, I keep forgetting 'argc' in this context also contains the
script name itself. The memcpy adjustment is indeed the only fix required
for this part:
diff --git a/posix/execvpe.c b/posix/execvpe.c
index d933f9c..7cdb06a 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -48,12 +48,13 @@ maybe_script_execute (const char *file, char *const argv[], char *const envp[])
}
}
- /* Construct an argument list for the shell. */
+ /* Construct an argument list for the shell. It will contain at minimum 3
+ arguments (current shell, script, and an ending NULL. */
char *new_argv[argc + 1];
new_argv[0] = (char *) _PATH_BSHELL;
new_argv[1] = (char *) file;
if (argc > 1)
- memcpy (new_argv + 2, argv + 1, argc * sizeof(char *));
+ memcpy (new_argv + 2, argv + 1, (argc - 1) * sizeof(char *));
else
new_argv[2] = NULL;
With this change are you ok to push this in?