about summary refs log tree commit diff
path: root/posix/execvpe.c
diff options
context:
space:
mode:
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>2016-11-22 16:23:28 -0200
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>2016-12-07 09:48:26 -0200
commit8047e7cf7152a356510f51c18c5a198865470af2 (patch)
treedc3656c24e086b0d8d5af68355c68ed6896ca843 /posix/execvpe.c
parent657c084cd6f69d6cc880c2ae65129a0723d053c5 (diff)
downloadglibc-8047e7cf7152a356510f51c18c5a198865470af2.tar.gz
glibc-8047e7cf7152a356510f51c18c5a198865470af2.tar.xz
glibc-8047e7cf7152a356510f51c18c5a198865470af2.zip
Fix writes past the allocated array bounds in execvpe (BZ#20847)
Commit 6c9e1be87a37bf wrongly fixes BZ#20847 by lefting the else branch
on maybe_script_execute to still being able to invalid write on stack
allocated buffer.  It happens if execvp{e} is executed with an empty
arguments list ({ NULL }) and although manual states first argument
should be the script name itself, by convention, old and current
implementation allows it.

This patch fixes the issue by just account for arguments and not the
final 'NULL' (since the 'argv + 1' will indeed ignored the script name).
The empty argument list is handled in a special case with a minimum
allocated size.  The patch also adds extra tests for such case in
tst-vfork3.

Tested on x86_64.

	[BZ #20847]
	* posix/execvpe.c (maybe_script_execute): Remove write past allocated
	array bounds for else branch.
	(__execvpe): Style fixes.
	* posix/tst-vfork3.c (run_script): New function.
	(create_script): Likewise.
	(do_test): Use run_script internal function.
	(do_prepare): Use create_script internal function.
Diffstat (limited to 'posix/execvpe.c')
-rw-r--r--posix/execvpe.c19
1 files changed, 12 insertions, 7 deletions
diff --git a/posix/execvpe.c b/posix/execvpe.c
index 7cdb06a611..a2d01452d0 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -38,8 +38,8 @@
 static void
 maybe_script_execute (const char *file, char *const argv[], char *const envp[])
 {
-  ptrdiff_t argc = 0;
-  while (argv[argc++] != NULL)
+  ptrdiff_t argc;
+  for (argc = 0; argv[argc] != NULL; argc++)
     {
       if (argc == INT_MAX - 1)
 	{
@@ -48,13 +48,18 @@ maybe_script_execute (const char *file, char *const argv[], char *const envp[])
 	}
     }
 
-  /* 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];
+  /* Construct an argument list for the shell based on original arguments:
+     1. Empty list (argv = { NULL }, argc = 1 }: new argv will contain 3
+	arguments - default shell, script to execute, and ending NULL.
+     2. Non empty argument list (argc = { ..., NULL }, argc > 1}: new argv
+	will contain also the default shell and the script to execute.  It
+	will also skip the script name in arguments and only copy script
+	arguments.  */
+  char *new_argv[argc > 1 ? 2 + argc : 3];
   new_argv[0] = (char *) _PATH_BSHELL;
   new_argv[1] = (char *) file;
   if (argc > 1)
-    memcpy (new_argv + 2, argv + 1, (argc - 1) * sizeof(char *));
+    memcpy (new_argv + 2, argv + 1, argc * sizeof(char *));
   else
     new_argv[2] = NULL;
 
@@ -96,7 +101,7 @@ __execvpe (const char *file, char *const argv[], char *const envp[])
   size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
 
   /* NAME_MAX does not include the terminating null character.  */
-  if (((file_len-1) > NAME_MAX)
+  if ((file_len - 1 > NAME_MAX)
       || !__libc_alloca_cutoff (path_len + file_len + 1))
     {
       errno = ENAMETOOLONG;