From: Phil Pennock Date: Mon, 3 Oct 2011 11:36:12 +0000 (-0400) Subject: Merge branch 'list_safety' X-Git-Tag: exim-4_77_RC1~2 X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/d6cc7c78f624e505bb889c8ccd2879706d6dc9e1?hp=d641a34087b20f3f6026eeaf35ddebfdf70cbf65 Merge branch 'list_safety' (gnutls fixes had updated some text docs) --- 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_{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 @@ -1204,6 +1204,26 @@ TMPDIR="/tmp" # SUPPORT_MOVE_FROZEN_MESSAGES=yes +#------------------------------------------------------------------------------ +# 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. 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); }