Taint: When a non-wildcarded localpart affix is matched in a router,

	 make affix variables untainted

4 files changed, 39 insertions, 27 deletions

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);