--- /dev/null
+From cf84d126bc1f04746eb7c8e8b3468f7e70add3ec Mon Sep 17 00:00:00 2001
+From: Jeremy Harris <jgh146exb@wizmail.org>
+Date: Fri, 5 Jul 2019 15:38:15 +0100
+Subject: [PATCH] Avoid re-expansion in ${sort } CVE-2019-13917
+ OVE-20190718-0006
+
+(cherry picked from commit 5c887f836e4d8e3f79da1c15565b56b40d9bd0dd)
+---
+ doc/ChangeLog | 6 ++
+ doc/doc-txt/cve-2019-13917 | 46 ++++++++
+ src/expand.c | 214 +++++++++++++++++++++++++------------
+ 3 files changed, 199 insertions(+), 67 deletions(-)
+ create mode 100644 doc/doc-txt/cve-2019-13917
+
+--- a/src/expand.c
++++ b/src/expand.c
+@@ -2115,6 +2115,55 @@ return ret;
+
+
+
++/************************************************/
++/* Return offset in ops table, or -1 if not found.
++Repoint to just after the operator in the string.
++
++Argument:
++ ss string representation of operator
++ opname split-out operator name
++*/
++
++static int
++identify_operator(const uschar ** ss, uschar ** opname)
++{
++const uschar * s = *ss;
++uschar name[256];
++
++/* Numeric comparisons are symbolic */
++
++if (*s == '=' || *s == '>' || *s == '<')
++ {
++ int p = 0;
++ name[p++] = *s++;
++ if (*s == '=')
++ {
++ name[p++] = '=';
++ s++;
++ }
++ name[p] = 0;
++ }
++
++/* All other conditions are named */
++
++else
++ s = read_name(name, sizeof(name), s, US"_");
++*ss = s;
++
++/* If we haven't read a name, it means some non-alpha character is first. */
++
++if (!name[0])
++ {
++ expand_string_message = string_sprintf("condition name expected, "
++ "but found \"%.16s\"", s);
++ return -1;
++ }
++if (opname)
++ *opname = string_copy(name);
++
++return chop_match(name, cond_table, nelem(cond_table));
++}
++
+
+ /*************************************************
+ * Read and evaluate a condition *
+@@ -2145,6 +2194,7 @@ BOOL sub2_honour_dollar = TRUE;
+ int i, rc, cond_type, roffset;
+ int_eximarith_t num[2];
+ struct stat statbuf;
++uschar * opname;
+ uschar name[256];
+ const uschar *sub[10];
+
+@@ -2157,37 +2207,7 @@ for (;;)
+ if (*s == '!') { testfor = !testfor; s++; } else break;
+ }
+
+-/* Numeric comparisons are symbolic */
+-
+-if (*s == '=' || *s == '>' || *s == '<')
+- {
+- int p = 0;
+- name[p++] = *s++;
+- if (*s == '=')
+- {
+- name[p++] = '=';
+- s++;
+- }
+- name[p] = 0;
+- }
+-
+-/* All other conditions are named */
+-
+-else s = read_name(name, 256, s, US"_");
+-
+-/* If we haven't read a name, it means some non-alpha character is first. */
+-
+-if (name[0] == 0)
+- {
+- expand_string_message = string_sprintf("condition name expected, "
+- "but found \"%.16s\"", s);
+- return NULL;
+- }
+-
+-/* Find which condition we are dealing with, and switch on it */
+-
+-cond_type = chop_match(name, cond_table, nelem(cond_table));
+-switch(cond_type)
++switch(cond_type = identify_operator(&s, &opname))
+ {
+ /* def: tests for a non-empty variable, or for the existence of a header. If
+ yield == NULL we are in a skipping state, and don't care about the answer. */
+@@ -2506,7 +2526,7 @@ switch(cond_type)
+ {
+ if (i == 0) goto COND_FAILED_CURLY_START;
+ expand_string_message = string_sprintf("missing 2nd string in {} "
+- "after \"%s\"", name);
++ "after \"%s\"", opname);
+ return NULL;
+ }
+ sub[i] = expand_string_internal(s+1, TRUE, &s, yield == NULL,
+@@ -2518,7 +2538,7 @@ switch(cond_type)
+ conditions that compare numbers do not start with a letter. This just saves
+ checking for them individually. */
+
+- if (!isalpha(name[0]) && yield != NULL)
++ if (!isalpha(opname[0]) && yield != NULL)
+ if (sub[i][0] == 0)
+ {
+ num[i] = 0;
+@@ -2832,7 +2852,7 @@ switch(cond_type)
+ uschar *save_iterate_item = iterate_item;
+ int (*compare)(const uschar *, const uschar *);
+
+- DEBUG(D_expand) debug_printf_indent("condition: %s\n", name);
++ DEBUG(D_expand) debug_printf_indent("condition: %s\n", opname);
+
+ tempcond = FALSE;
+ compare = cond_type == ECOND_INLISTI
+@@ -2871,14 +2891,14 @@ switch(cond_type)
+ if (*s != '{') /* }-for-text-editors */
+ {
+ expand_string_message = string_sprintf("each subcondition "
+- "inside an \"%s{...}\" condition must be in its own {}", name);
++ "inside an \"%s{...}\" condition must be in its own {}", opname);
+ return NULL;
+ }
+
+ if (!(s = eval_condition(s+1, resetok, subcondptr)))
+ {
+ expand_string_message = string_sprintf("%s inside \"%s{...}\" condition",
+- expand_string_message, name);
++ expand_string_message, opname);
+ return NULL;
+ }
+ while (isspace(*s)) s++;
+@@ -2888,7 +2908,7 @@ switch(cond_type)
+ {
+ /* {-for-text-editors */
+ expand_string_message = string_sprintf("missing } at end of condition "
+- "inside \"%s\" group", name);
++ "inside \"%s\" group", opname);
+ return NULL;
+ }
+
+@@ -2920,7 +2940,7 @@ switch(cond_type)
+ int sep = 0;
+ uschar *save_iterate_item = iterate_item;
+
+- DEBUG(D_expand) debug_printf_indent("condition: %s\n", name);
++ DEBUG(D_expand) debug_printf_indent("condition: %s\n", opname);
+
+ while (isspace(*s)) s++;
+ if (*s++ != '{') goto COND_FAILED_CURLY_START; /* }-for-text-editors */
+@@ -2941,7 +2961,7 @@ switch(cond_type)
+ if (!(s = eval_condition(sub[1], resetok, NULL)))
+ {
+ expand_string_message = string_sprintf("%s inside \"%s\" condition",
+- expand_string_message, name);
++ expand_string_message, opname);
+ return NULL;
+ }
+ while (isspace(*s)) s++;
+@@ -2951,7 +2971,7 @@ switch(cond_type)
+ {
+ /* {-for-text-editors */
+ expand_string_message = string_sprintf("missing } at end of condition "
+- "inside \"%s\"", name);
++ "inside \"%s\"", opname);
+ return NULL;
+ }
+
+@@ -2963,11 +2983,11 @@ switch(cond_type)
+ if (!eval_condition(sub[1], resetok, &tempcond))
+ {
+ expand_string_message = string_sprintf("%s inside \"%s\" condition",
+- expand_string_message, name);
++ expand_string_message, opname);
+ iterate_item = save_iterate_item;
+ return NULL;
+ }
+- DEBUG(D_expand) debug_printf_indent("%s: condition evaluated to %s\n", name,
++ DEBUG(D_expand) debug_printf_indent("%s: condition evaluated to %s\n", opname,
+ tempcond? "true":"false");
+
+ if (yield != NULL) *yield = (tempcond == testfor);
+@@ -3060,19 +3080,20 @@ switch(cond_type)
+ /* Unknown condition */
+
+ default:
+- expand_string_message = string_sprintf("unknown condition \"%s\"", name);
+- return NULL;
++ if (!expand_string_message || !*expand_string_message)
++ expand_string_message = string_sprintf("unknown condition \"%s\"", opname);
++ return NULL;
+ } /* End switch on condition type */
+
+ /* Missing braces at start and end of data */
+
+ COND_FAILED_CURLY_START:
+-expand_string_message = string_sprintf("missing { after \"%s\"", name);
++expand_string_message = string_sprintf("missing { after \"%s\"", opname);
+ return NULL;
+
+ COND_FAILED_CURLY_END:
+ expand_string_message = string_sprintf("missing } at end of \"%s\" condition",
+- name);
++ opname);
+ return NULL;
+
+ /* A condition requires code that is not compiled */
+@@ -3082,7 +3103,7 @@ return NULL;
+ !defined(SUPPORT_CRYPTEQ) || !defined(CYRUS_SASLAUTHD_SOCKET)
+ COND_FAILED_NOT_COMPILED:
+ expand_string_message = string_sprintf("support for \"%s\" not compiled",
+- name);
++ opname);
+ return NULL;
+ #endif
+ }
+@@ -3793,6 +3814,58 @@ return x;
+ }
+
+
++/************************************************/
++/* Comparison operation for sort expansion. We need to avoid
++re-expanding the fields being compared, so need a custom routine.
++
++Arguments:
++ cond_type Comparison operator code
++ leftarg, rightarg Arguments for comparison
++
++Return true iff (leftarg compare rightarg)
++*/
++
++static BOOL
++sortsbefore(int cond_type, BOOL alpha_cond,
++ const uschar * leftarg, const uschar * rightarg)
++{
++int_eximarith_t l_num, r_num;
++
++if (!alpha_cond)
++ {
++ l_num = expanded_string_integer(leftarg, FALSE);
++ if (expand_string_message) return FALSE;
++ r_num = expanded_string_integer(rightarg, FALSE);
++ if (expand_string_message) return FALSE;
++
++ switch (cond_type)
++ {
++ case ECOND_NUM_G: return l_num > r_num;
++ case ECOND_NUM_GE: return l_num >= r_num;
++ case ECOND_NUM_L: return l_num < r_num;
++ case ECOND_NUM_LE: return l_num <= r_num;
++ default: break;
++ }
++ }
++else
++ switch (cond_type)
++ {
++ case ECOND_STR_LT: return Ustrcmp (leftarg, rightarg) < 0;
++ case ECOND_STR_LTI: return strcmpic(leftarg, rightarg) < 0;
++ case ECOND_STR_LE: return Ustrcmp (leftarg, rightarg) <= 0;
++ case ECOND_STR_LEI: return strcmpic(leftarg, rightarg) <= 0;
++ case ECOND_STR_GT: return Ustrcmp (leftarg, rightarg) > 0;
++ case ECOND_STR_GTI: return strcmpic(leftarg, rightarg) > 0;
++ case ECOND_STR_GE: return Ustrcmp (leftarg, rightarg) >= 0;
++ case ECOND_STR_GEI: return strcmpic(leftarg, rightarg) >= 0;
++ default: break;
++ }
++return FALSE; /* should not happen */
++}
++
++
++
++
+
+ /*************************************************
+ * Expand string *
+@@ -5904,9 +5977,10 @@ while (*s != 0)
+
+ case EITEM_SORT:
+ {
++ int cond_type;
+ int sep = 0;
+ const uschar *srclist, *cmp, *xtract;
+- uschar *srcitem;
++ uschar * opname, * srcitem;
+ const uschar *dstlist = NULL, *dstkeylist = NULL;
+ uschar * tmp;
+ uschar *save_iterate_item = iterate_item;
+@@ -5941,6 +6015,25 @@ while (*s != 0)
+ goto EXPAND_FAILED_CURLY;
+ }
+
++ if ((cond_type = identify_operator(&cmp, &opname)) == -1)
++ {
++ if (!expand_string_message)
++ expand_string_message = string_sprintf("unknown condition \"%s\"", s);
++ goto EXPAND_FAILED;
++ }
++ switch(cond_type)
++ {
++ case ECOND_NUM_L: case ECOND_NUM_LE:
++ case ECOND_NUM_G: case ECOND_NUM_GE:
++ case ECOND_STR_GE: case ECOND_STR_GEI: case ECOND_STR_GT: case ECOND_STR_GTI:
++ case ECOND_STR_LE: case ECOND_STR_LEI: case ECOND_STR_LT: case ECOND_STR_LTI:
++ break;
++
++ default:
++ expand_string_message = US"comparator not handled for sort";
++ goto EXPAND_FAILED;
++ }
++
+ while (isspace(*s)) s++;
+ if (*s++ != '{')
+ {
+@@ -5969,10 +6062,9 @@ while (*s != 0)
+
+ while ((srcitem = string_nextinlist(&srclist, &sep, NULL, 0)))
+ {
+- uschar * dstitem;
++ uschar * srcfield, * dstitem;
+ uschar * newlist = NULL;
+ uschar * newkeylist = NULL;
+- uschar * srcfield;
+
+ DEBUG(D_expand) debug_printf_indent("%s: $item = \"%s\"\n", name, srcitem);
+
+@@ -5993,25 +6085,15 @@ while (*s != 0)
+ while ((dstitem = string_nextinlist(&dstlist, &sep, NULL, 0)))
+ {
+ uschar * dstfield;
+- uschar * expr;
+- BOOL before;
+
+ /* field for comparison */
+ if (!(dstfield = string_nextinlist(&dstkeylist, &sep, NULL, 0)))
+ goto sort_mismatch;
+
+- /* build and run condition string */
+- expr = string_sprintf("%s{%s}{%s}", cmp, srcfield, dstfield);
+-
+- DEBUG(D_expand) debug_printf_indent("%s: cond = \"%s\"\n", name, expr);
+- if (!eval_condition(expr, &resetok, &before))
+- {
+- expand_string_message = string_sprintf("comparison in sort: %s",
+- expr);
+- goto EXPAND_FAILED;
+- }
++ /* String-comparator names start with a letter; numeric names do not */
+
+- if (before)
++ if (sortsbefore(cond_type, isalpha(opname[0]),
++ srcfield, dstfield))
+ {
+ /* New-item sorts before this dst-item. Append new-item,
+ then dst-item, then remainder of dst list. */
HCoop / hcoop / debian / exim4.git / commitdiff