[exim-cvs] Taint: When a non-wildcarded localpart affix is m…

Top Page
Delete this message
Reply to this message
Author: Exim Git Commits Mailing List
Date:  
To: exim-cvs
Subject: [exim-cvs] Taint: When a non-wildcarded localpart affix is matched in a router,
Gitweb: https://git.exim.org/exim.git/commitdiff/19849de0dd5a6cf2ec8344a8adef9a433d7e7cf1
Commit:     19849de0dd5a6cf2ec8344a8adef9a433d7e7cf1
Parent:     40bffa31bd7057a0e88e29bb76fa63382d4aa1bc
Author:     Jeremy Harris <jgh146exb@???>
AuthorDate: Mon May 4 21:33:59 2020 +0100
Committer:  Jeremy Harris <jgh146exb@???>
CommitDate: Mon May 4 21:33:59 2020 +0100


    Taint: When a non-wildcarded localpart affix is matched in a router,
         make affix variables untainted
---
 doc/doc-docbook/spec.xfpt | 10 ++++++++--
 src/src/exim.c            | 34 ++++++++++++++--------------------
 src/src/filter.c          |  4 ++--
 src/src/route.c           | 18 +++++++++++++++---
 4 files changed, 39 insertions(+), 27 deletions(-)


diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 828b757..ff6a115 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 042b978..6bc95d2 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 5fc76a2..402ad6a 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 cee2f74..7538b75 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);