Keep router-variables separate on addrs, to avoid taint contamination
authorJeremy Harris <jgh146exb@wizmail.org>
Thu, 11 Jul 2019 16:12:26 +0000 (17:12 +0100)
committerJeremy Harris <jgh146exb@wizmail.org>
Thu, 11 Jul 2019 17:22:51 +0000 (18:22 +0100)
12 files changed:
doc/doc-docbook/spec.xfpt
src/src/deliver.c
src/src/functions.h
src/src/globals.c
src/src/route.c
src/src/routers/queryprogram.c
src/src/routers/redirect.c
src/src/structs.h
src/src/tree.c
src/src/verify.c
test/confs/0620
test/mail/0620.b

index a073730c61e99427d4b613128c5244fe206c4c5c..5463cc1a52b8529389630f138da74b1bfc7bdde8 100644 (file)
@@ -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)
index 7b794720fecf47536eb6458eeb46669104009ab6..a597c9a886c088be371dc7a337a8ef81275c36e5 100644 (file)
@@ -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 */
 
index 3fc27cb5adb20af703dd6196fdf793bfada1a1ef..806ba755dc73e439be79bcd76bc09fd7aebc6cbe 100644 (file)
@@ -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 *);
index a7b0234b92123b0df057f37fc5620abfac8d2559..742584ed1d65b62c19a1af92bf46cd1adde94142 100644 (file)
@@ -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
index 74466733e0cab1dc800d0243e088b6c5b60fa144..0817a4eda62a784ba66229de589339ba68442714 100644 (file)
@@ -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
index 02ada2950ee1dd682f795c84940cbd7f61d78498..45321338701da05652cc97081d9c467633aff8c3 100644 (file)
@@ -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;
index 920a74a140b2139df6875862344930f7685e6df2..09f15d035bcb124d0a155e125c7b5066c07d6a8e 100644 (file)
@@ -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;
index 1925c49323d99b352c0e1986c1aa9dcf38f834d8..c40750045fe4d9999f958e25c552fb243eb043a3 100644 (file)
@@ -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;
 
 
index 3b6c3603b0f0770ff5fc0d761f9dced6c757c762..ff792bb6e293ff303412c25a776d37b54c556369 100644 (file)
@@ -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 */
index bf91a838873d32c0432d3f2a3fb861e4e1ae1a50..25a4a0ca745d4611f0204c2c0dfdc348f6f72720 100644 (file)
@@ -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 */
 
index b1f48c40ee2b136a5f7f44236b06dc9887e367a7..61f577417eb49e5a856e5ff88ad31f0c5d63128e 100644 (file)
@@ -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
 
 
index 11db13d2391d9fe0a375986741d7a4e4aad897dc..a30deb5581fc2ed5fda8cacbc8b0c52c20013737 100644 (file)
@@ -8,6 +8,6 @@ Message-Id: <E10HmaX-0005vi-00@the.local.host.name>
 From: CALLER_NAME <CALLER@test.ex>
 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