| 1 |
From a39208bd7fb76c1b01c127b4c61f9bfd915bfe7c Mon Sep 17 00:00:00 2001 |
| 2 |
From: Carlos O'Donell <carlos@redhat.com> |
| 3 |
Date: Wed, 19 Nov 2014 11:44:12 -0500 |
| 4 |
Subject: [PATCH] CVE-2014-7817: wordexp fails to honour WRDE_NOCMD. |
| 5 |
|
| 6 |
The function wordexp() fails to properly handle the WRDE_NOCMD |
| 7 |
flag when processing arithmetic inputs in the form of "$((... ``))" |
| 8 |
where "..." can be anything valid. The backticks in the arithmetic |
| 9 |
epxression are evaluated by in a shell even if WRDE_NOCMD forbade |
| 10 |
command substitution. This allows an attacker to attempt to pass |
| 11 |
dangerous commands via constructs of the above form, and bypass |
| 12 |
the WRDE_NOCMD flag. This patch fixes this by checking for WRDE_NOCMD |
| 13 |
in exec_comm(), the only place that can execute a shell. All other |
| 14 |
checks for WRDE_NOCMD are superfluous and removed. |
| 15 |
|
| 16 |
We expand the testsuite and add 3 new regression tests of roughly |
| 17 |
the same form but with a couple of nested levels. |
| 18 |
|
| 19 |
On top of the 3 new tests we add fork validation to the WRDE_NOCMD |
| 20 |
testing. If any forks are detected during the execution of a wordexp() |
| 21 |
call with WRDE_NOCMD, the test is marked as failed. This is slightly |
| 22 |
heuristic since vfork might be used in the future, but it provides a |
| 23 |
higher level of assurance that no shells were executed as part of |
| 24 |
command substitution with WRDE_NOCMD in effect. In addition it doesn't |
| 25 |
require libpthread or libdl, instead we use the public implementation |
| 26 |
namespace function __register_atfork (already part of the public ABI |
| 27 |
for libpthread). |
| 28 |
|
| 29 |
Tested on x86_64 with no regressions. |
| 30 |
--- |
| 31 |
ChangeLog | 22 ++++++++++++++++++++++ |
| 32 |
NEWS | 8 +++++++- |
| 33 |
posix/wordexp-test.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ |
| 34 |
posix/wordexp.c | 16 ++++------------ |
| 35 |
4 files changed, 77 insertions(+), 13 deletions(-) |
| 36 |
|
| 37 |
#diff --git a/ChangeLog b/ChangeLog |
| 38 |
#index 09e308c..2fa59cf 100644 |
| 39 |
#--- a/ChangeLog |
| 40 |
#+++ b/ChangeLog |
| 41 |
#@@ -1,3 +1,25 @@ |
| 42 |
#+2014-11-19 Carlos O'Donell <carlos@redhat.com> |
| 43 |
#+ Florian Weimer <fweimer@redhat.com> |
| 44 |
#+ Joseph Myers <joseph@codesourcery.com> |
| 45 |
#+ Adam Conrad <adconrad@0c3.net> |
| 46 |
#+ Andreas Schwab <schwab@suse.de> |
| 47 |
#+ Brooks <bmoses@google.com> |
| 48 |
#+ |
| 49 |
#+ [BZ #17625] |
| 50 |
#+ * wordexp-test.c (__dso_handle): Add prototype. |
| 51 |
#+ (__register_atfork): Likewise. |
| 52 |
#+ (__app_register_atfork): New function. |
| 53 |
#+ (registered_forks): New global. |
| 54 |
#+ (register_fork): New function. |
| 55 |
#+ (test_case): Add 3 new tests for WRDE_CMDSUB. |
| 56 |
#+ (main): Call __app_register_atfork. |
| 57 |
#+ (testit): If WRDE_NOCMD set registered_forks to zero, run test, and if |
| 58 |
#+ fork count is non-zero fail the test. |
| 59 |
#+ * posix/wordexp.c (exec_comm): Return WRDE_CMDSUB if WRDE_NOCMD flag |
| 60 |
#+ is set. |
| 61 |
#+ (parse_dollars): Remove check for WRDE_NOCMD. |
| 62 |
#+ (parse_dquote): Likewise. |
| 63 |
#+ |
| 64 |
# 2014-11-19 Siddhesh Poyarekar <siddhesh@redhat.com> |
| 65 |
# |
| 66 |
# * Makeconfig (built-modules): List non-library modules to be |
| 67 |
#diff --git a/NEWS b/NEWS |
| 68 |
#index b152488..4b7eeb4 100644 |
| 69 |
#--- a/NEWS |
| 70 |
#+++ b/NEWS |
| 71 |
#@@ -12,7 +12,13 @@ Version 2.21 |
| 72 |
# 6652, 12926, 14132, 14138, 14171, 15215, 15884, 17266, 17344, 17363, |
| 73 |
# 17370, 17371, 17411, 17460, 17475, 17485, 17501, 17506, 17508, 17522, |
| 74 |
# 17555, 17570, 17571, 17572, 17573, 17574, 17582, 17583, 17584, 17585, |
| 75 |
#- 17589, 17594, 17616. |
| 76 |
#+ 17589, 17594, 17616, 17625. |
| 77 |
#+ |
| 78 |
#+* CVE-2104-7817 The wordexp function could ignore the WRDE_NOCMD flag |
| 79 |
#+ under certain input conditions resulting in the execution of a shell for |
| 80 |
#+ command substitution when the applicaiton did not request it. The |
| 81 |
#+ implementation now checks WRDE_NOCMD immediately before executing the |
| 82 |
#+ shell and returns the error WRDE_CMDSUB as expected. |
| 83 |
# |
| 84 |
# * The minimum GCC version that can be used to build this version of the GNU |
| 85 |
# C Library is GCC 4.6. Older GCC versions, and non-GNU compilers, can |
| 86 |
diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c |
| 87 |
index 4957006..bdd65e4 100644 |
| 88 |
--- a/posix/wordexp-test.c |
| 89 |
+++ b/posix/wordexp-test.c |
| 90 |
@@ -27,6 +27,25 @@ |
| 91 |
|
| 92 |
#define IFS " \n\t" |
| 93 |
|
| 94 |
+extern void *__dso_handle __attribute__ ((__weak__, __visibility__ ("hidden"))); |
| 95 |
+extern int __register_atfork (void (*) (void), void (*) (void), void (*) (void), void *); |
| 96 |
+ |
| 97 |
+static int __app_register_atfork (void (*prepare) (void), void (*parent) (void), void (*child) (void)) |
| 98 |
+{ |
| 99 |
+ return __register_atfork (prepare, parent, child, |
| 100 |
+ &__dso_handle == NULL ? NULL : __dso_handle); |
| 101 |
+} |
| 102 |
+ |
| 103 |
+/* Number of forks seen. */ |
| 104 |
+static int registered_forks; |
| 105 |
+ |
| 106 |
+/* For each fork increment the fork count. */ |
| 107 |
+static void |
| 108 |
+register_fork (void) |
| 109 |
+{ |
| 110 |
+ registered_forks++; |
| 111 |
+} |
| 112 |
+ |
| 113 |
struct test_case_struct |
| 114 |
{ |
| 115 |
int retval; |
| 116 |
@@ -206,6 +225,12 @@ struct test_case_struct |
| 117 |
{ WRDE_SYNTAX, NULL, "$((2+))", 0, 0, { NULL, }, IFS }, |
| 118 |
{ WRDE_SYNTAX, NULL, "`", 0, 0, { NULL, }, IFS }, |
| 119 |
{ WRDE_SYNTAX, NULL, "$((010+4+))", 0, 0, { NULL }, IFS }, |
| 120 |
+ /* Test for CVE-2014-7817. We test 3 combinations of command |
| 121 |
+ substitution inside an arithmetic expression to make sure that |
| 122 |
+ no commands are executed and error is returned. */ |
| 123 |
+ { WRDE_CMDSUB, NULL, "$((`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS }, |
| 124 |
+ { WRDE_CMDSUB, NULL, "$((1+`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS }, |
| 125 |
+ { WRDE_CMDSUB, NULL, "$((1+$((`echo 1`))))", WRDE_NOCMD, 0, { NULL, }, IFS }, |
| 126 |
|
| 127 |
{ -1, NULL, NULL, 0, 0, { NULL, }, IFS }, |
| 128 |
}; |
| 129 |
@@ -258,6 +283,15 @@ main (int argc, char *argv[]) |
| 130 |
return -1; |
| 131 |
} |
| 132 |
|
| 133 |
+ /* If we are not allowed to do command substitution, we install |
| 134 |
+ fork handlers to verify that no forks happened. No forks should |
| 135 |
+ happen at all if command substitution is disabled. */ |
| 136 |
+ if (__app_register_atfork (register_fork, NULL, NULL) != 0) |
| 137 |
+ { |
| 138 |
+ printf ("Failed to register fork handler.\n"); |
| 139 |
+ return -1; |
| 140 |
+ } |
| 141 |
+ |
| 142 |
for (test = 0; test_case[test].retval != -1; test++) |
| 143 |
if (testit (&test_case[test])) |
| 144 |
++fail; |
| 145 |
@@ -367,6 +401,9 @@ testit (struct test_case_struct *tc) |
| 146 |
|
| 147 |
printf ("Test %d (%s): ", ++tests, tc->words); |
| 148 |
|
| 149 |
+ if (tc->flags & WRDE_NOCMD) |
| 150 |
+ registered_forks = 0; |
| 151 |
+ |
| 152 |
if (tc->flags & WRDE_APPEND) |
| 153 |
{ |
| 154 |
/* initial wordexp() call, to be appended to */ |
| 155 |
@@ -378,6 +415,13 @@ testit (struct test_case_struct *tc) |
| 156 |
} |
| 157 |
retval = wordexp (tc->words, &we, tc->flags); |
| 158 |
|
| 159 |
+ if ((tc->flags & WRDE_NOCMD) |
| 160 |
+ && (registered_forks > 0)) |
| 161 |
+ { |
| 162 |
+ printf ("FAILED fork called for WRDE_NOCMD\n"); |
| 163 |
+ return 1; |
| 164 |
+ } |
| 165 |
+ |
| 166 |
if (tc->flags & WRDE_DOOFFS) |
| 167 |
start_offs = sav_we.we_offs; |
| 168 |
|
| 169 |
diff --git a/posix/wordexp.c b/posix/wordexp.c |
| 170 |
index b6b65dd..26f3a26 100644 |
| 171 |
--- a/posix/wordexp.c |
| 172 |
+++ b/posix/wordexp.c |
| 173 |
@@ -893,6 +893,10 @@ exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length, |
| 174 |
pid_t pid; |
| 175 |
int noexec = 0; |
| 176 |
|
| 177 |
+ /* Do nothing if command substitution should not succeed. */ |
| 178 |
+ if (flags & WRDE_NOCMD) |
| 179 |
+ return WRDE_CMDSUB; |
| 180 |
+ |
| 181 |
/* Don't fork() unless necessary */ |
| 182 |
if (!comm || !*comm) |
| 183 |
return 0; |
| 184 |
@@ -2082,9 +2086,6 @@ parse_dollars (char **word, size_t *word_length, size_t *max_length, |
| 185 |
} |
| 186 |
} |
| 187 |
|
| 188 |
- if (flags & WRDE_NOCMD) |
| 189 |
- return WRDE_CMDSUB; |
| 190 |
- |
| 191 |
(*offset) += 2; |
| 192 |
return parse_comm (word, word_length, max_length, words, offset, flags, |
| 193 |
quoted? NULL : pwordexp, ifs, ifs_white); |
| 194 |
@@ -2196,9 +2197,6 @@ parse_dquote (char **word, size_t *word_length, size_t *max_length, |
| 195 |
break; |
| 196 |
|
| 197 |
case '`': |
| 198 |
- if (flags & WRDE_NOCMD) |
| 199 |
- return WRDE_CMDSUB; |
| 200 |
- |
| 201 |
++(*offset); |
| 202 |
error = parse_backtick (word, word_length, max_length, words, |
| 203 |
offset, flags, NULL, NULL, NULL); |
| 204 |
@@ -2357,12 +2355,6 @@ wordexp (const char *words, wordexp_t *pwordexp, int flags) |
| 205 |
break; |
| 206 |
|
| 207 |
case '`': |
| 208 |
- if (flags & WRDE_NOCMD) |
| 209 |
- { |
| 210 |
- error = WRDE_CMDSUB; |
| 211 |
- goto do_error; |
| 212 |
- } |
| 213 |
- |
| 214 |
++words_offset; |
| 215 |
error = parse_backtick (&word, &word_length, &max_length, words, |
| 216 |
&words_offset, flags, pwordexp, ifs, |
| 217 |
-- |
| 218 |
1.7.1 |
| 219 |
|
Parent Directory
| 
Revision Log