Gitweb:
https://git.exim.org/exim.git/commitdiff/21aa05977abff1eaa69bb97ef99080220915f7c0
Commit: 21aa05977abff1eaa69bb97ef99080220915f7c0
Parent: b48cf0793366791f5bdef8526a4956d4523c6778
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Fri Jul 5 15:38:15 2019 +0100
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Thu Jul 25 10:20:26 2019 +0100
Avoid re-expansion in ${sort }
---
doc/doc-txt/ChangeLog | 2 +
doc/doc-txt/cve-2019-13917 | 46 ++++++++++
src/src/expand.c | 209 +++++++++++++++++++++++++++++++--------------
3 files changed, 193 insertions(+), 64 deletions(-)
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index c1bbf26..2e83903 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -147,6 +147,8 @@ JH/30 Bug 2411: Fix DSN generation when RFC 3461 failure notification is
requested. Previously not bounce was generated and a log entry of
error ignored was made.
+JH/31 Avoid re-expansion in ${sort } expansion. (CVE-2019-13917)
+
Exim version 4.92
-----------------
diff --git a/doc/doc-txt/cve-2019-13917 b/doc/doc-txt/cve-2019-13917
new file mode 100644
index 0000000..fd94da8
--- /dev/null
+++ b/doc/doc-txt/cve-2019-13917
@@ -0,0 +1,46 @@
+CVE ID: CVE-2019-13917
+OVE ID: OVE-20190718-0006
+Date: 2019-07-18
+Credits: Jeremy Harris
+Version(s): 4.85 up to and including 4.92
+Issue: A local or remote attacker can execute programs with root
+ privileges - if you've an unusual configuration. See below.
+
+Conditions to be vulnerable
+===========================
+
+If your configuration uses the ${sort } expansion for items that can be
+controlled by an attacker (e.g. $local_part, $domain). The default
+config, as shipped by the Exim developers, does not contain ${sort }.
+
+Details
+=======
+
+The vulnerability is exploitable either remotely or locally and could
+be used to execute other programs with root privilege. The ${sort }
+expansion re-evaluates its items.
+
+Mitigation
+==========
+
+Do not use ${sort } in your configuration.
+
+Fix
+===
+
+Download and build a fixed version:
+
+ Tarballs: http://ftp.exim.org/pub/exim/exim4/
+ Git: https://github.com/Exim/exim.git
+ - tag exim-4.92.1
+ - branch exim-4.92+fixes
+
+The tagged commit is the officially released version. The +fixes branch
+isn't officially maintained, but contains useful patches *and* the
+security fix.
+
+If you can't install the above versions, ask your package maintainer for
+a version containing the backported fix. On request and depending on our
+resources we will support you in backporting the fix. (Please note,
+that Exim project officially doesn't support versions prior the current
+stable version.)
diff --git a/src/src/expand.c b/src/src/expand.c
index b3be83a..65c585d 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -2246,6 +2246,56 @@ return US item;
+/************************************************/
+/* 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 *
*************************************************/
@@ -2276,6 +2326,7 @@ BOOL is_forany, is_json, is_jsons;
int rc, cond_type, roffset;
int_eximarith_t num[2];
struct stat statbuf;
+uschar * opname;
uschar name[256];
const uschar *sub[10];
@@ -2288,37 +2339,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. */
@@ -2639,7 +2660,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,
@@ -2654,7 +2675,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;
@@ -2966,7 +2987,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
@@ -3008,14 +3029,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++;
@@ -3025,7 +3046,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;
}
@@ -3063,7 +3084,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 */
@@ -3084,7 +3105,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++;
@@ -3094,7 +3115,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;
}
@@ -3118,11 +3139,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) *yield = (tempcond == testfor);
@@ -3215,19 +3236,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 */
@@ -3237,7 +3259,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
}
@@ -3961,6 +3983,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 */
+}
+
+
/*************************************************
* Expand string *
*************************************************/
@@ -6284,9 +6356,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;
@@ -6321,6 +6394,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++ != '{')
{
@@ -6348,11 +6440,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);
@@ -6373,25 +6464,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. */
