Convert more cases of list-walking to use self-assigned memory for the list-item

diff --git a/src/src/auths/plaintext.c b/src/src/auths/plaintext.c
index 778e6c2c43a7a328031b24b71249e216a4609f19..ab703daec6aae645fb959cd9c3c7f325b7be25a4 100644 (file)
--- a/src/src/auths/plaintext.c
+++ b/src/src/auths/plaintext.c
@@ -109,7 +109,7 @@ already been provided as part of the AUTH command. For the rest, send them
 out as prompts, and get a data item back. If the data item is "*", abandon the
 authentication attempt. Otherwise, split it into items as above. */
 
-while (  (s = string_nextinlist(&prompts, &sep, big_buffer, big_buffer_size))
+while (  (s = string_nextinlist(&prompts, &sep, NULL, 0))
       && expand_nmax < EXPAND_MAXN)
   if (number++ > expand_nmax)
     if ((rc = auth_prompt(CUS s)) != OK)

diff --git a/src/src/bmi_spam.c b/src/src/bmi_spam.c
index 6651de5ad7d195a9d5ee0212fa976750bf4a3ff5..6972bc3a7957bdac5f09028b0cd59448734974c6 100644 (file)
--- a/src/src/bmi_spam.c
+++ b/src/src/bmi_spam.c
@@ -448,9 +448,11 @@ int bmi_check_rule(uschar *base64_verdict, uschar *option_list) {
   }
 
   /* loop through numbers */
+  /* option_list doesn't seem to be expanded so cannot be tainted.  If it ever is we
+  will trap here */
   rule_ptr = option_list;
   while ((rule_num = string_nextinlist(&rule_ptr, &sep,
-                                       rule_buffer, 32)) != NULL) {
+                                       rule_buffer, sizeof(rule_buffer)))) {
     int rule_int = -1;
 
     /* try to translate to int */

diff --git a/src/src/daemon.c b/src/src/daemon.c
index 83131fa1d94330eb439e779cd63bdbf3d16f5d5c..21807f64f3397a09c6b37fd5c343264ba4cde98d 100644 (file)
--- a/src/src/daemon.c
+++ b/src/src/daemon.c
@@ -1426,6 +1426,7 @@ if (f.daemon_listen && !f.inetd_wait_mode)
 
   list = tls_in.on_connect_ports;
   sep = 0;
+  /* the list isn't expanded so cannot be tainted.  If it ever is we will trap here */
   while ((s = string_nextinlist(&list, &sep, big_buffer, big_buffer_size)))
     if (!isdigit(*s))
       {

diff --git a/src/src/deliver.c b/src/src/deliver.c
index ec5990cc1b80b90c44999ebc508617bb13d33ba4..cba8651e9971016bae12f5714adcfdbc230fc457 100644 (file)
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -3184,6 +3184,7 @@ const uschar *listptr = remote_sort_domains;
 uschar *pattern;
 uschar patbuf[256];
 
+/*XXX The list is used before expansion. Not sure how that ties up with the docs */
 while (  *aptr
       && (pattern = string_nextinlist(&listptr, &sep, patbuf, sizeof(patbuf)))
       )

diff --git a/src/src/exim.c b/src/src/exim.c
index 60a44bb09c8025621ed382fa19921e69cfdbda34..8f33bde2642b0a873bfdc479d83432b04c6c1e8d 100644 (file)
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -2373,6 +2373,7 @@ on the second character (the one after '-'), to save some effort. */
       int len = Ustrlen(ALT_CONFIG_PREFIX);
       const uschar *list = argrest;
       uschar *filename;
+      /* The argv is untainted, so big_buffer (also untainted) is ok to use */
       while((filename = string_nextinlist(&list, &sep, big_buffer,
              big_buffer_size)))
         if (  (  Ustrlen(filename) < len

diff --git a/src/src/expand.c b/src/src/expand.c
index 839821ef7e4ebbff6b44a4ad74d1463ec1cb0f13..e8e97dab8c4c5b9dd6944401360ab847fc581f94 100644 (file)
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -7251,7 +7251,9 @@ while (*s)
        uschar * item;
        uschar * suffix = US"";
        BOOL needsep = FALSE;
-       uschar buffer[256];
+#define LISTNAMED_BUF_SIZE 256
+       uschar b[LISTNAMED_BUF_SIZE];
+       uschar * buffer = b;
 
        if (*sub == '+') sub++;
        if (!arg)               /* no-argument version */
@@ -7286,7 +7288,11 @@ while (*s)
 
        list = ((namedlist_block *)(t->data.ptr))->string;
 
-       while ((item = string_nextinlist(&list, &sep, buffer, sizeof(buffer))))
+       /* The list could be quite long so we (re)use a buffer for each element
+       rather than getting each in new memory */
+
+       if (is_tainted(list)) buffer = store_get(LISTNAMED_BUF_SIZE, TRUE);
+       while ((item = string_nextinlist(&list, &sep, buffer, LISTNAMED_BUF_SIZE)))
          {
          uschar * buf = US" : ";
          if (needsep)

diff --git a/src/src/filter.c b/src/src/filter.c
index 59c08f882ea224bb8d125fc30bbfc7860c3c254b..3897ae0f997232f27a372489f6175b375bf2b694 100644 (file)
--- a/src/src/filter.c
+++ b/src/src/filter.c
@@ -2007,11 +2007,9 @@ while (commands)
        else if (subtype == FALSE)
          {
          int sep = 0;
-         uschar *ss;
-         const uschar *list = s;
-         uschar buffer[128];
-         while ((ss = string_nextinlist(&list, &sep, buffer, sizeof(buffer)))
-                != NULL)
+         const uschar * list = s;
+
+         for (uschar * ss; ss = string_nextinlist(&list, &sep, NULL, 0); )
            header_remove(0, ss);
          }
 

diff --git a/src/src/host.c b/src/src/host.c
index 4e1cb8a4578e1fb0de4f39e08a14a8289e9e825a..a31c09b659f15bee1740e210d49dd004634db492 100644 (file)
--- a/src/src/host.c
+++ b/src/src/host.c
@@ -1232,14 +1232,11 @@ BOOL
 host_is_tls_on_connect_port(int port)
 {
 int sep = 0;
-uschar buffer[32];
-const uschar *list = tls_in.on_connect_ports;
-uschar *s;
-uschar *end;
+const uschar * list = tls_in.on_connect_ports;
 
 if (tls_in.on_connect) return TRUE;
 
-while ((s = string_nextinlist(&list, &sep, buffer, sizeof(buffer))))
+for (uschar * s, * end; s = string_nextinlist(&list, &sep, NULL, 0); )
   if (Ustrtol(s, &end, 10) == port)
     return TRUE;
 

diff --git a/src/src/lookups/ibase.c b/src/src/lookups/ibase.c
index 4789b6cb7a54c563ea97580cc2ab74e71f12e9e6..72a27c0109388c5e5a0b34391730ca18a58309f2 100644 (file)
--- a/src/src/lookups/ibase.c
+++ b/src/src/lookups/ibase.c
@@ -457,11 +457,10 @@ ibase_find(void * handle, const uschar * filename, uschar * query, int length,
 int sep = 0;
 uschar *server;
 uschar *list = ibase_servers;
-uschar buffer[512];
 
 DEBUG(D_lookup) debug_printf_indent("Interbase query: %s\n", query);
 
-while ((server = string_nextinlist(&list, &sep, buffer, sizeof(buffer))))
+while ((server = string_nextinlist(&list, &sep, NULL, 0)))
   {
   BOOL defer_break = FALSE;
   int rc = perform_ibase_search(query, server, result, errmsg, &defer_break);

diff --git a/src/src/lookups/oracle.c b/src/src/lookups/oracle.c
index 0712c89e3f84a90c06cbec93109a8d4c4b9e3fc4..136971590c8dec1ff7280ce964c63abeb18ea4f5 100644 (file)
--- a/src/src/lookups/oracle.c
+++ b/src/src/lookups/oracle.c
@@ -510,13 +510,12 @@ oracle_find(void * handle, const uschar * filename, uschar * query, int length,
 int sep = 0;
 uschar *server;
 uschar *list = oracle_servers;
-uschar buffer[512];
 
 do_cache = do_cache;   /* Placate picky compilers */
 
 DEBUG(D_lookup) debug_printf_indent("ORACLE query: %s\n", query);
 
-while ((server = string_nextinlist(&list, &sep, buffer, sizeof(buffer))))
+while ((server = string_nextinlist(&list, &sep, NULL, 0)))
   {
   BOOL defer_break;
   int rc = perform_oracle_search(query, server, result, errmsg, &defer_break);

diff --git a/src/src/match.c b/src/src/match.c
index bf8cb3b98f8331700c187a9430ad8ebd9034b568..597b633fee8d932df4165aeaee7ccc14f06b9db2 100644 (file)
--- a/src/src/match.c
+++ b/src/src/match.c
@@ -1061,7 +1061,6 @@ if (pattern[0] == '@' && pattern[1] == '@')
   {
   int watchdog = 50;
   uschar *list, *ss;
-  uschar buffer[1024];
 
   if (sdomain == subject + 1 && *subject == '*') return FAIL;
 
@@ -1092,7 +1091,7 @@ if (pattern[0] == '@' && pattern[1] == '@')
     /* Look up the local parts provided by the list; negation is permitted.
     If a local part has to begin with !, a regex can be used. */
 
-    while ((ss = string_nextinlist(CUSS &list, &sep, buffer, sizeof(buffer))))
+    while ((ss = string_nextinlist(CUSS &list, &sep, NULL, 0)))
       {
       int local_yield;
 

diff --git a/src/src/moan.c b/src/src/moan.c
index 4e7fbd607bc98f92d73837d3a99a788f85f5e848..bf5483ce38b620c4410ce59794ef9737fc94d00b 100644 (file)
--- a/src/src/moan.c
+++ b/src/src/moan.c
@@ -718,7 +718,6 @@ moan_check_errorcopy(uschar *recipient)
 uschar *item, *localpart, *domain;
 const uschar *listptr = errors_copy;
 uschar *yield = NULL;
-uschar buffer[256];
 int sep = 0;
 int llen;
 
@@ -734,7 +733,7 @@ llen = domain++ - recipient;
 
 /* Scan through the configured items */
 
-while ((item = string_nextinlist(&listptr, &sep, buffer, sizeof(buffer))))
+while ((item = string_nextinlist(&listptr, &sep, NULL, 0)))
   {
   const uschar *newaddress = item;
   const uschar *pattern = string_dequote(&newaddress);

diff --git a/src/src/readconf.c b/src/src/readconf.c
index 0b78a88b9179b6f51eae0f560c3400c71e20b414..7f808def84c31cc1881e1108dac117fc593bfdf2 100644 (file)
--- a/src/src/readconf.c
+++ b/src/src/readconf.c
@@ -1848,6 +1848,7 @@ switch (type)
         flagptr = (int *)ol3->v.value;
         }
 
+      /* This will trap if sptr is tainted. Not sure if that can happen */
       while ((p = string_nextinlist(CUSS &sptr, &sep, big_buffer, BIG_BUFFER_SIZE)))
         {
         rewrite_rule *next = readconf_one_rewrite(p, flagptr, FALSE);
@@ -1992,6 +1993,7 @@ switch (type)
       while (count-- > 1)
         {
         int sep = 0;
+       /* If p is tainted we trap.  Not sure that can happen */
         (void)string_nextinlist(&p, &sep, big_buffer, BIG_BUFFER_SIZE);
         if (!route_finduser(big_buffer, NULL, &uid))
           log_write(0, LOG_PANIC_DIE|LOG_CONFIG_IN, "user %s was not found",
@@ -2033,6 +2035,7 @@ switch (type)
       while (count-- > 1)
         {
         int sep = 0;
+       /* If p is tainted we trap.  Not sure that can happen */
         (void)string_nextinlist(&p, &sep, big_buffer, BIG_BUFFER_SIZE);
         if (!route_findgroup(big_buffer, &gid))
           log_write(0, LOG_PANIC_DIE|LOG_CONFIG_IN, "group %s was not found",
@@ -3115,6 +3118,7 @@ const uschar *list = config_main_filelist;
 
 /* Loop through the possible file names */
 
+/* Should never be a tainted list */
 while((filename = string_nextinlist(&list, &sep, big_buffer, big_buffer_size)))
   {
 
@@ -3428,6 +3432,7 @@ if (*log_file_path)
       "\"%s\": %s", log_file_path, expand_string_message);
 
   ss = s;
+  /* should never be a tainted list */
   while ((sss = string_nextinlist(&ss, &sep, big_buffer, big_buffer_size)))
     {
     uschar *t;

diff --git a/src/src/receive.c b/src/src/receive.c
index 95c44c01ca064bcf3e0ba6f8fb0297f2a2d85762..00c431fc8934e688e93560e550f8c20468322a7d 100644 (file)
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -176,6 +176,7 @@ else
   empty item in a list. */
 
   if (*p == 0) p = US":";
+  /* should never be a tainted list */
   while ((path = string_nextinlist(&p, &sep, buffer, sizeof(buffer))))
     if (Ustrcmp(path, "syslog") != 0)
       break;
@@ -1228,9 +1229,8 @@ if (acl_removed_headers)
     const uschar * list = acl_removed_headers;
     int sep = ':';         /* This is specified as a colon-separated list */
     uschar *s;
-    uschar buffer[128];
 
-    while ((s = string_nextinlist(&list, &sep, buffer, sizeof(buffer))))
+    while ((s = string_nextinlist(&list, &sep, NULL, 0)))
       if (header_testname(h, s, Ustrlen(s), FALSE))
        {
        h->type = htype_old;

diff --git a/src/src/route.c b/src/src/route.c
index a5f5feeaf9c7097d503104374003993fb19caa7b..1d87b079a7b3c6a2ecd552092581a5c71618205d 100644 (file)
--- a/src/src/route.c
+++ b/src/src/route.c
@@ -607,14 +607,13 @@ gid_t gid = 0;            /* For picky compilers */
 BOOL ugid_set = FALSE;
 const uschar *listptr;
 uschar *check;
-uschar buffer[1024];
 
 if (!s) return OK;
 
 DEBUG(D_route) debug_printf("checking require_files\n");
 
 listptr = s;
-while ((check = string_nextinlist(&listptr, &sep, buffer, sizeof(buffer))))
+while ((check = string_nextinlist(&listptr, &sep, NULL, 0)))
   {
   int rc;
   int eacces_code = 0;

diff --git a/src/src/routers/dnslookup.c b/src/src/routers/dnslookup.c
index a836c61472cea601ebc004d7bcfa3085df00a4e4..41a5ad78d22df03fcb8c0634b8c5ff6298f4a52a 100644 (file)
--- a/src/src/routers/dnslookup.c
+++ b/src/src/routers/dnslookup.c
@@ -204,6 +204,7 @@ if (  ob->widen_domains
    && (verify != v_sender || !ob->rewrite_headers || addr->parent))
   {
   listptr = ob->widen_domains;
+  /* not expanded so should never be tainted */
   widen = string_nextinlist(&listptr, &widen_sep, widen_buffer,
     sizeof(widen_buffer));
 
@@ -233,6 +234,7 @@ for (;;)
   else if (widen)
     {
     h.name = string_sprintf("%s.%s", addr->domain, widen);
+    /* not expanded so should never be tainted */
     widen = string_nextinlist(&listptr, &widen_sep, widen_buffer,
       sizeof(widen_buffer));
     DEBUG(D_route) debug_printf("%s router widened %s to %s\n", rblock->name,

diff --git a/src/src/routers/iplookup.c b/src/src/routers/iplookup.c
index 4412c625ec60a4885fd598ccc5b59245b42db151..3035b8863bb7b99407a509257af41709484a5ad4 100644 (file)
--- a/src/src/routers/iplookup.c
+++ b/src/src/routers/iplookup.c
@@ -200,6 +200,7 @@ response it received. Initialization insists on the port being set and there
 being a host list. */
 
 listptr = ob->hosts;
+/* not expanded so should never be tainted */
 while ((hostname = string_nextinlist(&listptr, &sep, host_buffer,
        sizeof(host_buffer))))
   {

diff --git a/src/src/smtp_out.c b/src/src/smtp_out.c
index 911dd537e268becd78ac34e48a0be338b6fc0d55..0bf619795f4a4999141635eb98890c1e10c3356a 100644 (file)
--- a/src/src/smtp_out.c
+++ b/src/src/smtp_out.c
@@ -67,6 +67,7 @@ if (is_tainted(expint))
 Uskip_whitespace(&expint);
 if (!*expint) return TRUE;
 
+/* we just tested to ensure no taint, so big_buffer is ok */
 while ((iface = string_nextinlist(&expint, &sep, big_buffer,
           big_buffer_size)))
   {

diff --git a/src/src/srs.c b/src/src/srs.c
index 657cd1771d39d028c417c3a8692f6498a814c8d2..5cca182daf6a46537d92dc9f92c982dfca6715b6 100644 (file)
--- a/src/src/srs.c
+++ b/src/src/srs.c
@@ -50,6 +50,7 @@ if (srs == NULL)
   co = 0;
   if (srs_config != NULL)
     {
+    /* looks like list not expanded, so cannot be tainted */
     secret = string_nextinlist(&list, &co, secret_buf, SRS_MAX_SECRET_LENGTH);
 
     if ((sbufp = string_nextinlist(&list, &co, sbuf, sizeof(sbuf))))
@@ -72,6 +73,7 @@ if (srs == NULL)
   co = 0;
   list = srs_secrets;
   if (secret == NULL || *secret == '\0')
+    /* looks like list not expanded so cannot be tainted */
     if (!(secret = string_nextinlist(&list, &co, secret_buf, SRS_MAX_SECRET_LENGTH)))
       {
       log_write(0, LOG_MAIN | LOG_PANIC,
@@ -104,6 +106,7 @@ if (srs == NULL)
   srs_set_option(srs, SRS_OPTION_USEHASH, usehash);
 
   /* Extra secrets? */
+  /* looks like list not expanded so cannot be tainted */
   while((secret = string_nextinlist(&list, &co, secret_buf, SRS_MAX_SECRET_LENGTH)))
       srs_add_secret(srs, secret,
                Ustrlen(secret) > SRS_MAX_SECRET_LENGTH ? SRS_MAX_SECRET_LENGTH :  Ustrlen(secret));

diff --git a/src/src/string.c b/src/src/string.c
index 53ff0a8343bf240f2e19980994e02d02e26d5bb1..4726fbe4e30e0de5b7fa5256dd4c10a6199d1dbd 100644 (file)
--- a/src/src/string.c
+++ b/src/src/string.c
@@ -858,6 +858,9 @@ Arguments:
   separator  a pointer to the separator character in an int (see above)
   buffer     where to put a copy of the next string in the list; or
                NULL if the next string is returned in new memory
+            Note that if the list is tainted then a provided buffer must be
+            also (else we trap, with a message referencing the callsite).
+            If we do the allocation, taint is handled there.
   buflen     when buffer is not NULL, the size of buffer; otherwise ignored
 
 Returns:     pointer to buffer, containing the next substring,

diff --git a/src/src/transports/appendfile.c b/src/src/transports/appendfile.c
index 8ef1a564e741fe5408147f06af54861763ebd53c..93c89a4507a0ff945eec50b1f83a334d0e89468a 100644 (file)
--- a/src/src/transports/appendfile.c
+++ b/src/src/transports/appendfile.c
@@ -644,7 +644,8 @@ if (len == 0) return tblock;
 
 /* Search the formats for a match */
 
-while ((s = string_nextinlist(&format,&sep,big_buffer,big_buffer_size)))
+/* not expanded so cannot be tainted */
+while ((s = string_nextinlist(&format, &sep, big_buffer, big_buffer_size)))
   {
   int slen = Ustrlen(s);
   BOOL match = len >= slen && Ustrncmp(data, s, slen) == 0;