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 |
|