Gitweb:
https://git.exim.org/exim.git/commitdiff/b4f579d134197249b448cb5d8abf801ba4c729bb
Commit: b4f579d134197249b448cb5d8abf801ba4c729bb
Parent: df04890c398664e60aedbddb8d646cb1d353383f
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Thu Jul 11 17:12:26 2019 +0100
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Thu Jul 11 18:22:51 2019 +0100
Keep router-variables separate on addrs, to avoid taint contamination
---
doc/doc-docbook/spec.xfpt | 16 +++++---
src/src/deliver.c | 43 +-------------------
src/src/functions.h | 1 +
src/src/globals.c | 2 +-
src/src/route.c | 89 ++++++++++++++++++++++++++++++++++++++++--
src/src/routers/queryprogram.c | 2 +-
src/src/routers/redirect.c | 3 +-
src/src/structs.h | 10 ++---
src/src/tree.c | 23 +++++++++++
src/src/verify.c | 6 ++-
test/confs/0620 | 10 ++++-
test/mail/0620.b | 2 +-
12 files changed, 144 insertions(+), 63 deletions(-)
diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index a073730..5463cc1 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 7b79472..a597c9a 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 3fc27cb..806ba75 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 a7b0234..742584e 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 7446673..0817a4e 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 02ada29..4532133 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 920a74a..09f15d0 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 1925c49..c407500 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 3b6c360..ff792bb 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 bf91a83..25a4a0c 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 b1f48c4..61f5774 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 11db13d..a30deb5 100644
--- a/test/mail/0620.b
+++ b/test/mail/0620.b
@@ -8,6 +8,6 @@ Message-Id: <E10HmaX-0005vi-00@???>
From: CALLER_NAME <CALLER@???>
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
