Taint: check on supplied buffer vs. list when extracting elements
authorJeremy Harris <jgh146exb@wizmail.org>
Sun, 5 Apr 2020 22:21:40 +0000 (23:21 +0100)
committerJeremy Harris <jgh146exb@wizmail.org>
Sun, 5 Apr 2020 22:27:18 +0000 (23:27 +0100)
15 files changed:
src/src/acl.c
src/src/daemon.c
src/src/functions.h
src/src/host.c
src/src/match.c
src/src/string.c
src/src/transports/pipe.c
src/src/transports/smtp.c
src/src/verify.c
test/confs/0027
test/confs/0029
test/confs/0251
test/confs/0306
test/confs/0307
test/stderr/0023

index 9ed0057..02251b1 100644 (file)
@@ -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;
index 3b21d27..6560da1 100644 (file)
@@ -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))
index efd039b..8206c48 100644 (file)
@@ -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);
index 3361d59..1426bff 100644 (file)
@@ -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;
index a0899bf..6da1d27 100644 (file)
@@ -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;
 
index 4ef2fee..80cf49f 100644 (file)
@@ -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);
index 6a7f150..27422bd 100644 (file)
@@ -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)
      {
index fc5bb78..5fb22bc 100644 (file)
@@ -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; }
   }
 
index 4b584c0..cd9df1f 100644 (file)
@@ -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;
index 19bdaa0..c2d0f01 100644 (file)
@@ -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
 
 
index 09e7796..342e6a5 100644 (file)
@@ -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
 
index ea6b78f..9c95152 100644 (file)
@@ -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:
index 779e155..b3c18f4 100644 (file)
@@ -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
index 1f61ca3..81857ec 100644 (file)
@@ -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
index 8111c9f..ddf364b 100644 (file)
@@ -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"