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 828b757bb665f8e86dadc4d3ca28889b3fab39aa..ff6a115c54c10a5e42f5968fcfdcc0bff446cc88 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 042b97861c2f15368a2cdd1a9f96555a3d559fa3..6bc95d241061f9f9d64f52ae7f7fac2f1b22c94f 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 5fc76a21ea3d71c43205bde9c9b3aeb84a8f3011..402ad6ae53f52065f4e7df29b0f164ce2f67cc26 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 cee2f74c1ba47130d6dad13210d0dacc4c2f1616..7538b75659d6528c1e27297ed5130e1f2b8ae1fb 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);