| 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 |
| 17 | @@ -2147,6 +2147,55 @@ return ret; |
| 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 * |
| 73 | @@ -2177,6 +2226,7 @@ BOOL sub2_honour_dollar = TRUE; |
| 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 | |
| 81 | @@ -2189,37 +2239,7 @@ for (;;) |
| 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. */ |
| 120 | @@ -2538,7 +2558,7 @@ switch(cond_type) |
| 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 | } |
| 128 | if (!(sub[i] = expand_string_internal(s+1, TRUE, &s, yield == NULL, |
| 129 | @@ -2553,7 +2573,7 @@ switch(cond_type) |
| 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; |
| 138 | @@ -2867,7 +2887,7 @@ switch(cond_type) |
| 139 | uschar *save_iterate_item = iterate_item; |
| 140 | int (*compare)(const uschar *, const uschar *); |
| 141 | |
| 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]); |
| 144 | |
| 145 | tempcond = FALSE; |
| 146 | compare = cond_type == ECOND_INLISTI |
| 147 | @@ -2909,14 +2929,14 @@ switch(cond_type) |
| 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++; |
| 164 | @@ -2926,7 +2946,7 @@ switch(cond_type) |
| 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 | |
| 173 | @@ -2958,7 +2978,7 @@ switch(cond_type) |
| 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 */ |
| 182 | @@ -2979,7 +2999,7 @@ switch(cond_type) |
| 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++; |
| 191 | @@ -2989,7 +3009,7 @@ switch(cond_type) |
| 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 | |
| 200 | @@ -3001,11 +3021,11 @@ switch(cond_type) |
| 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); |
| 214 | @@ -3098,19 +3118,20 @@ switch(cond_type) |
| 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 */ |
| 239 | @@ -3120,7 +3141,7 @@ return NULL; |
| 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 | } |
| 248 | @@ -3849,6 +3870,56 @@ return x; |
| 249 | |
| 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 | + |
| 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 | */ |
| 308 | |
| 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) |
| 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; |
| 326 | @@ -6280,6 +6352,25 @@ while (*s != 0) |
| 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 | { |
| 352 | @@ -6307,11 +6398,10 @@ while (*s != 0) |
| 353 | if (skipping) continue; |
| 354 | |
| 355 | while ((srcitem = string_nextinlist(&srclist, &sep, NULL, 0))) |
| 356 | - { |
| 357 | - uschar * dstitem; |
| 358 | + { |
| 359 | + uschar * srcfield, * dstitem; |
| 360 | gstring * newlist = NULL; |
| 361 | gstring * newkeylist = NULL; |
| 362 | - uschar * srcfield; |
| 363 | |
| 364 | DEBUG(D_expand) debug_printf_indent("%s: $item = \"%s\"\n", name, srcitem); |
| 365 | |
| 366 | @@ -6332,25 +6422,15 @@ while (*s != 0) |
| 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. */ |