path: root/debian/patches/77_Avoid-re-expansion-in-sort-CVE-2019-13917-OVE-201907.patch

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
@@ -2147,6 +2147,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           *
@@ -2177,6 +2226,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];
 
@@ -2189,37 +2239,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. */
@@ -2538,7 +2558,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;
       }
     if (!(sub[i] = expand_string_internal(s+1, TRUE, &s, yield == NULL,
@@ -2553,7 +2573,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;
@@ -2867,7 +2887,7 @@ switch(cond_type)
       uschar *save_iterate_item = iterate_item;
       int (*compare)(const uschar *, const uschar *);
 
-      DEBUG(D_expand) debug_printf_indent("condition: %s  item: %s\n", name, sub[0]);
+      DEBUG(D_expand) debug_printf_indent("condition: %s  item: %s\n", opname, sub[0]);
 
       tempcond = FALSE;
       compare = cond_type == ECOND_INLISTI
@@ -2909,14 +2929,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++;
@@ -2926,7 +2946,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;
       }
 
@@ -2958,7 +2978,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 */
@@ -2979,7 +2999,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++;
@@ -2989,7 +3009,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;
       }
 
@@ -3001,11 +3021,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);
@@ -3098,19 +3118,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 */
@@ -3120,7 +3141,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
 }
@@ -3849,6 +3870,56 @@ 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 */
+}
+
+
 /* Return pointer to dewrapped string, with enclosing specified chars removed.
 The given string is modified on return.  Leading whitespace is skipped while
 looking for the opening wrap character, then the rest is scanned for the trailing
@@ -3905,7 +3976,7 @@ The element may itself be an object or a
 Return NULL when the list is empty.
 */
 
-uschar *
+static uschar *
 json_nextinlist(const uschar ** list)
 {
 unsigned array_depth = 0, object_depth = 0;
@@ -6243,9 +6314,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;
@@ -6280,6 +6352,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++ != '{')
         {
@@ -6307,11 +6398,10 @@ while (*s != 0)
       if (skipping) continue;
 
       while ((srcitem = string_nextinlist(&srclist, &sep, NULL, 0)))
-        {
-	uschar * dstitem;
+	{
+	uschar * srcfield, * dstitem;
 	gstring * newlist = NULL;
 	gstring * newkeylist = NULL;
-	uschar * srcfield;
 
         DEBUG(D_expand) debug_printf_indent("%s: $item = \"%s\"\n", name, srcitem);
 
@@ -6332,25 +6422,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. */