From: Jeremy Harris Date: Thu, 11 Jul 2019 16:12:26 +0000 (+0100) Subject: Keep router-variables separate on addrs, to avoid taint contamination X-Git-Url: https://git.exim.org/users/jgh/exim.git/commitdiff_plain/b4f579d134197249b448cb5d8abf801ba4c729bb Keep router-variables separate on addrs, to avoid taint contamination --- diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index a073730c6..5463cc1a5 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -19007,14 +19007,20 @@ matters. .new -.option set routers string unset +.option set routers "string list" unset .cindex router variables -This option may be used multiple times on a router. -Each string given must be of the form $"name = value"$ +This option may be used multiple times on a router; +because of this the list aspect is mostly irrelevant. +The list separator is a colon but can be changed in the +usual way. + +Each list-element given must be of the form $"name = value"$ and the names used must start with the string &"r_"&. -Strings are accumulated for each router which is run. +Values containing colons should either have them doubled, or +the entire list should be prefixed with a list-separator change. When a router runs, the strings are evaluated in order, -to create variables. +to create variables which are added to the set associated with +the address. The variable is set with the expansion of the value. The variables can be used by the router options (not including any preconditions) diff --git a/src/src/deliver.c b/src/src/deliver.c index 7b794720f..a597c9a88 100644 --- a/src/src/deliver.c +++ b/src/src/deliver.c @@ -155,47 +155,6 @@ return addr; -/************************************************/ -/* Set router-assigned variables, forgetting any previous. -Return FALSE on failure */ - -static BOOL -set_router_vars(gstring * g_varlist) -{ -const uschar * varlist; -int sep = 0; - -router_var = NULL; -if (!g_varlist) return TRUE; -varlist = CUS string_from_gstring(g_varlist); - -/* Walk the varlist, creating variables */ - -for (uschar * ele; (ele = string_nextinlist(&varlist, &sep, NULL, 0)); ) - { - const uschar * assignment = ele; - int esep = '='; - uschar * name = string_nextinlist(&assignment, &esep, NULL, 0); - tree_node * node, ** root = &router_var; - - /* Variable name must exist and start "r_". */ - - if (!name || name[0] != 'r' || name[1] != '_' || !name[2]) - return FALSE; - name += 2; - - if (!(node = tree_search(*root, name))) - { - node = store_get(sizeof(tree_node) + Ustrlen(name)); - Ustrcpy(node->name, name); - (void)tree_insertnode(root, node); - } - node->data.ptr = US assignment; - } -return TRUE; -} - - /************************************************* * Set expansion values for an address * *************************************************/ @@ -239,7 +198,7 @@ deliver_recipients = addr; deliver_address_data = addr->prop.address_data; deliver_domain_data = addr->prop.domain_data; deliver_localpart_data = addr->prop.localpart_data; -set_router_vars(addr->prop.set); /*XXX failure cases? */ +router_var = addr->prop.variables; /* These may be unset for multiple addresses */ diff --git a/src/src/functions.h b/src/src/functions.h index 3fc27cb5a..806ba755d 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -551,6 +551,7 @@ extern BOOL transport_write_message(transport_ctx *, int); extern void tree_add_duplicate(uschar *, address_item *); extern void tree_add_nonrecipient(uschar *); extern void tree_add_unusable(host_item *); +extern void tree_dup(tree_node **, tree_node *); extern int tree_insertnode(tree_node **, tree_node *); extern tree_node *tree_search(tree_node *, const uschar *); extern void tree_write(tree_node *, FILE *); diff --git a/src/src/globals.c b/src/src/globals.c index a7b0234b9..742584ed1 100644 --- a/src/src/globals.c +++ b/src/src/globals.c @@ -585,7 +585,7 @@ address_item address_defaults = { .errors_address = NULL, .extra_headers = NULL, .remove_headers = NULL, - .set = NULL, + .variables = NULL, #ifdef EXPERIMENTAL_SRS .srs_sender = NULL, #endif diff --git a/src/src/route.c b/src/src/route.c index 74466733e..0817a4eda 100644 --- a/src/src/route.c +++ b/src/src/route.c @@ -1363,7 +1363,8 @@ new->prop.errors_address = parent->prop.errors_address; new->prop.ignore_error = addr->prop.ignore_error; new->prop.address_data = addr->prop.address_data; -new->prop.set = addr->prop.set; +new->prop.variables = NULL; +tree_dup((tree_node **)&new->prop.variables, addr->prop.variables); new->dsn_flags = addr->dsn_flags; new->dsn_orcpt = addr->dsn_orcpt; @@ -1406,6 +1407,79 @@ if (addr->transport && tree_search(tree_nonrecipients, addr->unique)) +/************************************************/ +/* Add router-assigned variables +Return OK/DEFER/FAIL/PASS */ + +static int +set_router_vars(address_item * addr, const router_instance * r) +{ +const uschar * varlist = r->set; +tree_node ** root = (tree_node **) &addr->prop.variables; +int sep = 0; + +if (!varlist) return OK; + +/* Walk the varlist, creating variables */ + +for (uschar * ele; (ele = string_nextinlist(&varlist, &sep, NULL, 0)); ) + { + const uschar * assignment = ele; + int esep = '='; + uschar * name = string_nextinlist(&assignment, &esep, NULL, 0); + uschar * val; + tree_node * node; + + /* Variable name must exist and start "r_". */ + + if (!name || name[0] != 'r' || name[1] != '_' || !name[2]) + return FAIL; + name += 2; + + if (!(val = expand_string(US assignment))) + if (f.expand_string_forcedfail) + { + int yield; + BOOL more; + DEBUG(D_route) debug_printf("forced failure in expansion of \"%s\" " + "(router variable): decline action taken\n", ele); + + /* Expand "more" if necessary; DEFER => an expansion failed */ + + yield = exp_bool(addr, US"router", r->name, D_route, + US"more", r->more, r->expand_more, &more); + if (yield != OK) return yield; + + if (!more) + { + DEBUG(D_route) + debug_printf("\"more\"=false: skipping remaining routers\n"); + router_name = NULL; + r = NULL; + return FAIL; + } + return PASS; + } + else + { + addr->message = string_sprintf("expansion of \"%s\" failed " + "in %s router: %s", ele, r->name, expand_string_message); + return DEFER; + } + + if (!(node = tree_search(*root, name))) + { + node = store_get(sizeof(tree_node) + Ustrlen(name)); + Ustrcpy(node->name, name); + (void)tree_insertnode(root, node); + } + node->data.ptr = US val; + DEBUG(D_route) debug_printf("set r_%s = '%s'\n", name, val); + } +return OK; +} + + /************************************************* * Route one address * *************************************************/ @@ -1605,11 +1679,19 @@ for (r = addr->start_router ? addr->start_router : routers; r; r = nextr) search_error_message = NULL; - /* Add any variable-settings that are on the router, to the list on the + /* Add any variable-settings that are on the router, to the set on the addr. Expansion is done here and not later when the addr is used. There may be multiple settings, gathered during readconf; this code gathers them during - router traversal. */ + router traversal. On the addr string they are held as a variable tree, so + as to maintain the post-expansion taints separate. */ + if ((yield = set_router_vars(addr, r)) != OK) + if (yield == PASS) + continue; /* with next router */ + else + goto ROUTE_EXIT; + +#ifdef notdef if (r->set) { const uschar * list = r->set; @@ -1650,6 +1732,7 @@ for (r = addr->start_router ? addr->start_router : routers; r; r = nextr) addr->prop.set = string_append_listele(addr->prop.set, ':', ee); } } +#endif /* Finally, expand the address_data field in the router. Forced failure behaves as if the router declined. Any other failure is more serious. On diff --git a/src/src/routers/queryprogram.c b/src/src/routers/queryprogram.c index 02ada2950..453213387 100644 --- a/src/src/routers/queryprogram.c +++ b/src/src/routers/queryprogram.c @@ -232,7 +232,7 @@ errors address and extra header stuff. */ bzero(&addr_prop, sizeof(addr_prop)); addr_prop.address_data = deliver_address_data; -addr_prop.set = addr->prop.set; +tree_dup((tree_node **)&addr_prop.variables, addr->prop.variables); rc = rf_get_errors_address(addr, rblock, verify, &addr_prop.errors_address); if (rc != OK) return rc; diff --git a/src/src/routers/redirect.c b/src/src/routers/redirect.c index 920a74a14..09f15d035 100644 --- a/src/src/routers/redirect.c +++ b/src/src/routers/redirect.c @@ -563,7 +563,8 @@ addr_prop.localpart_data = deliver_localpart_data; addr_prop.errors_address = NULL; addr_prop.extra_headers = NULL; addr_prop.remove_headers = NULL; -addr_prop.set = addr->prop.set; +addr_prop.variables = NULL; +tree_dup((tree_node **)&addr_prop.variables, addr->prop.variables); #ifdef EXPERIMENTAL_SRS addr_prop.srs_sender = NULL; diff --git a/src/src/structs.h b/src/src/structs.h index 1925c4932..c40750045 100644 --- a/src/src/structs.h +++ b/src/src/structs.h @@ -511,17 +511,17 @@ typedef struct address_item_propagated { uschar *errors_address; /* where to send errors (NULL => sender) */ header_line *extra_headers; /* additional headers */ uschar *remove_headers; /* list of those to remove */ - gstring *set; /* list of variables, with values */ + void *variables; /* router-vasriables */ - #ifdef EXPERIMENTAL_SRS +#ifdef EXPERIMENTAL_SRS uschar *srs_sender; /* Change return path when delivering */ - #endif +#endif BOOL ignore_error:1; /* ignore delivery error */ - #ifdef SUPPORT_I18N +#ifdef SUPPORT_I18N BOOL utf8_msg:1; /* requires SMTPUTF8 processing */ BOOL utf8_downcvt:1; /* mandatory downconvert on delivery */ BOOL utf8_downcvt_maybe:1; /* optional downconvert on delivery */ - #endif +#endif } address_item_propagated; diff --git a/src/src/tree.c b/src/src/tree.c index 3b6c3603b..ff792bb6e 100644 --- a/src/src/tree.c +++ b/src/src/tree.c @@ -362,4 +362,27 @@ tree_walk(p->right, f, ctx); } + +/* Add a node to a tree, ignoring possibility of duplicates */ + +static void +tree_add_var(uschar * name, uschar * val, void * ctx) +{ +tree_node ** root = ctx; +tree_node * node = store_get(sizeof(tree_node) + Ustrlen(name)); +Ustrcpy(node->name, name); +node->data.ptr = val; +(void) tree_insertnode(root, node); +} + +/* Duplicate a tree */ + +void +tree_dup(tree_node ** dstp, tree_node * src) +{ +tree_walk(src, tree_add_var, dstp); +} + + + /* End of tree.c */ diff --git a/src/src/verify.c b/src/src/verify.c index bf91a8388..25a4a0ca7 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -1529,7 +1529,8 @@ if (addr != vaddr) vaddr->basic_errno = addr->basic_errno; vaddr->more_errno = addr->more_errno; vaddr->prop.address_data = addr->prop.address_data; - vaddr->prop.set = addr->prop.set; + vaddr->prop.variables = NULL; + tree_dup((tree_node **)&vaddr->prop.variables, addr->prop.variables); copyflag(vaddr, addr, af_pass_message); } return yield; @@ -2090,7 +2091,8 @@ while (addr_new) of $address_data to be that of the child */ vaddr->prop.address_data = addr->prop.address_data; - vaddr->prop.set = addr->prop.set; + vaddr->prop.variables = NULL; + tree_dup((tree_node **)&vaddr->prop.variables, addr->prop.variables); /* If stopped because more than one new address, cannot cutthrough */ diff --git a/test/confs/0620 b/test/confs/0620 index b1f48c40e..61f577417 100644 --- a/test/confs/0620 +++ b/test/confs/0620 @@ -8,6 +8,12 @@ domainlist local_domains = test.ex qualify_domain = test.ex +acl_not_smtp = not_smtp + +begin acl + +not_smtp: + accept log_message = rcpt <$recipients> l <$local_part> # ----- Routers ----- @@ -17,13 +23,13 @@ alias: driver = redirect debug_print = DEBUG: $r_r1 $r_r2 data = b - set = r_r1 = $local_part + set = <; r_r1 = $local_part aaa:bbb bar=baz user: driver = accept debug_print = DEBUG: $r_r1 $r_r2 set = r_r1 = $local_part - set = r_r2 = $local_part + set = <; r_r2 = $local_part 2a00:1940:100::ff:0:1 foo=bar transport = local_delivery diff --git a/test/mail/0620.b b/test/mail/0620.b index 11db13d23..a30deb558 100644 --- a/test/mail/0620.b +++ b/test/mail/0620.b @@ -8,6 +8,6 @@ Message-Id: From: CALLER_NAME Date: Tue, 2 Mar 1999 09:44:33 +0000 X-r1: b -X-r2: b +X-r2: b 2a00:1940:100::ff:0:1 foo=bar