Taint: When a non-wildcarded localpart affix is matched in a router,
authorJeremy Harris <jgh146exb@wizmail.org>
Mon, 4 May 2020 20:33:59 +0000 (21:33 +0100)
committerJeremy Harris <jgh146exb@wizmail.org>
Mon, 4 May 2020 20:33:59 +0000 (21:33 +0100)
 make affix variables untainted

doc/doc-docbook/spec.xfpt
src/src/exim.c
src/src/filter.c
src/src/route.c

index 828b757..ff6a115 100644 (file)
@@ -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
index 042b978..6bc95d2 100644 (file)
@@ -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. */
index 5fc76a2..402ad6a 100644 (file)
@@ -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);
index cee2f74..7538b75 100644 (file)
@@ -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);