From: Jeremy Harris Date: Mon, 4 May 2020 20:33:59 +0000 (+0100) Subject: Taint: When a non-wildcarded localpart affix is matched in a router, X-Git-Tag: exim-4.94-RC1~24 X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/19849de0dd5a6cf2ec8344a8adef9a433d7e7cf1 Taint: When a non-wildcarded localpart affix is matched in a router, make affix variables untainted --- diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index 828b757bb..ff6a115c5 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -12343,7 +12343,7 @@ If the origin of the data is an incoming message, the result of expanding this variable is tainted. When un untainted version is needed, one should be obtained from looking up the value in a local (therefore trusted) database. -See also &$domain_data$&. +Often &$domain_data$& is usable in this role. .wen @@ -12554,6 +12554,7 @@ For traditional full user accounts, use &%check_local_users%& and the For virtual users, store a suitable pathname component in the database which is used for account name validation, and use that retrieved value rather than this variable. +Often &$local_part_data$& is usable in this role. If needed, use a router &%address_data%& or &%set%& option for the retrieved data. .wen @@ -12568,9 +12569,14 @@ value of &$local_part$& during routing and subsequent delivery. The values of any prefix or suffix are in &$local_part_prefix$& and &$local_part_suffix$&, respectively. .new +.cindex "tainted data" If the affix specification included a wildcard then the portion of the affix matched by the wildcard is in -&$local_part_prefix_v$& or &$local_part_suffix_v$& as appropriate. +&$local_part_prefix_v$& or &$local_part_suffix_v$& as appropriate, +and both the whole and variable values are tainted. + +If the specification did not include a wildcard then +the affix variable value is not tainted. .wen When a message is being delivered to a file, pipe, or autoreply transport as a diff --git a/src/src/exim.c b/src/src/exim.c index 042b97861..6bc95d241 100644 --- a/src/src/exim.c +++ b/src/src/exim.c @@ -5570,17 +5570,15 @@ while (more) if (filter_test != FTEST_NONE) { - deliver_domain = (ftest_domain != NULL)? - ftest_domain : qualify_domain_recipient; + deliver_domain = ftest_domain ? ftest_domain : qualify_domain_recipient; deliver_domain_orig = deliver_domain; - deliver_localpart = (ftest_localpart != NULL)? - ftest_localpart : originator_login; + deliver_localpart = ftest_localpart ? ftest_localpart : originator_login; deliver_localpart_orig = deliver_localpart; deliver_localpart_prefix = ftest_prefix; deliver_localpart_suffix = ftest_suffix; deliver_home = originator_home; - if (return_path == NULL) + if (!return_path) { printf("Return-path copied from sender\n"); return_path = string_copy(sender_address); @@ -5591,14 +5589,14 @@ while (more) receive_add_recipient( string_sprintf("%s%s%s@%s", - (ftest_prefix == NULL)? US"" : ftest_prefix, + ftest_prefix ? ftest_prefix : US"", deliver_localpart, - (ftest_suffix == NULL)? US"" : ftest_suffix, + ftest_suffix ? ftest_suffix : US"", deliver_domain), -1); printf("Recipient = %s\n", recipients_list[0].address); - if (ftest_prefix != NULL) printf("Prefix = %s\n", ftest_prefix); - if (ftest_suffix != NULL) printf("Suffix = %s\n", ftest_suffix); + if (ftest_prefix) printf("Prefix = %s\n", ftest_prefix); + if (ftest_suffix) printf("Suffix = %s\n", ftest_suffix); if (chdir("/")) /* Get away from wherever the user is running this from */ { @@ -5611,13 +5609,13 @@ while (more) available to the user filter. We need to copy the filter variables explicitly. */ - if ((filter_test & FTEST_SYSTEM) != 0) + if (filter_test & FTEST_SYSTEM) if (!filter_runtest(filter_sfd, filter_test_sfile, TRUE, more)) exim_exit(EXIT_FAILURE); memcpy(filter_sn, filter_n, sizeof(filter_sn)); - if ((filter_test & FTEST_USER) != 0) + if (filter_test & FTEST_USER) if (!filter_runtest(filter_ufd, filter_test_ufile, FALSE, more)) exim_exit(EXIT_FAILURE); @@ -5629,9 +5627,9 @@ while (more) will be TRUE. If it is not, check on the number of messages received in this connection. */ - if (!session_local_queue_only && - smtp_accept_queue_per_connection > 0 && - receive_messagecount > smtp_accept_queue_per_connection) + if ( !session_local_queue_only + && smtp_accept_queue_per_connection > 0 + && receive_messagecount > smtp_accept_queue_per_connection) { session_local_queue_only = TRUE; queue_only_reason = 2; @@ -5647,16 +5645,12 @@ while (more) ones. However, there are odd cases where this is not wanted, so this can be changed by setting queue_only_load_latch false. */ - local_queue_only = session_local_queue_only; - if (!local_queue_only && queue_only_load >= 0) - { - local_queue_only = (load_average = OS_GETLOADAVG()) > queue_only_load; - if (local_queue_only) + if (!(local_queue_only = session_local_queue_only) && queue_only_load >= 0) + if ((local_queue_only = (load_average = OS_GETLOADAVG()) > queue_only_load)) { queue_only_reason = 3; if (queue_only_load_latch) session_local_queue_only = TRUE; } - } /* If running as an MUA wrapper, all queueing options and freezing options are ignored. */ diff --git a/src/src/filter.c b/src/src/filter.c index 5fc76a21e..402ad6ae5 100644 --- a/src/src/filter.c +++ b/src/src/filter.c @@ -2431,9 +2431,9 @@ suffixed version of the local part in the tests. */ if (deliver_localpart_prefix || deliver_localpart_suffix) { psself = string_sprintf("%s%s%s@%s", - (deliver_localpart_prefix == NULL)? US"" : deliver_localpart_prefix, + deliver_localpart_prefix ? deliver_localpart_prefix : US"", deliver_localpart, - (deliver_localpart_suffix == NULL)? US"" : deliver_localpart_suffix, + deliver_localpart_suffix ? deliver_localpart_suffix : US"", deliver_domain); psself_from = rewrite_one(psself, rewrite_from, NULL, FALSE, US"", global_rewrite_rules); diff --git a/src/src/route.c b/src/src/route.c index cee2f74c1..7538b7565 100644 --- a/src/src/route.c +++ b/src/src/route.c @@ -1655,8 +1655,18 @@ for (r = addr->start_router ? addr->start_router : routers; r; r = nextr) int plen = route_check_prefix(addr->local_part, r->prefix, &vlen); if (plen > 0) { - addr->prefix = string_copyn(addr->local_part, plen); - if (vlen) addr->prefix_v = string_copyn(addr->local_part, vlen); + /* If the variable-part is zero-length then the prefix was not + wildcarded and we can detaint-copy it since it matches the + (non-expandable) router option. Otherwise copy the (likely) tainted match + and the variable-part of the match from the local_part. */ + + if (vlen) + { + addr->prefix = string_copyn(addr->local_part, plen); + addr->prefix_v = string_copyn(addr->local_part, vlen); + } + else + addr->prefix = string_copyn_taint(addr->local_part, plen, FALSE); addr->local_part += plen; DEBUG(D_route) debug_printf("stripped prefix %s\n", addr->prefix); } @@ -1677,7 +1687,9 @@ for (r = addr->start_router ? addr->start_router : routers; r; r = nextr) if (slen > 0) { int lplen = Ustrlen(addr->local_part) - slen; - addr->suffix = addr->local_part + lplen; + addr->suffix = vlen + ? addr->local_part + lplen + : string_copy_taint(addr->local_part + lplen, slen); addr->suffix_v = addr->suffix + Ustrlen(addr->suffix) - vlen; addr->local_part = string_copyn(addr->local_part, lplen); DEBUG(D_route) debug_printf("stripped suffix %s\n", addr->suffix);