Merge branch 'list_safety'

(gnutls fixes had updated some text docs)

8 files changed, 139 insertions, 25 deletions

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 51c9b8bab..81f69afab 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -10079,6 +10079,25 @@ string is lexically greater than the second string. For &%gt%& the comparison
 includes the case of letters, whereas for &%gti%& the comparison is
 case-independent.
 
+.new
+.vitem &*inlist&~{*&<&'string1'&>&*}{*&<&'string2'&>&*}*& &&&
+       &*inlisti&~{*&<&'string1'&>&*}{*&<&'string2'&>&*}*&
+.cindex "string" "comparison"
+.cindex "list" "iterative conditions"
+Both strings are expanded; the second string is treated as a list of simple
+strings; if the first string is a member of the second, then the condition
+is true.
+
+These are simpler to use versions of the more powerful &*forany*& condition.
+Examples, and the &*forany*& equivalents:
+.code
+${if inlist{needle}{foo:needle:bar}}
+  ${if forany{foo:needle:bar}{eq{$item}{needle}}}
+${if inlisti{Needle}{fOo:NeeDLE:bAr}}
+  ${if forany{fOo:NeeDLE:bAr}{eqi{$item}{Needle}}}
+.endd
+.wen
+
 .vitem &*isip&~{*&<&'string'&>&*}*&  &&&
        &*isip4&~{*&<&'string'&>&*}*& &&&
        &*isip6&~{*&<&'string'&>&*}*&
@@ -10265,6 +10284,11 @@ item can be used, as in all address lists, to cause subsequent items to
 have their local parts matched casefully. Domains are always matched
 caselessly.
 
+.new
+Note that <&'string2'&> is not itself subject to string expansion, unless
+Exim was built with the EXPAND_LISTMATCH_RHS option.
+.wen
+
 &*Note*&: Host lists are &'not'& supported in this way. This is because
 hosts have two identities: a name and an IP address, and it is not clear
 how to specify cleanly how such a test would work. However, IP addresses can be
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index e58136040..a911d80a1 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -117,6 +117,10 @@ PP/10 GnuTLS: support TLS 1.2 & 1.1.
       Use gnutls_certificate_verify_peers2() [patch from Andreas Metzler].
       Bugzilla 1095.
 
+PP/11 match_* no longer expand right-hand-side by default.
+      New compile-time build option, EXPAND_LISTMATCH_RHS.
+      New expansion conditions, "inlist", "inlisti".
+
 
 Exim version 4.76
 -----------------
diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff
index bf247e67e..d22d4e582 100644
--- a/doc/doc-txt/NewStuff
+++ b/doc/doc-txt/NewStuff
@@ -18,6 +18,15 @@ Version 4.77
  3. New variable $av_failed, set true if the AV scanner deferred; ie, when
     there is a problem talking to the AV scanner, or the AV scanner running.
 
+ 4. New expansion conditions, "inlist" and "inlisti", which take simple lists
+    and check if the search item is a member of the list.  This does not
+    support named lists, but does subject the list part to string expansion.
+
+ 5. Unless the new EXPAND_LISTMATCH_RHS build option is set when Exim was
+    built, Exim no longer performs string expansion on the second string of
+    the match_* expansion conditions: "match_address", "match_domain",
+    "match_ip" & "match_local_part".  Named lists can still be used.
+
 
 Version 4.76
 ------------
diff --git a/doc/doc-txt/OptionLists.txt b/doc/doc-txt/OptionLists.txt
index 21fd0fa26..6c820fbea 100644
--- a/doc/doc-txt/OptionLists.txt
+++ b/doc/doc-txt/OptionLists.txt
@@ -825,6 +825,7 @@ EXIWHAT_MULTIKILL_CMD        system**
 EXIWHAT_MULTIKILL_ARG        system**
 EXIWHAT_PS_ARG               system**     to list all processes
 EXIWHAT_PS_CMD               system**     path to ps command
+EXPAND_LISTMATCH_RHS         optional*    restore pre-4.77 match_*{}{} behaviour
 EXTRALIBS                    system       additional libraries
 EXTRALIBS_EXIM               system       additional libraries for Exim only
 EXTRALIBS_EXIMON             system       additional libraries for the monitor
diff --git a/src/README.UPDATING b/src/README.UPDATING
index fadf59aa9..8a0533b10 100644
--- a/src/README.UPDATING
+++ b/src/README.UPDATING
@@ -35,6 +35,13 @@ Exim version 4.77
    problem.  Prior to this release, supported values were "TLS1" and "SSL3",
    so you should be able to update configuration prior to update.
 
+ * The match_<type>{string1}{string2} expansion conditions no longer subject
+   string2 to string expansion, unless Exim was built with the new
+   "EXPAND_LISTMATCH_RHS" option.  Too many people have inadvertently created
+   insecure configurations that way.  If you need the functionality and turn on
+   that build option, please let the developers know, and know why, so we can
+   try to provide a safer mechanism for you.
+
 
 Exim version 4.74
 -----------------
diff --git a/src/src/EDITME b/src/src/EDITME
index 4c1c366b8..a180cd5cd 100644
--- a/src/src/EDITME
+++ b/src/src/EDITME
@@ -1205,6 +1205,26 @@ TMPDIR="/tmp"
 
 
 #------------------------------------------------------------------------------
+# Expanding match_* second paramters: BE CAREFUL IF ENABLING THIS!
+# It has proven too easy in practice for administrators to configure security
+# problems into their Exim install, by treating match_domain{}{} and friends
+# as a form of string comparison, where the second string comes from untrusted
+# data. Because these options take lists, which can include lookup;LOOKUPDATA
+# style elements, a foe can then cause Exim to, eg, execute an arbitrary MySQL
+# query, dropping tables.
+# From Exim 4.77 onwards, the second parameter is not expanded; it can still
+# be a list literal, or a macro, or a named list reference.  There is also
+# the new expansion condition "inlisti" which does expand the second parameter,
+# but treats it as a list of strings; also, there's "eqi" which is probably
+# what is normally wanted.
+#
+# If you really need to have the old behaviour, know what you are doing and
+# will not complain if your system is compromised as a result of doing so, then
+# uncomment this option to get the old behaviour back.
+
+# EXPAND_LISTMATCH_RHS=yes
+
+#------------------------------------------------------------------------------
 # Disabling the use of fsync(): DO NOT UNCOMMENT THE FOLLOWING LINE unless you
 # really, really, really know what you are doing. And even then, think again.
 # You should never uncomment this when compiling a binary for distribution.
diff --git a/src/src/config.h.defaults b/src/src/config.h.defaults
index a36cfb0d2..bc983c444 100644
--- a/src/src/config.h.defaults
+++ b/src/src/config.h.defaults
@@ -51,6 +51,7 @@ it's a default value. */
 /* Both uid and gid are triggered by this */
 #define EXIM_UID
 #define EXPAND_DLFUNC
+#define EXPAND_LISTMATCH_RHS
 
 #define FIXED_NEVER_USERS         "root"
 
diff --git a/src/src/expand.c b/src/src/expand.c
index ec4dd71f9..ef40fd0c5 100644
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -13,7 +13,7 @@
 
 /* Recursively called function */
 
-static uschar *expand_string_internal(uschar *, BOOL, uschar **, BOOL);
+static uschar *expand_string_internal(uschar *, BOOL, uschar **, BOOL, BOOL);
 
 #ifdef STAND_ALONE
 #ifndef SUPPORT_CRYPTEQ
@@ -258,6 +258,8 @@ static uschar *cond_table[] = {
   US"gei",
   US"gt",
   US"gti",
+  US"inlist",
+  US"inlisti",
   US"isip",
   US"isip4",
   US"isip6",
@@ -301,6 +303,8 @@ enum {
   ECOND_STR_GEI,
   ECOND_STR_GT,
   ECOND_STR_GTI,
+  ECOND_INLIST,
+  ECOND_INLISTI,
   ECOND_ISIP,
   ECOND_ISIP4,
   ECOND_ISIP6,
@@ -1699,7 +1703,7 @@ for (i = 0; i < n; i++)
     sub[i] = NULL;
     break;
     }
-  sub[i] = expand_string_internal(s+1, TRUE, &s, skipping);
+  sub[i] = expand_string_internal(s+1, TRUE, &s, skipping, TRUE);
   if (sub[i] == NULL) return 3;
   if (*s++ != '}') return 1;
   while (isspace(*s)) s++;
@@ -1771,6 +1775,7 @@ eval_condition(uschar *s, BOOL *yield)
 BOOL testfor = TRUE;
 BOOL tempcond, combined_cond;
 BOOL *subcondptr;
+BOOL *sub2_honour_dollar = TRUE;
 int i, rc, cond_type, roffset;
 int num[2];
 struct stat statbuf;
@@ -1903,7 +1908,7 @@ switch(cond_type)
   while (isspace(*s)) s++;
   if (*s != '{') goto COND_FAILED_CURLY_START;
 
-  sub[0] = expand_string_internal(s+1, TRUE, &s, yield == NULL);
+  sub[0] = expand_string_internal(s+1, TRUE, &s, yield == NULL, TRUE);
   if (sub[0] == NULL) return NULL;
   if (*s++ != '}') goto COND_FAILED_CURLY_END;
 
@@ -2016,22 +2021,30 @@ switch(cond_type)
   /* symbolic operators for numeric and string comparison, and a number of
   other operators, all requiring two arguments.
 
+  crypteq:           encrypts plaintext and compares against an encrypted text,
+                       using crypt(), crypt16(), MD5 or SHA-1
+  inlist/inlisti:    checks if first argument is in the list of the second
   match:             does a regular expression match and sets up the numerical
                        variables if it succeeds
   match_address:     matches in an address list
   match_domain:      matches in a domain list
   match_ip:          matches a host list that is restricted to IP addresses
   match_local_part:  matches in a local part list
-  crypteq:           encrypts plaintext and compares against an encrypted text,
-                       using crypt(), crypt16(), MD5 or SHA-1
   */
 
-  case ECOND_MATCH:
   case ECOND_MATCH_ADDRESS:
   case ECOND_MATCH_DOMAIN:
   case ECOND_MATCH_IP:
   case ECOND_MATCH_LOCAL_PART:
+#ifndef EXPAND_LISTMATCH_RHS
+    sub2_honour_dollar = FALSE;
+#endif
+    /* FALLTHROUGH */
+
   case ECOND_CRYPTEQ:
+  case ECOND_INLIST:
+  case ECOND_INLISTI:
+  case ECOND_MATCH:
 
   case ECOND_NUM_L:     /* Numerical comparisons */
   case ECOND_NUM_LE:
@@ -2053,6 +2066,13 @@ switch(cond_type)
 
   for (i = 0; i < 2; i++)
     {
+    /* Sometimes, we don't expand substrings; too many insecure configurations
+    created using match_address{}{} and friends, where the second param
+    includes information from untrustworthy sources. */
+    BOOL honour_dollar = TRUE;
+    if ((i > 0) && !sub2_honour_dollar)
+      honour_dollar = FALSE;
+
     while (isspace(*s)) s++;
     if (*s != '{')
       {
@@ -2061,7 +2081,8 @@ switch(cond_type)
         "after \"%s\"", name);
       return NULL;
       }
-    sub[i] = expand_string_internal(s+1, TRUE, &s, yield == NULL);
+    sub[i] = expand_string_internal(s+1, TRUE, &s, yield == NULL,
+        honour_dollar);
     if (sub[i] == NULL) return NULL;
     if (*s++ != '}') goto COND_FAILED_CURLY_END;
 
@@ -2320,6 +2341,7 @@ switch(cond_type)
       }
 
     else   /* {crypt} or {crypt16} and non-{ at start */
+           /* }-for-text-editors */
       {
       int which = 0;
       uschar *coded;
@@ -2366,6 +2388,30 @@ switch(cond_type)
       }
     break;
     #endif  /* SUPPORT_CRYPTEQ */
+
+    case ECOND_INLIST:
+    case ECOND_INLISTI:
+      {
+      int sep = 0;
+      BOOL found = FALSE;
+      uschar *save_iterate_item = iterate_item;
+      int (*compare)(const uschar *, const uschar *);
+
+      if (cond_type == ECOND_INLISTI)
+        compare = strcmpic;
+      else
+        compare = (int (*)(const uschar *, const uschar *)) strcmp;
+
+      while ((iterate_item = string_nextinlist(&sub[1], &sep, NULL, 0)) != NULL)
+        if (compare(sub[0], iterate_item) == 0)
+          {
+          found = TRUE;
+          break;
+          }
+      iterate_item = save_iterate_item;
+      *yield = found;
+      }
+
     }   /* Switch for comparison conditions */
 
   return s;    /* End of comparison conditions */
@@ -2437,7 +2483,7 @@ switch(cond_type)
 
     while (isspace(*s)) s++;
     if (*s++ != '{') goto COND_FAILED_CURLY_START;
-    sub[0] = expand_string_internal(s, TRUE, &s, (yield == NULL));
+    sub[0] = expand_string_internal(s, TRUE, &s, (yield == NULL), TRUE);
     if (sub[0] == NULL) return NULL;
     if (*s++ != '}') goto COND_FAILED_CURLY_END;
 
@@ -2719,7 +2765,7 @@ if (*s++ != '{') goto FAILED_CURLY;
 want this string. Set skipping in the call in the fail case (this will always
 be the case if we were already skipping). */
 
-sub1 = expand_string_internal(s, TRUE, &s, !yes);
+sub1 = expand_string_internal(s, TRUE, &s, !yes, TRUE);
 if (sub1 == NULL && (yes || !expand_string_forcedfail)) goto FAILED;
 expand_string_forcedfail = FALSE;
 if (*s++ != '}') goto FAILED_CURLY;
@@ -2744,7 +2790,7 @@ already skipping. */
 while (isspace(*s)) s++;
 if (*s == '{')
   {
-  sub2 = expand_string_internal(s+1, TRUE, &s, yes || skipping);
+  sub2 = expand_string_internal(s+1, TRUE, &s, yes || skipping, TRUE);
   if (sub2 == NULL && (!yes || !expand_string_forcedfail)) goto FAILED;
   expand_string_forcedfail = FALSE;
   if (*s++ != '}') goto FAILED_CURLY;
@@ -3307,6 +3353,8 @@ Arguments:
                  expansion is placed here (typically used with ket_ends)
   skipping       TRUE for recursive calls when the value isn't actually going
                  to be used (to allow for optimisation)
+  honour_dollar  TRUE if $ is to be expanded,
+                 FALSE if it's just another character
 
 Returns:         NULL if expansion fails:
                    expand_string_forcedfail is set TRUE if failure was forced
@@ -3316,7 +3364,7 @@ Returns:         NULL if expansion fails:
 
 static uschar *
 expand_string_internal(uschar *string, BOOL ket_ends, uschar **left,
-  BOOL skipping)
+  BOOL skipping, BOOL honour_dollar)
 {
 int ptr = 0;
 int size = Ustrlen(string)+ 64;
@@ -3372,7 +3420,7 @@ while (*s != 0)
 
   if (ket_ends && *s == '}') break;
 
-  if (*s != '$')
+  if (*s != '$' || !honour_dollar)
     {
     yield = string_cat(yield, &size, &ptr, s++, 1);
     continue;
@@ -3588,7 +3636,7 @@ while (*s != 0)
       while (isspace(*s)) s++;
       if (*s == '{')
         {
-        key = expand_string_internal(s+1, TRUE, &s, skipping);
+        key = expand_string_internal(s+1, TRUE, &s, skipping, TRUE);
         if (key == NULL) goto EXPAND_FAILED;
         if (*s++ != '}') goto EXPAND_FAILED_CURLY;
         while (isspace(*s)) s++;
@@ -3654,7 +3702,7 @@ while (*s != 0)
       first. */
 
       if (*s != '{') goto EXPAND_FAILED_CURLY;
-      filename = expand_string_internal(s+1, TRUE, &s, skipping);
+      filename = expand_string_internal(s+1, TRUE, &s, skipping, TRUE);
       if (filename == NULL) goto EXPAND_FAILED;
       if (*s++ != '}') goto EXPAND_FAILED_CURLY;
       while (isspace(*s)) s++;
@@ -4323,7 +4371,7 @@ while (*s != 0)
 
       if (*s == '{')
         {
-        if (expand_string_internal(s+1, TRUE, &s, TRUE) == NULL)
+        if (expand_string_internal(s+1, TRUE, &s, TRUE, TRUE) == NULL)
           goto EXPAND_FAILED;
         if (*s++ != '}') goto EXPAND_FAILED_CURLY;
         while (isspace(*s)) s++;
@@ -4338,7 +4386,7 @@ while (*s != 0)
       SOCK_FAIL:
       if (*s != '{') goto EXPAND_FAILED;
       DEBUG(D_any) debug_printf("%s\n", expand_string_message);
-      arg = expand_string_internal(s+1, TRUE, &s, FALSE);
+      arg = expand_string_internal(s+1, TRUE, &s, FALSE, TRUE);
       if (arg == NULL) goto EXPAND_FAILED;
       yield = string_cat(yield, &size, &ptr, arg, Ustrlen(arg));
       if (*s++ != '}') goto EXPAND_FAILED_CURLY;
@@ -4367,7 +4415,7 @@ while (*s != 0)
 
       while (isspace(*s)) s++;
       if (*s != '{') goto EXPAND_FAILED_CURLY;
-      arg = expand_string_internal(s+1, TRUE, &s, skipping);
+      arg = expand_string_internal(s+1, TRUE, &s, skipping, TRUE);
       if (arg == NULL) goto EXPAND_FAILED;
       while (isspace(*s)) s++;
       if (*s++ != '}') goto EXPAND_FAILED_CURLY;
@@ -4798,7 +4846,7 @@ while (*s != 0)
         while (isspace(*s)) s++;
         if (*s == '{')
           {
-          sub[i] = expand_string_internal(s+1, TRUE, &s, skipping);
+          sub[i] = expand_string_internal(s+1, TRUE, &s, skipping, TRUE);
           if (sub[i] == NULL) goto EXPAND_FAILED;
           if (*s++ != '}') goto EXPAND_FAILED_CURLY;
 
@@ -4893,7 +4941,7 @@ while (*s != 0)
       while (isspace(*s)) s++;
       if (*s++ != '{') goto EXPAND_FAILED_CURLY;
 
-      list = expand_string_internal(s, TRUE, &s, skipping);
+      list = expand_string_internal(s, TRUE, &s, skipping, TRUE);
       if (list == NULL) goto EXPAND_FAILED;
       if (*s++ != '}') goto EXPAND_FAILED_CURLY;
 
@@ -4901,7 +4949,7 @@ while (*s != 0)
         {
         while (isspace(*s)) s++;
         if (*s++ != '{') goto EXPAND_FAILED_CURLY;
-        temp = expand_string_internal(s, TRUE, &s, skipping);
+        temp = expand_string_internal(s, TRUE, &s, skipping, TRUE);
         if (temp == NULL) goto EXPAND_FAILED;
         lookup_value = temp;
         if (*s++ != '}') goto EXPAND_FAILED_CURLY;
@@ -4925,7 +4973,7 @@ while (*s != 0)
         }
       else
         {
-        temp = expand_string_internal(s, TRUE, &s, TRUE);
+        temp = expand_string_internal(s, TRUE, &s, TRUE, TRUE);
         }
 
       if (temp == NULL)
@@ -4984,7 +5032,7 @@ while (*s != 0)
 
         else
           {
-          temp = expand_string_internal(expr, TRUE, NULL, skipping);
+          temp = expand_string_internal(expr, TRUE, NULL, skipping, TRUE);
           if (temp == NULL)
             {
             iterate_item = save_iterate_item;
@@ -5166,7 +5214,7 @@ while (*s != 0)
     {
     int c;
     uschar *arg = NULL;
-    uschar *sub = expand_string_internal(s+1, TRUE, &s, skipping);
+    uschar *sub = expand_string_internal(s+1, TRUE, &s, skipping, TRUE);
     if (sub == NULL) goto EXPAND_FAILED;
     s++;
 
@@ -5241,7 +5289,7 @@ while (*s != 0)
 
       case EOP_EXPAND:
         {
-        uschar *expanded = expand_string_internal(sub, FALSE, NULL, skipping);
+        uschar *expanded = expand_string_internal(sub, FALSE, NULL, skipping, TRUE);
         if (expanded == NULL)
           {
           expand_string_message =
@@ -6009,7 +6057,7 @@ expand_string(uschar *string)
 search_find_defer = FALSE;
 malformed_header = FALSE;
 return (Ustrpbrk(string, "$\\") == NULL)? string :
-  expand_string_internal(string, FALSE, NULL, FALSE);
+  expand_string_internal(string, FALSE, NULL, FALSE, TRUE);
 }