[patch] Fix for BZ #18043 buffer-overflow (read past the end) in wordexp/parse_dollars/parse_param

[Paul Pluzhnikov]

On Thu, Mar 5, 2015 at 11:48 AM, Carlos O'Donell <carlos@redhat.com> wrote:

> OK to commit as long as you verified that test case fails before
> and passes afterwards on at least x86_64.

It doesn't. To make it fail I would have to mmap two pages, mprotect
the second, and place the string at the end of the first page... which
is quite a bit of code.

Must I do that? I guess if we hope to catch any regression here, I must...

Attached patch is verified to fail posix/wordexp-test with expected
SIGSEGV due to overflow before the fix, and pass after.


2015-03-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

        [BZ #18043]
        * posix/wordexp.c (parse_param): Fix buffer overflow.
        * posix/wordexp-test.c (do_bz18043): Add test case.



-- 
Paul Pluzhnikov
-------------- next part --------------
diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
index 8a312e0..c928438 100644
--- a/posix/wordexp-test.c
+++ b/posix/wordexp-test.c
@@ -17,6 +17,7 @@
 
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <sys/mman.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <pwd.h>
@@ -249,6 +250,31 @@ command_line_test (const char *words)
     printf ("we_wordv[%d] = \"%s\"\n", i, we.we_wordv[i]);
 }
 
+static int
+do_bz18043 (void)
+{
+  const int pagesize = getpagesize ();
+  char *start = mmap (0, 2 * pagesize, PROT_READ|PROT_WRITE,
+		      MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+
+  if (start == MAP_FAILED)
+    return 1;
+
+  if (mprotect (start + pagesize, pagesize, PROT_NONE))
+    return 2;
+
+  const char word[] = "${";
+  char *word_start = start + pagesize - sizeof (word);
+  memcpy (word_start, word, sizeof (word));
+
+  wordexp_t w;
+  wordexp (word_start, &w, 0);
+
+  munmap (start, 2 * pagesize);
+
+  return 0;
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -370,6 +396,9 @@ main (int argc, char *argv[])
 
   printf ("tests failed: %d\n", fail);
 
+  if (do_bz18043 ())
+    ++fail;
+
   return fail != 0;
 }
 
diff --git a/posix/wordexp.c b/posix/wordexp.c
index e3d8d6b..1c14401 100644
--- a/posix/wordexp.c
+++ b/posix/wordexp.c
@@ -1299,7 +1299,7 @@ parse_param (char **word, size_t *word_length, size_t *max_length,
 	}
       while (isdigit(words[++*offset]));
     }
-  else if (strchr ("*@$", words[*offset]) != NULL)
+  else if (words[*offset] != '\0' && strchr ("*@$", words[*offset]) != NULL)
     {
       /* Special parameter. */
       special = 1;