From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Sun, 5 Apr 2020 22:21:40 +0000 (+0100)
Subject: Taint: check on supplied buffer vs. list when extracting elements
X-Git-Url: https://git.exim.org/users/jgh/exim.git/commitdiff_plain/ba74fb8d95d2e9af2122e0a95c4d5334b4f0466c?hp=ab1604ea9202d3dbc0fd7fd230dc693d83d3092c

Taint: check on supplied buffer vs. list when extracting elements
---

diff --git a/src/src/acl.c b/src/src/acl.c
index 9ed005781..02251b197 100644
--- a/src/src/acl.c
+++ b/src/src/acl.c
@@ -1600,7 +1600,7 @@ an error if options are given for items that don't expect them.
 
 uschar *slash = Ustrchr(arg, '/');
 const uschar *list = arg;
-uschar *ss = string_nextinlist(&list, &sep, big_buffer, big_buffer_size);
+uschar *ss = string_nextinlist(&list, &sep, NULL, 0);
 verify_type_t * vp;
 
 if (!ss) goto BAD_VERIFY;
diff --git a/src/src/daemon.c b/src/src/daemon.c
index 3b21d272b..6560da1d4 100644
--- a/src/src/daemon.c
+++ b/src/src/daemon.c
@@ -1353,7 +1353,7 @@ if (f.daemon_listen && !f.inetd_wait_mode)
 
     list = override_local_interfaces;
     sep = 0;
-    while ((s = string_nextinlist(&list, &sep, big_buffer, big_buffer_size)))
+    while ((s = string_nextinlist(&list, &sep, NULL, 0)))
       {
       uschar joinstr[4];
       gstring ** gp = Ustrpbrk(s, ".:") ? &new_local_interfaces : &new_smtp_port;
@@ -1391,13 +1391,13 @@ if (f.daemon_listen && !f.inetd_wait_mode)
 
   list = daemon_smtp_port;
   sep = 0;
-  while ((s = string_nextinlist(&list, &sep, big_buffer, big_buffer_size)))
+  while ((s = string_nextinlist(&list, &sep, NULL, 0)))
     pct++;
   default_smtp_port = store_get((pct+1) * sizeof(int), FALSE);
   list = daemon_smtp_port;
   sep = 0;
   for (pct = 0;
-       (s = string_nextinlist(&list, &sep, big_buffer, big_buffer_size));
+       (s = string_nextinlist(&list, &sep, NULL, 0));
        pct++)
     {
     if (isdigit(*s))
diff --git a/src/src/functions.h b/src/src/functions.h
index efd039b5e..8206c480e 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -521,7 +521,6 @@ extern int     string_is_ip_address(const uschar *, int *);
 #ifdef SUPPORT_I18N
 extern BOOL    string_is_utf8(const uschar *);
 #endif
-extern uschar *string_nextinlist(const uschar **, int *, uschar *, int);
 extern const uschar *string_printing2(const uschar *, BOOL);
 extern uschar *string_split_message(uschar *);
 extern uschar *string_unprinting(uschar *);
@@ -549,6 +548,11 @@ extern gstring *string_vformat_trc(gstring *, const uschar *, unsigned,
 extern uschar *string_open_failed_trc(int, const uschar *, unsigned,
 			const char *, ...) PRINTF_FUNCTION(4,5);
 
+#define string_nextinlist(lp, sp, b, l) \
+	string_nextinlist_trc((lp), (sp), (b), (l), US __FUNCTION__, __LINE__)
+extern uschar *string_nextinlist_trc(const uschar **listptr, int *separator, uschar *buffer, int buflen,
+			const uschar * func, int line);
+
 extern int     strcmpic(const uschar *, const uschar *);
 extern int     strncmpic(const uschar *, const uschar *, int);
 extern uschar *strstric(uschar *, uschar *, BOOL);
diff --git a/src/src/host.c b/src/src/host.c
index 3361d5912..1426bff97 100644
--- a/src/src/host.c
+++ b/src/src/host.c
@@ -729,6 +729,7 @@ host_build_ifacelist(const uschar *list, uschar *name)
 int sep = 0;
 uschar *s;
 ip_address_item * yield = NULL, * last = NULL, * next;
+BOOL taint = is_tainted(list);
 
 while ((s = string_nextinlist(&list, &sep, NULL, 0)))
   {
@@ -747,7 +748,7 @@ while ((s = string_nextinlist(&list, &sep, NULL, 0)))
   address above. The field in the ip_address_item is large enough to hold an
   IPv6 address. */
 
-  next = store_get(sizeof(ip_address_item), FALSE);
+  next = store_get(sizeof(ip_address_item), taint);
   next->next = NULL;
   Ustrcpy(next->address, s);
   next->port = port;
diff --git a/src/src/match.c b/src/src/match.c
index a0899bf3e..6da1d2766 100644
--- a/src/src/match.c
+++ b/src/src/match.c
@@ -446,7 +446,6 @@ BOOL ignore_defer = FALSE;
 const uschar *list;
 uschar *sss;
 uschar *ot = NULL;
-uschar buffer[1024];
 
 /* Save time by not scanning for the option name when we don't need it. */
 
@@ -506,12 +505,12 @@ else
 
 /* For an unnamed list, use the expanded version in comments */
 
-HDEBUG(D_any) if (ot == NULL) ot = string_sprintf("%s in \"%s\"?", name, list);
+HDEBUG(D_any) if (!ot) ot = string_sprintf("%s in \"%s\"?", name, list);
 
 /* Now scan the list and process each item in turn, until one of them matches,
 or we hit an error. */
 
-while ((sss = string_nextinlist(&list, &sep, buffer, sizeof(buffer))))
+while ((sss = string_nextinlist(&list, &sep, NULL, 0)))
   {
   uschar * ss = sss;
 
diff --git a/src/src/string.c b/src/src/string.c
index 4ef2fee62..80cf49fdf 100644
--- a/src/src/string.c
+++ b/src/src/string.c
@@ -863,7 +863,8 @@ Returns:     pointer to buffer, containing the next substring,
 */
 
 uschar *
-string_nextinlist(const uschar **listptr, int *separator, uschar *buffer, int buflen)
+string_nextinlist_trc(const uschar **listptr, int *separator, uschar *buffer, int buflen,
+ const uschar * func, int line)
 {
 int sep = *separator;
 const uschar *s = *listptr;
@@ -906,6 +907,8 @@ sep_is_special = iscntrl(sep);
 if (buffer)
   {
   int p = 0;
+  if (is_tainted(s) && !is_tainted(buffer))
+    die_tainted(US"string_nextinlist", func, line);
   for (; *s; s++)
     {
     if (*s == sep && (*(++s) != sep || sep_is_special)) break;
@@ -1638,7 +1641,7 @@ doesn't seem much we can do about that. */
 
 va_start(ap, format);
 (void) string_vformat_trc(g, func, line, STRING_SPRINTF_BUFFER_SIZE,
-	0, format, ap);
+	SVFMT_REBUFFER, format, ap);
 string_from_gstring(g);
 gstring_release_unused(g);
 va_end(ap);
diff --git a/src/src/transports/pipe.c b/src/src/transports/pipe.c
index 6a7f150ac..27422bd42 100644
--- a/src/src/transports/pipe.c
+++ b/src/src/transports/pipe.c
@@ -678,7 +678,7 @@ if (envlist)
     return FALSE;
     }
 
-while ((ss = string_nextinlist(&envlist, &envsep, big_buffer, big_buffer_size)))
+while ((ss = string_nextinlist(&envlist, &envsep, NULL, 0)))
    {
    if (envcount > nelem(envp) - 2)
      {
diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c
index fc5bb7801..5fb22bcd3 100644
--- a/src/src/transports/smtp.c
+++ b/src/src/transports/smtp.c
@@ -911,11 +911,9 @@ names = string_copyn(expand_nstring[1], expand_nlength[1]);
 for (au = auths, authnum = 0; au; au = au->next, authnum++) if (au->client)
   {
   const uschar * list = names;
-  int sep = ' ';
-  uschar name[32];
-
-  while (string_nextinlist(&list, &sep, name, sizeof(name)))
-    if (strcmpic(au->public_name, name) == 0)
+  uschar * s;
+  for (int sep = ' '; s = string_nextinlist(&list, &sep, NULL, 0); )
+    if (strcmpic(au->public_name, s) == 0)
       { authbits |= BIT(authnum); break; }
   }
 
diff --git a/src/src/verify.c b/src/src/verify.c
index 4b584c05e..cd9df1f71 100644
--- a/src/src/verify.c
+++ b/src/src/verify.c
@@ -2260,7 +2260,7 @@ for (header_line * h = header_list; h && yield == OK; h = h->next)
 
   colon = Ustrchr(h->text, ':');
   s = colon + 1;
-  while (isspace(*s)) s++;
+  Uskip_whitespace(&s);
 
   /* Loop for multiple addresses in the header, enabling group syntax. Note
   that we have to reset this after the header has been scanned. */
@@ -2339,7 +2339,7 @@ for (header_line * h = header_list; h && yield == OK; h = h->next)
     /* Advance to the next address */
 
     s = ss + (terminator ? 1 : 0);
-    while (isspace(*s)) s++;
+    Uskip_whitespace(&s);
     }   /* Next address */
 
   f.parse_allow_group = FALSE;
@@ -3383,11 +3383,13 @@ dns_scan dnss;
 tree_node *t;
 dnsbl_cache_block *cb;
 int old_pool = store_pool;
-uschar query[256];         /* DNS domain max length */
+uschar * query;
+int qlen;
 
 /* Construct the specific query domainname */
 
-if (!string_format(query, sizeof(query), "%s.%s", prepend, domain))
+query = string_sprintf("%s.%s", prepend, domain);
+if ((qlen = Ustrlen(query)) >= 256)
   {
   log_write(0, LOG_MAIN|LOG_PANIC, "dnslist query is too long "
     "(ignored): %s...", query);
@@ -3422,7 +3424,7 @@ else
 
   else
     {	/* Set up a tree entry to cache the lookup */
-    t = store_get(sizeof(tree_node) + Ustrlen(query), is_tainted(query));
+    t = store_get(sizeof(tree_node) + qlen + 1 + 1, is_tainted(query));
     Ustrcpy(t->name, query);
     t->data.ptr = cb = store_get(sizeof(dnsbl_cache_block), FALSE);
     (void)tree_insertnode(&dnsbl_cache, t);
@@ -3529,7 +3531,6 @@ if (cb->rc == DNS_SUCCEED)
     for (da = cb->rhs; da; da = da->next)
       {
       int ipsep = ',';
-      uschar ip[46];
       const uschar *ptr = iplist;
       uschar *res;
 
@@ -3537,8 +3538,8 @@ if (cb->rc == DNS_SUCCEED)
 
       if (!bitmask)
 	{
-        while ((res = string_nextinlist(&ptr, &ipsep, ip, sizeof(ip))))
-          if (Ustrcmp(CS da->address, ip) == 0)
+        while ((res = string_nextinlist(&ptr, &ipsep, NULL, 0)))
+          if (Ustrcmp(CS da->address, res) == 0)
 	    break;
 	}
 
@@ -3560,9 +3561,9 @@ if (cb->rc == DNS_SUCCEED)
 
         /* Scan the returned addresses, skipping any that are IPv6 */
 
-        while ((res = string_nextinlist(&ptr, &ipsep, ip, sizeof(ip))))
+        while ((res = string_nextinlist(&ptr, &ipsep, NULL, 0)))
           {
-          if (host_aton(ip, address) != 1) continue;
+          if (host_aton(res, address) != 1) continue;
           if ((address[0] & mask) == address[0]) break;
           }
         }
@@ -3732,7 +3733,6 @@ int sep = 0;
 int defer_return = FAIL;
 const uschar *list = *listptr;
 uschar *domain;
-uschar buffer[1024];
 uschar revadd[128];        /* Long enough for IPv6 address */
 
 /* Indicate that the inverted IP address is not yet set up */
@@ -3745,7 +3745,7 @@ dns_init(FALSE, FALSE, FALSE);	/*XXX dnssec? */
 
 /* Loop through all the domains supplied, until something matches */
 
-while ((domain = string_nextinlist(&list, &sep, buffer, sizeof(buffer))))
+while ((domain = string_nextinlist(&list, &sep, NULL, 0)))
   {
   int rc;
   BOOL bitmask = FALSE;
diff --git a/test/confs/0027 b/test/confs/0027
index 19bdaa0b6..c2d0f01be 100644
--- a/test/confs/0027
+++ b/test/confs/0027
@@ -39,7 +39,7 @@ data3:
 acl_rcpt:
   warn	 set acl_m_1 = ${acl {data}}
   accept endpass
-         acl = ${tr{$local_part}{:}{\n}}
+         acl = ${bless:${tr{$local_part}{:}{\n}}}
   deny   message = this message should not occur
 
 
diff --git a/test/confs/0029 b/test/confs/0029
index 09e77963f..342e6a5ec 100644
--- a/test/confs/0029
+++ b/test/confs/0029
@@ -16,8 +16,8 @@ begin acl
 check_rcpt:
   require  verify = sender
            verify = sender=\
-                    ${if eq {${domain:$sender_address}}{test.ex}\
-                    {${local_part:$sender_address}@abc.test.ex}\
+                    ${if eq {$sender_address_domain}{test.ex}\
+                    {$sender_address_local_part@abc.test.ex}\
                     {$sender_address}}
   accept
 
diff --git a/test/confs/0251 b/test/confs/0251
index ea6b78f5e..9c951528c 100644
--- a/test/confs/0251
+++ b/test/confs/0251
@@ -39,7 +39,7 @@ exeter_listr:
   require_files = DIR/aux-fixed/TESTNUM.restrict.${local_part}
   retry_use_local_part
   senders = ${if exists{DIR/aux-fixed/TESTNUM.restrict.${local_part}} \
-    {DIR/aux-fixed/TESTNUM.restrict.${local_part}}{zzzz}}
+    {${bless:DIR/aux-fixed/TESTNUM.restrict.${local_part}}}{zzzz}}
   syntax_errors_to = ${local_part}-request@test.ex
 
 exeter_listf:
diff --git a/test/confs/0306 b/test/confs/0306
index 779e155fc..b3c18f4fa 100644
--- a/test/confs/0306
+++ b/test/confs/0306
@@ -33,7 +33,7 @@ r2:
   driver = redirect
   domains = lists.test.ex
   senders = ${if exists {DIR/aux-fixed/TESTNUM/$local_part}\
-             {lsearch;DIR/aux-fixed/TESTNUM/$local_part}{*}}
+             {lsearch;${bless:DIR/aux-fixed/TESTNUM/$local_part}}{*}}
   file = DIR/aux-fixed/TESTNUM/${bless:$local_part}
   forbid_pipe
   forbid_file
diff --git a/test/confs/0307 b/test/confs/0307
index 1f61ca3cb..81857eca4 100644
--- a/test/confs/0307
+++ b/test/confs/0307
@@ -22,7 +22,7 @@ r1:
   senders = ${if eq {$local_part_suffix}{-request}{*}\
             {\
             ${if exists {DIR/aux-fixed/TESTNUM/$local_part}\
-             {lsearch;DIR/aux-fixed/TESTNUM/$local_part}{*}}\
+             {lsearch;${bless:DIR/aux-fixed/TESTNUM/$local_part}}{*}}\
             }}
   file = DIR/aux-fixed/TESTNUM/${bless:$local_part$local_part_suffix}
   forbid_pipe
diff --git a/test/stderr/0023 b/test/stderr/0023
index 8111c9fd1..ddf364bcb 100644
--- a/test/stderr/0023
+++ b/test/stderr/0023
@@ -1254,7 +1254,7 @@ LOG: H=[30.30.30.30] F=<a@13.12.11.V4NET.rbl> rejected RCPT <x@y>: domain=test.e
 >>> check dnslists = test.ex/$sender_address_domain+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+END
 >>>                = test.ex/y+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+END
 >>> dnslists check: test.ex/y+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+END
-LOG: dnslist query is too long (ignored): y+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+...
+LOG: dnslist query is too long (ignored): y+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+extra+END.test.ex...
 >>> deny: condition test failed in ACL "acl_31_31_31"
 >>> processing "accept" (TESTSUITE/test-config 168)
 >>> accept: condition test succeeded in ACL "acl_31_31_31"