Gitweb:
https://git.exim.org/exim.git/commitdiff/53cc1417d804b27674f9e18fec09dee3badd080b
Commit: 53cc1417d804b27674f9e18fec09dee3badd080b
Parent: 942f0be6c2cd3ec8c39ca234a449561d9d3c1075
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Thu Dec 24 20:59:29 2020 +0000
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Thu Dec 24 20:59:29 2020 +0000
Convert more cases of list-walking to use self-assigned memory for the list-item
---
src/src/auths/plaintext.c | 2 +-
src/src/bmi_spam.c | 4 +++-
src/src/daemon.c | 1 +
src/src/deliver.c | 1 +
src/src/exim.c | 1 +
src/src/expand.c | 10 ++++++++--
src/src/filter.c | 8 +++-----
src/src/host.c | 7 ++-----
src/src/lookups/ibase.c | 3 +--
src/src/lookups/oracle.c | 3 +--
src/src/match.c | 3 +--
src/src/moan.c | 3 +--
src/src/readconf.c | 5 +++++
src/src/receive.c | 4 ++--
src/src/route.c | 3 +--
src/src/routers/dnslookup.c | 2 ++
src/src/routers/iplookup.c | 1 +
src/src/smtp_out.c | 1 +
src/src/srs.c | 3 +++
src/src/string.c | 3 +++
src/src/transports/appendfile.c | 3 ++-
21 files changed, 44 insertions(+), 27 deletions(-)
diff --git a/src/src/auths/plaintext.c b/src/src/auths/plaintext.c
index 778e6c2..ab703da 100644
--- 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 6651de5..6972bc3 100644
--- 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 83131fa..21807f6 100644
--- 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 ec5990c..cba8651 100644
--- 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 60a44bb..8f33bde 100644
--- 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 839821e..e8e97da 100644
--- 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 59c08f8..3897ae0 100644
--- 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 4e1cb8a..a31c09b 100644
--- 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 4789b6c..72a27c0 100644
--- 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 0712c89..1369715 100644
--- 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 bf8cb3b..597b633 100644
--- 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 4e7fbd6..bf5483c 100644
--- 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 0b78a88..7f808de 100644
--- 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 95c44c0..00c431f 100644
--- 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 a5f5fee..1d87b07 100644
--- 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 a836c61..41a5ad7 100644
--- 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 4412c62..3035b88 100644
--- 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 911dd53..0bf6197 100644
--- 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 657cd17..5cca182 100644
--- 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 53ff0a8..4726fbe 100644
--- 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 8ef1a56..93c89a4 100644
--- 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;
