Attachment 951233 Details for Bug 1157689 – CVE-2014-7817-wordexp.diff

[patch] CVE-2014-7817-wordexp.diff

CVE-2014-7817-wordexp.diff (text/plain), 5.91 KB, created by Carlos O'Donell on 2014-10-28 03:22:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Carlos O'Donell

Created: 2014-10-28 03:22:28 UTC

Size: 5.91 KB

patch

obsolete

>WARNING !!! WARNING !!! WARNING !!! WARNING !!! WARNING !!! WARNING !!!
>EMBARGOED !!! EMBARGOED !!! EMARGOED !!! EMBARGOED !!! EMBARGOED !!!
>SECURITY !!! SECURITY !!! SECURITY !!! SECURITY !!! SECURITY !!!
>
>CVE-2014-7817:
>
>The function wordexp() fails to properly handle the WRDE_NOCMD
>flag when processing arithmetic inputs in the form of "$((... ``))"
>where "..." can be anything valid. The backticks in the arithmetic
>epxression are evaluated by in a shell even if WRDE_NOCMD forbade
>command substitution. This allows an attacker to attempt to pass
>dangerous commands via constructs of the above form, and bypass
>the WRDE_NOCMD flag. This patch fixes this by checking for WRDE_NOCMD
>in parse_arith(). The patch also hardens parse_backticks() and 
>parse_comm() to check for WRDE_NOCMD flag and return an error instead
>of ever running a shell.
>
>We expand the testsuite and add 3 new regression tests of roughtly
>the same form but with a couple of nested levels. 
>
>On top of the 3 new tests we add fork validation to the WRDE_NOCMD
>testing. If any forks are detected during the execution of a wordexp()
>call with WRDE_NOCMD, the test is marked as failed. This is slightly
>heuristic since vfork might be used, but it provides a higher level
>of assurance that no shells were executed as part of command substitution
>with WRDE_NOCMD in effect. In addition it doesn't require libpthread or
>libdl, instead we use the public implementation namespace function
>__register_atfork (already part of the public ABI for libpthread).
>
>Tested on x86_64 with no regressions.
>
>2014-10-27  Carlos O'Donell  <carlos@redhat.com>
>
>	* wordexp-test.c (__dso_handle): Add prototype.
>	(__register_atfork): Likewise.
>	(__app_register_atfork): New function.
>	(registered_forks): New global.
>	(register_fork): New function.
>	(test_case): Add 3 new tests for WRDE_CMDSUB.
>	(main): Call __app_register_atfork.
>	(testit): If WRDE_NOCMD set registered_forks to zero, run test, and
>	if fork count is non-zero fail the test.
>	* posix/wordexp.c (parse_arith): Return WRDE_NOCMD if WRDE_NOCMD flag
>	is set and parsing '`'. 
>	(parse_comm): Return WRDE_NOCMD if WRDE_NOCMD flag is set.
>	(parse_backtick): Return WRDE_NOCMD if WRDE_NOCMD flag is set and
>	parsing '`'.
>
>diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
>index 4957006..5ce2a1b 100644
>--- a/posix/wordexp-test.c
>+++ b/posix/wordexp-test.c
>@@ -27,6 +27,25 @@
> 
> #define IFS " \n\t"
> 
>+extern void *__dso_handle __attribute__ ((__weak__, __visibility__ ("hidden")));
>+extern int __register_atfork (void (*) (void), void (*) (void), void (*) (void), void *);
>+
>+static int __app_register_atfork (void (*prepare) (void), void (*parent) (void), void (*child) (void))
>+{
>+  return __register_atfork (prepare, parent, child,
>+			    &__dso_handle == NULL ? NULL : __dso_handle);
>+}
>+
>+/* Number of forks seen.  */
>+static int registered_forks;
>+
>+/* For each fork increment the fork count.  */
>+static void
>+register_fork (void)
>+{
>+  registered_forks++;
>+}
>+
> struct test_case_struct
> {
>   int retval;
>@@ -206,6 +225,12 @@ struct test_case_struct
>     { WRDE_SYNTAX, NULL, "$((2+))", 0, 0, { NULL, }, IFS },
>     { WRDE_SYNTAX, NULL, "`", 0, 0, { NULL, }, IFS },
>     { WRDE_SYNTAX, NULL, "$((010+4+))", 0, 0, { NULL }, IFS },
>+    /* Test for CVE-2014-7817. We test 3 combinations of command
>+       substitution inside an arithmetic expression to make sure that
>+       no commands are executed and error is returned.  */
>+    { WRDE_CMDSUB, NULL, "$((`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS },
>+    { WRDE_CMDSUB, NULL, "$((1+`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS },
>+    { WRDE_CMDSUB, NULL, "$((1+$((`echo 1`))))", WRDE_NOCMD, 0, { NULL, }, IFS },
> 
>     { -1, NULL, NULL, 0, 0, { NULL, }, IFS },
>   };
>@@ -258,6 +283,15 @@ main (int argc, char *argv[])
> 	  return -1;
>     }
> 
>+  /* If we are not allowed to do command substitution, we install
>+     fork handlers to verify that no forks happened.  No forks should
>+     happen at all if command substitution is disabled.  */
>+  if (__app_register_atfork (register_fork, NULL, NULL) != 0)
>+    {
>+      printf ("Failed to register fork handler.\n");
>+      return -1;
>+    }
>+
>   for (test = 0; test_case[test].retval != -1; test++)
>     if (testit (&test_case[test]))
>       ++fail;
>@@ -367,6 +401,9 @@ testit (struct test_case_struct *tc)
> 
>   printf ("Test %d (%s): ", ++tests, tc->words);
> 
>+  if (tc->flags & WRDE_NOCMD)
>+    registered_forks = 0;
>+
>   if (tc->flags & WRDE_APPEND)
>     {
>       /* initial wordexp() call, to be appended to */
>@@ -378,6 +415,13 @@ testit (struct test_case_struct *tc)
>     }
>   retval = wordexp (tc->words, &we, tc->flags);
> 
>+  if ((tc->flags & WRDE_NOCMD)
>+      && (registered_forks > 0))
>+    {
>+      printf ("FAILED fork called for WRDE_NOCMD\n");
>+      return 1;
>+    }
>+
>   if (tc->flags & WRDE_DOOFFS)
>       start_offs = sav_we.we_offs;
> 
>diff --git a/posix/wordexp.c b/posix/wordexp.c
>index b6b65dd..d6a158f 100644
>--- a/posix/wordexp.c
>+++ b/posix/wordexp.c
>@@ -693,6 +693,12 @@ parse_arith (char **word, size_t *word_length, size_t *max_length,
> 	  break;
> 
> 	case '`':
>+	   if (flags & WRDE_NOCMD)
>+	     {
>+	       free (expr);
>+	       return WRDE_NOCMD;
>+	     }
>+
> 	  (*offset)++;
> 	  error = parse_backtick (&expr, &expr_length, &expr_maxlen,
> 				  words, offset, flags, NULL, NULL, NULL);
>@@ -1144,6 +1150,10 @@ parse_comm (char **word, size_t *word_length, size_t *max_length,
>   size_t comm_maxlen;
>   char *comm = w_newword (&comm_length, &comm_maxlen);
> 
>+  /* Do nothing if command substitution should not succeed.  */
>+  if (flags & WRDE_NOCMD)
>+    return WRDE_CMDSUB;
>+
>   for (; words[*offset]; ++(*offset))
>     {
>       switch (words[*offset])
>@@ -2121,6 +2131,9 @@ parse_backtick (char **word, size_t *word_length, size_t *max_length,
>       switch (words[*offset])
> 	{
> 	case '`':
>+	  if (flags & WRDE_NOCMD)
>+	    return WRDE_NOCMD;
>+
> 	  /* Go -- give the script to the shell */
> 	  error = exec_comm (comm, word, word_length, max_length, flags,
> 			     pwordexp, ifs, ifs_white);

Actions: View | Diff

Attachments on bug 1157689: 951233