Commit | Line | Data |
---|---|---|
b032efff AM |
1 | From cf84d126bc1f04746eb7c8e8b3468f7e70add3ec Mon Sep 17 00:00:00 2001 |
2 | From: Jeremy Harris <jgh146exb@wizmail.org> | |
3 | Date: Fri, 5 Jul 2019 15:38:15 +0100 | |
4 | Subject: [PATCH] Avoid re-expansion in ${sort } CVE-2019-13917 | |
5 | OVE-20190718-0006 | |
6 | ||
7 | (cherry picked from commit 5c887f836e4d8e3f79da1c15565b56b40d9bd0dd) | |
8 | --- | |
9 | doc/ChangeLog | 6 ++ | |
10 | doc/doc-txt/cve-2019-13917 | 46 ++++++++ | |
11 | src/expand.c | 214 +++++++++++++++++++++++++------------ | |
12 | 3 files changed, 199 insertions(+), 67 deletions(-) | |
13 | create mode 100644 doc/doc-txt/cve-2019-13917 | |
14 | ||
15 | --- a/src/expand.c | |
16 | +++ b/src/expand.c | |
01e60269 | 17 | @@ -2147,6 +2147,55 @@ return ret; |
b032efff AM |
18 | |
19 | ||
20 | ||
21 | +/************************************************/ | |
22 | +/* Return offset in ops table, or -1 if not found. | |
23 | +Repoint to just after the operator in the string. | |
24 | + | |
25 | +Argument: | |
26 | + ss string representation of operator | |
27 | + opname split-out operator name | |
28 | +*/ | |
29 | + | |
30 | +static int | |
31 | +identify_operator(const uschar ** ss, uschar ** opname) | |
32 | +{ | |
33 | +const uschar * s = *ss; | |
34 | +uschar name[256]; | |
35 | + | |
36 | +/* Numeric comparisons are symbolic */ | |
37 | + | |
38 | +if (*s == '=' || *s == '>' || *s == '<') | |
39 | + { | |
40 | + int p = 0; | |
41 | + name[p++] = *s++; | |
42 | + if (*s == '=') | |
43 | + { | |
44 | + name[p++] = '='; | |
45 | + s++; | |
46 | + } | |
47 | + name[p] = 0; | |
48 | + } | |
49 | + | |
50 | +/* All other conditions are named */ | |
51 | + | |
52 | +else | |
53 | + s = read_name(name, sizeof(name), s, US"_"); | |
54 | +*ss = s; | |
55 | + | |
56 | +/* If we haven't read a name, it means some non-alpha character is first. */ | |
57 | + | |
58 | +if (!name[0]) | |
59 | + { | |
60 | + expand_string_message = string_sprintf("condition name expected, " | |
61 | + "but found \"%.16s\"", s); | |
62 | + return -1; | |
63 | + } | |
64 | +if (opname) | |
65 | + *opname = string_copy(name); | |
66 | + | |
67 | +return chop_match(name, cond_table, nelem(cond_table)); | |
68 | +} | |
69 | + | |
70 | ||
71 | /************************************************* | |
72 | * Read and evaluate a condition * | |
01e60269 | 73 | @@ -2177,6 +2226,7 @@ BOOL sub2_honour_dollar = TRUE; |
b032efff AM |
74 | int i, rc, cond_type, roffset; |
75 | int_eximarith_t num[2]; | |
76 | struct stat statbuf; | |
77 | +uschar * opname; | |
78 | uschar name[256]; | |
79 | const uschar *sub[10]; | |
80 | ||
01e60269 | 81 | @@ -2189,37 +2239,7 @@ for (;;) |
b032efff AM |
82 | if (*s == '!') { testfor = !testfor; s++; } else break; |
83 | } | |
84 | ||
85 | -/* Numeric comparisons are symbolic */ | |
86 | - | |
87 | -if (*s == '=' || *s == '>' || *s == '<') | |
88 | - { | |
89 | - int p = 0; | |
90 | - name[p++] = *s++; | |
91 | - if (*s == '=') | |
92 | - { | |
93 | - name[p++] = '='; | |
94 | - s++; | |
95 | - } | |
96 | - name[p] = 0; | |
97 | - } | |
98 | - | |
99 | -/* All other conditions are named */ | |
100 | - | |
101 | -else s = read_name(name, 256, s, US"_"); | |
102 | - | |
103 | -/* If we haven't read a name, it means some non-alpha character is first. */ | |
104 | - | |
105 | -if (name[0] == 0) | |
106 | - { | |
107 | - expand_string_message = string_sprintf("condition name expected, " | |
108 | - "but found \"%.16s\"", s); | |
109 | - return NULL; | |
110 | - } | |
111 | - | |
112 | -/* Find which condition we are dealing with, and switch on it */ | |
113 | - | |
114 | -cond_type = chop_match(name, cond_table, nelem(cond_table)); | |
115 | -switch(cond_type) | |
116 | +switch(cond_type = identify_operator(&s, &opname)) | |
117 | { | |
118 | /* def: tests for a non-empty variable, or for the existence of a header. If | |
119 | yield == NULL we are in a skipping state, and don't care about the answer. */ | |
01e60269 | 120 | @@ -2538,7 +2558,7 @@ switch(cond_type) |
b032efff AM |
121 | { |
122 | if (i == 0) goto COND_FAILED_CURLY_START; | |
123 | expand_string_message = string_sprintf("missing 2nd string in {} " | |
124 | - "after \"%s\"", name); | |
125 | + "after \"%s\"", opname); | |
126 | return NULL; | |
127 | } | |
01e60269 AM |
128 | if (!(sub[i] = expand_string_internal(s+1, TRUE, &s, yield == NULL, |
129 | @@ -2553,7 +2573,7 @@ switch(cond_type) | |
b032efff AM |
130 | conditions that compare numbers do not start with a letter. This just saves |
131 | checking for them individually. */ | |
132 | ||
133 | - if (!isalpha(name[0]) && yield != NULL) | |
134 | + if (!isalpha(opname[0]) && yield != NULL) | |
135 | if (sub[i][0] == 0) | |
136 | { | |
137 | num[i] = 0; | |
01e60269 | 138 | @@ -2867,7 +2887,7 @@ switch(cond_type) |
b032efff AM |
139 | uschar *save_iterate_item = iterate_item; |
140 | int (*compare)(const uschar *, const uschar *); | |
141 | ||
01e60269 AM |
142 | - DEBUG(D_expand) debug_printf_indent("condition: %s item: %s\n", name, sub[0]); |
143 | + DEBUG(D_expand) debug_printf_indent("condition: %s item: %s\n", opname, sub[0]); | |
b032efff AM |
144 | |
145 | tempcond = FALSE; | |
146 | compare = cond_type == ECOND_INLISTI | |
01e60269 | 147 | @@ -2909,14 +2929,14 @@ switch(cond_type) |
b032efff AM |
148 | if (*s != '{') /* }-for-text-editors */ |
149 | { | |
150 | expand_string_message = string_sprintf("each subcondition " | |
151 | - "inside an \"%s{...}\" condition must be in its own {}", name); | |
152 | + "inside an \"%s{...}\" condition must be in its own {}", opname); | |
153 | return NULL; | |
154 | } | |
155 | ||
156 | if (!(s = eval_condition(s+1, resetok, subcondptr))) | |
157 | { | |
158 | expand_string_message = string_sprintf("%s inside \"%s{...}\" condition", | |
159 | - expand_string_message, name); | |
160 | + expand_string_message, opname); | |
161 | return NULL; | |
162 | } | |
163 | while (isspace(*s)) s++; | |
01e60269 | 164 | @@ -2926,7 +2946,7 @@ switch(cond_type) |
b032efff AM |
165 | { |
166 | /* {-for-text-editors */ | |
167 | expand_string_message = string_sprintf("missing } at end of condition " | |
168 | - "inside \"%s\" group", name); | |
169 | + "inside \"%s\" group", opname); | |
170 | return NULL; | |
171 | } | |
172 | ||
01e60269 | 173 | @@ -2958,7 +2978,7 @@ switch(cond_type) |
b032efff AM |
174 | int sep = 0; |
175 | uschar *save_iterate_item = iterate_item; | |
176 | ||
177 | - DEBUG(D_expand) debug_printf_indent("condition: %s\n", name); | |
178 | + DEBUG(D_expand) debug_printf_indent("condition: %s\n", opname); | |
179 | ||
180 | while (isspace(*s)) s++; | |
181 | if (*s++ != '{') goto COND_FAILED_CURLY_START; /* }-for-text-editors */ | |
01e60269 | 182 | @@ -2979,7 +2999,7 @@ switch(cond_type) |
b032efff AM |
183 | if (!(s = eval_condition(sub[1], resetok, NULL))) |
184 | { | |
185 | expand_string_message = string_sprintf("%s inside \"%s\" condition", | |
186 | - expand_string_message, name); | |
187 | + expand_string_message, opname); | |
188 | return NULL; | |
189 | } | |
190 | while (isspace(*s)) s++; | |
01e60269 | 191 | @@ -2989,7 +3009,7 @@ switch(cond_type) |
b032efff AM |
192 | { |
193 | /* {-for-text-editors */ | |
194 | expand_string_message = string_sprintf("missing } at end of condition " | |
195 | - "inside \"%s\"", name); | |
196 | + "inside \"%s\"", opname); | |
197 | return NULL; | |
198 | } | |
199 | ||
01e60269 | 200 | @@ -3001,11 +3021,11 @@ switch(cond_type) |
b032efff AM |
201 | if (!eval_condition(sub[1], resetok, &tempcond)) |
202 | { | |
203 | expand_string_message = string_sprintf("%s inside \"%s\" condition", | |
204 | - expand_string_message, name); | |
205 | + expand_string_message, opname); | |
206 | iterate_item = save_iterate_item; | |
207 | return NULL; | |
208 | } | |
209 | - DEBUG(D_expand) debug_printf_indent("%s: condition evaluated to %s\n", name, | |
210 | + DEBUG(D_expand) debug_printf_indent("%s: condition evaluated to %s\n", opname, | |
211 | tempcond? "true":"false"); | |
212 | ||
213 | if (yield != NULL) *yield = (tempcond == testfor); | |
01e60269 | 214 | @@ -3098,19 +3118,20 @@ switch(cond_type) |
b032efff AM |
215 | /* Unknown condition */ |
216 | ||
217 | default: | |
218 | - expand_string_message = string_sprintf("unknown condition \"%s\"", name); | |
219 | - return NULL; | |
220 | + if (!expand_string_message || !*expand_string_message) | |
221 | + expand_string_message = string_sprintf("unknown condition \"%s\"", opname); | |
222 | + return NULL; | |
223 | } /* End switch on condition type */ | |
224 | ||
225 | /* Missing braces at start and end of data */ | |
226 | ||
227 | COND_FAILED_CURLY_START: | |
228 | -expand_string_message = string_sprintf("missing { after \"%s\"", name); | |
229 | +expand_string_message = string_sprintf("missing { after \"%s\"", opname); | |
230 | return NULL; | |
231 | ||
232 | COND_FAILED_CURLY_END: | |
233 | expand_string_message = string_sprintf("missing } at end of \"%s\" condition", | |
234 | - name); | |
235 | + opname); | |
236 | return NULL; | |
237 | ||
238 | /* A condition requires code that is not compiled */ | |
01e60269 | 239 | @@ -3120,7 +3141,7 @@ return NULL; |
b032efff AM |
240 | !defined(SUPPORT_CRYPTEQ) || !defined(CYRUS_SASLAUTHD_SOCKET) |
241 | COND_FAILED_NOT_COMPILED: | |
242 | expand_string_message = string_sprintf("support for \"%s\" not compiled", | |
243 | - name); | |
244 | + opname); | |
245 | return NULL; | |
246 | #endif | |
247 | } | |
01e60269 AM |
248 | @@ -3849,6 +3870,56 @@ return x; |
249 | ||
b032efff AM |
250 | |
251 | ||
252 | +/************************************************/ | |
253 | +/* Comparison operation for sort expansion. We need to avoid | |
254 | +re-expanding the fields being compared, so need a custom routine. | |
255 | + | |
256 | +Arguments: | |
257 | + cond_type Comparison operator code | |
258 | + leftarg, rightarg Arguments for comparison | |
259 | + | |
260 | +Return true iff (leftarg compare rightarg) | |
261 | +*/ | |
262 | + | |
263 | +static BOOL | |
264 | +sortsbefore(int cond_type, BOOL alpha_cond, | |
265 | + const uschar * leftarg, const uschar * rightarg) | |
266 | +{ | |
267 | +int_eximarith_t l_num, r_num; | |
268 | + | |
269 | +if (!alpha_cond) | |
270 | + { | |
271 | + l_num = expanded_string_integer(leftarg, FALSE); | |
272 | + if (expand_string_message) return FALSE; | |
273 | + r_num = expanded_string_integer(rightarg, FALSE); | |
274 | + if (expand_string_message) return FALSE; | |
275 | + | |
276 | + switch (cond_type) | |
277 | + { | |
278 | + case ECOND_NUM_G: return l_num > r_num; | |
279 | + case ECOND_NUM_GE: return l_num >= r_num; | |
280 | + case ECOND_NUM_L: return l_num < r_num; | |
281 | + case ECOND_NUM_LE: return l_num <= r_num; | |
282 | + default: break; | |
283 | + } | |
284 | + } | |
285 | +else | |
286 | + switch (cond_type) | |
287 | + { | |
288 | + case ECOND_STR_LT: return Ustrcmp (leftarg, rightarg) < 0; | |
289 | + case ECOND_STR_LTI: return strcmpic(leftarg, rightarg) < 0; | |
290 | + case ECOND_STR_LE: return Ustrcmp (leftarg, rightarg) <= 0; | |
291 | + case ECOND_STR_LEI: return strcmpic(leftarg, rightarg) <= 0; | |
292 | + case ECOND_STR_GT: return Ustrcmp (leftarg, rightarg) > 0; | |
293 | + case ECOND_STR_GTI: return strcmpic(leftarg, rightarg) > 0; | |
294 | + case ECOND_STR_GE: return Ustrcmp (leftarg, rightarg) >= 0; | |
295 | + case ECOND_STR_GEI: return strcmpic(leftarg, rightarg) >= 0; | |
296 | + default: break; | |
297 | + } | |
298 | +return FALSE; /* should not happen */ | |
299 | +} | |
300 | + | |
301 | + | |
01e60269 AM |
302 | /* Return pointer to dewrapped string, with enclosing specified chars removed. |
303 | The given string is modified on return. Leading whitespace is skipped while | |
304 | looking for the opening wrap character, then the rest is scanned for the trailing | |
305 | @@ -3905,7 +3976,7 @@ The element may itself be an object or a | |
306 | Return NULL when the list is empty. | |
307 | */ | |
b032efff | 308 | |
01e60269 AM |
309 | -uschar * |
310 | +static uschar * | |
311 | json_nextinlist(const uschar ** list) | |
312 | { | |
313 | unsigned array_depth = 0, object_depth = 0; | |
314 | @@ -6243,9 +6314,10 @@ while (*s != 0) | |
b032efff AM |
315 | |
316 | case EITEM_SORT: | |
317 | { | |
318 | + int cond_type; | |
319 | int sep = 0; | |
320 | const uschar *srclist, *cmp, *xtract; | |
321 | - uschar *srcitem; | |
322 | + uschar * opname, * srcitem; | |
323 | const uschar *dstlist = NULL, *dstkeylist = NULL; | |
324 | uschar * tmp; | |
325 | uschar *save_iterate_item = iterate_item; | |
01e60269 | 326 | @@ -6280,6 +6352,25 @@ while (*s != 0) |
b032efff AM |
327 | goto EXPAND_FAILED_CURLY; |
328 | } | |
329 | ||
330 | + if ((cond_type = identify_operator(&cmp, &opname)) == -1) | |
331 | + { | |
332 | + if (!expand_string_message) | |
333 | + expand_string_message = string_sprintf("unknown condition \"%s\"", s); | |
334 | + goto EXPAND_FAILED; | |
335 | + } | |
336 | + switch(cond_type) | |
337 | + { | |
338 | + case ECOND_NUM_L: case ECOND_NUM_LE: | |
339 | + case ECOND_NUM_G: case ECOND_NUM_GE: | |
340 | + case ECOND_STR_GE: case ECOND_STR_GEI: case ECOND_STR_GT: case ECOND_STR_GTI: | |
341 | + case ECOND_STR_LE: case ECOND_STR_LEI: case ECOND_STR_LT: case ECOND_STR_LTI: | |
342 | + break; | |
343 | + | |
344 | + default: | |
345 | + expand_string_message = US"comparator not handled for sort"; | |
346 | + goto EXPAND_FAILED; | |
347 | + } | |
348 | + | |
349 | while (isspace(*s)) s++; | |
350 | if (*s++ != '{') | |
351 | { | |
01e60269 AM |
352 | @@ -6307,11 +6398,10 @@ while (*s != 0) |
353 | if (skipping) continue; | |
b032efff AM |
354 | |
355 | while ((srcitem = string_nextinlist(&srclist, &sep, NULL, 0))) | |
01e60269 | 356 | - { |
b032efff | 357 | - uschar * dstitem; |
01e60269 | 358 | + { |
b032efff | 359 | + uschar * srcfield, * dstitem; |
01e60269 AM |
360 | gstring * newlist = NULL; |
361 | gstring * newkeylist = NULL; | |
b032efff AM |
362 | - uschar * srcfield; |
363 | ||
364 | DEBUG(D_expand) debug_printf_indent("%s: $item = \"%s\"\n", name, srcitem); | |
365 | ||
01e60269 | 366 | @@ -6332,25 +6422,15 @@ while (*s != 0) |
b032efff AM |
367 | while ((dstitem = string_nextinlist(&dstlist, &sep, NULL, 0))) |
368 | { | |
369 | uschar * dstfield; | |
370 | - uschar * expr; | |
371 | - BOOL before; | |
372 | ||
373 | /* field for comparison */ | |
374 | if (!(dstfield = string_nextinlist(&dstkeylist, &sep, NULL, 0))) | |
375 | goto sort_mismatch; | |
376 | ||
377 | - /* build and run condition string */ | |
378 | - expr = string_sprintf("%s{%s}{%s}", cmp, srcfield, dstfield); | |
379 | - | |
380 | - DEBUG(D_expand) debug_printf_indent("%s: cond = \"%s\"\n", name, expr); | |
381 | - if (!eval_condition(expr, &resetok, &before)) | |
382 | - { | |
383 | - expand_string_message = string_sprintf("comparison in sort: %s", | |
384 | - expr); | |
385 | - goto EXPAND_FAILED; | |
386 | - } | |
387 | + /* String-comparator names start with a letter; numeric names do not */ | |
388 | ||
389 | - if (before) | |
390 | + if (sortsbefore(cond_type, isalpha(opname[0]), | |
391 | + srcfield, dstfield)) | |
392 | { | |
393 | /* New-item sorts before this dst-item. Append new-item, | |
394 | then dst-item, then remainder of dst list. */ |