Fix $recipients expansion when used within ${run...}. Bug 3013

Broken-by: cfe6acff2ddc

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index ecb4aadec59cef9eee4f84c60b2778a4a1727f32..efdc228b60f9adcef3842d8aba3161f08347defc 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -173,6 +173,9 @@ JH/32 Fix "tls_dhparam = none" under GnuTLS.  At least with 3.7.9 this gave
 JH/33 Fix free for live variable $value created by a ${run ...} expansion.
       Although not seen, this could have resulted in a SIGSEGV.
 
+JH/34 Bug 3013: Fix use of $recipients within arguments for ${run...}.
+      In 4.96 this would expand to empty.
+
 
 Exim version 4.96
 -----------------

diff --git a/src/src/deliver.c b/src/src/deliver.c
index bea38c5d16facdf301f52df377d3705b234cbbbc..52270368ee3b7fc4eaacd991a303529772e2ce0f 100644 (file)
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -2376,7 +2376,7 @@ if ((pid = exim_fork(US"delivery-local")) == 0)
       {
       ok = transport_set_up_command(&transport_filter_argv,
         tp->filter_command,
-        TRUE, PANIC, addr, FALSE, US"transport filter", NULL);
+        TSUC_EXPAND_ARGS, PANIC, addr, US"transport filter", NULL);
       transport_filter_timeout = tp->filter_timeout;
       }
     else transport_filter_argv = NULL;

diff --git a/src/src/expand.c b/src/src/expand.c
index e0c571ade62cf194490a739e3e25eb5389bbb4ce..259d463a447f55aec1675f9c76403fb90606a245 100644 (file)
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -5623,7 +5623,7 @@ while (*s)
       {
       FILE * f;
       const uschar * arg, ** argv;
-      BOOL late_expand = TRUE;
+      unsigned late_expand = TSUC_EXPAND_ARGS | TSUC_ALLOW_TAINTED_ARGS | TSUC_ALLOW_RECIPIENTS;
       uschar * save_value = lookup_value;
       int yesno;
 
@@ -5637,7 +5637,7 @@ while (*s)
 
       while (*s == ',')
        if (Ustrncmp(++s, "preexpand", 9) == 0)
-         { late_expand = FALSE; s += 9; }
+         { late_expand = 0; s += 9; }
        else
          {
          const uschar * t = s;
@@ -5697,7 +5697,6 @@ while (*s)
            late_expand,                /* expand args if not already done */
             0,                          /* not relevant when... */
             NULL,                       /* no transporting address */
-           late_expand,                /* allow tainted args, when expand-after-split */
             US"${run} expansion",       /* for error messages */
             &expand_string_message))    /* where to put error message */
           goto EXPAND_FAILED;

diff --git a/src/src/functions.h b/src/src/functions.h
index b5829a54c40275559bef013852e7b86e3ce0b9b1..0b030e4fe2f7a0fb2ea9bd4113a86285a9df8b1b 100644 (file)
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -634,7 +634,7 @@ extern BOOL    transport_pass_socket(const uschar *, const uschar *, const uscha
                        );
 extern uschar *transport_rcpt_address(address_item *, BOOL);
 extern BOOL    transport_set_up_command(const uschar ***, const uschar *,
-                BOOL, int, address_item *, BOOL, const uschar *, uschar **);
+                unsigned, int, address_item *, const uschar *, uschar **);
 extern void    transport_update_waiting(host_item *, uschar *);
 extern BOOL    transport_write_block(transport_ctx *, uschar *, int, BOOL);
 extern void    transport_write_reset(int);

diff --git a/src/src/macros.h b/src/src/macros.h
index ed7a259aa6f850ce11e9403c6ededad8a8bab234..941c4f00c1b47b69ab8d186a043cba247ec201a7 100644 (file)
--- a/src/src/macros.h
+++ b/src/src/macros.h
@@ -1153,4 +1153,9 @@ typedef unsigned mcs_flags;
 #define QL_MSGID_ONLY          3
 #define QL_UNSORTED            8
 
+/* Flags for transport_set_up_command() */
+#define TSUC_EXPAND_ARGS       BIT(0)
+#define TSUC_ALLOW_TAINTED_ARGS        BIT(1)
+#define TSUC_ALLOW_RECIPIENTS  BIT(2)
+
 /* End of macros.h */

diff --git a/src/src/routers/queryprogram.c b/src/src/routers/queryprogram.c
index 51fdad2299ad4b42fab5d6405e3f1c363a6fc142..ae33682e2c6935250e14c15a7746e7024375d8ea 100644 (file)
--- a/src/src/routers/queryprogram.c
+++ b/src/src/routers/queryprogram.c
@@ -289,10 +289,9 @@ if (curr_uid != root_uid && (uid != curr_uid || gid != curr_gid))
 
 if (!transport_set_up_command(&argvptr, /* anchor for arg list */
     ob->command,                        /* raw command */
-    TRUE,                               /* expand the arguments */
+    TSUC_EXPAND_ARGS,                   /* arguments expanded but must not be tainted */
     0,                                  /* not relevant when... */
     NULL,                               /* no transporting address */
-    FALSE,                             /* args must be untainted */
     US"queryprogram router",            /* for error messages */
     &addr->message))                    /* where to put error message */
   return DEFER;

diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index e6f9808ddf5a48199ed8949b7f758f65ba56288b..765d33bf40a58152d252ade6bd04ca163c34331f 100644 (file)
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -5485,8 +5485,8 @@ while (done <= 0)
        BOOL rc;
        etrn_command = smtp_etrn_command;
        deliver_domain = smtp_cmd_data;
-       rc = transport_set_up_command(&argv, smtp_etrn_command, TRUE, 0, NULL,
-         FALSE, US"ETRN processing", &error);
+       rc = transport_set_up_command(&argv, smtp_etrn_command, TSUC_EXPAND_ARGS, 0, NULL,
+         US"ETRN processing", &error);
        deliver_domain = NULL;
        if (!rc)
          {

diff --git a/src/src/transport.c b/src/src/transport.c
index c125cc7c3995b93beb52f1516cd2ce976ede4db5..1e8bb4aa70b5d5377d3f4a1bd5e3941c94928c7c 100644 (file)
--- a/src/src/transport.c
+++ b/src/src/transport.c
@@ -2133,18 +2133,18 @@ return FALSE;
 
 /* This function is called when a command line is to be parsed and executed
 directly, without the use of /bin/sh. It is called by the pipe transport,
-the queryprogram router, and also from the main delivery code when setting up a
+the queryprogram router, for any ${run } expansion,
+and also from the main delivery code when setting up a
 transport filter process. The code for ETRN also makes use of this; in that
 case, no addresses are passed.
 
 Arguments:
   argvptr            pointer to anchor for argv vector
   cmd                points to the command string (modified IN PLACE)
-  expand_arguments   true if expansion is to occur
+  flags                     bits for expand-args, allow taint, allow $recipients
   expand_failed      error value to set if expansion fails; not relevant if
                      addr == NULL
   addr               chain of addresses, or NULL
-  allow_tainted_args as it says; used for ${run}
   etext              text for use in error messages
   errptr             where to put error message if addr is NULL;
                      otherwise it is put in the first address
@@ -2155,8 +2155,8 @@ Returns:             TRUE if all went well; otherwise an error will be
 
 BOOL
 transport_set_up_command(const uschar *** argvptr, const uschar * cmd,
-  BOOL expand_arguments, int expand_failed, address_item * addr,
-  BOOL allow_tainted_args, const uschar * etext, uschar ** errptr)
+  unsigned flags, int expand_failed, address_item * addr,
+  const uschar * etext, uschar ** errptr)
 {
 const uschar ** argv, * s;
 int address_count = 0, argcount = 0, max_args;
@@ -2231,10 +2231,10 @@ DEBUG(D_transport)
     debug_printf("  argv[%d] = '%s'\n", i, string_printing(argv[i]));
   }
 
-if (expand_arguments)
+if (flags & TSUC_EXPAND_ARGS)
   {
-  BOOL allow_dollar_recipients = addr && addr->parent
-    && Ustrcmp(addr->parent->address, "system-filter") == 0;
+  BOOL allow_dollar_recipients = (flags & TSUC_ALLOW_RECIPIENTS)
+    || (addr && addr->parent && Ustrcmp(addr->parent->address, "system-filter") == 0); /*XXX could we check this at caller? */
 
   for (int i = 0; argv[i]; i++)
     {
@@ -2421,7 +2421,7 @@ if (expand_arguments)
          debug_printf("SPECIFIC TESTSUITE EXEMPTION: tainted arg '%s'\n",
                      expanded_arg);
        }
-      else if (  !allow_tainted_args
+      else if (  !(flags & TSUC_ALLOW_TAINTED_ARGS)
              && arg_is_tainted(expanded_arg, i, addr, etext, errptr))
        return FALSE;
       argv[i] = expanded_arg;

diff --git a/src/src/transports/lmtp.c b/src/src/transports/lmtp.c
index 776c40e05070692406b3c9739bbd9ce853fe02ff..2dd0f328b3afe731c22c0297f40d38e28ad54cd3 100644 (file)
--- a/src/src/transports/lmtp.c
+++ b/src/src/transports/lmtp.c
@@ -490,8 +490,8 @@ if (ob->cmd)
   {
   DEBUG(D_transport) debug_printf("using command %s\n", ob->cmd);
   sprintf(CS buffer, "%.50s transport", tblock->name);
-  if (!transport_set_up_command(&argv, ob->cmd, TRUE, PANIC, addrlist, FALSE,
-       buffer, NULL))
+  if (!transport_set_up_command(&argv, ob->cmd, TSUC_EXPAND_ARGS, PANIC,
+       addrlist, buffer, NULL))
     return FALSE;
 
   /* If the -N option is set, can't do any more. Presume all has gone well. */

diff --git a/src/src/transports/pipe.c b/src/src/transports/pipe.c
index c3547eefe6b4f772e4ff3e44e4ffd3d7de922a7d..18f9fd84e852b2dc839c38a6ff67c484ac05e6a2 100644 (file)
--- a/src/src/transports/pipe.c
+++ b/src/src/transports/pipe.c
@@ -292,9 +292,9 @@ Returns:             TRUE if all went well; otherwise an error will be
 */
 
 static BOOL
-set_up_direct_command(const uschar ***argvptr, uschar *cmd,
-  BOOL expand_arguments, int expand_fail, address_item *addr, uschar *tname,
-  pipe_transport_options_block *ob)
+set_up_direct_command(const uschar *** argvptr, uschar * cmd,
+  BOOL expand_arguments, int expand_fail, address_item * addr, uschar * tname,
+  pipe_transport_options_block * ob)
 {
 BOOL permitted = FALSE;
 const uschar **argv;
@@ -304,8 +304,9 @@ call the common function for creating an argument list and expanding
 the items if necessary. If it fails, this function fails (error information
 is in the addresses). */
 
-if (!transport_set_up_command(argvptr, cmd, expand_arguments, expand_fail,
-      addr, FALSE, string_sprintf("%.50s transport", tname), NULL))
+if (!transport_set_up_command(argvptr, cmd,
+      expand_arguments ? TSUC_EXPAND_ARGS : 0,
+      expand_fail, addr, string_sprintf("%.50s transport", tname), NULL))
   return FALSE;
 
 /* Point to the set-up arguments. */

diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c
index c502d7365d63d32f5de33731f6320987369b2bd8..df94eebde581f3526b6fb4379d18115020ec4df1 100644 (file)
--- a/src/src/transports/smtp.c
+++ b/src/src/transports/smtp.c
@@ -3833,7 +3833,7 @@ if (tblock->filter_command)
   yield ERROR. */
 
   if (!transport_set_up_command(&transport_filter_argv,
-       tblock->filter_command, TRUE, DEFER, addrlist, FALSE,
+       tblock->filter_command, TSUC_EXPAND_ARGS, DEFER, addrlist,
        string_sprintf("%.50s transport filter", tblock->name), NULL))
     {
     set_errno_nohost(addrlist->next, addrlist->basic_errno, addrlist->message, DEFER,

diff --git a/test/log/0635 b/test/log/0635
index a8ccbcfbe545ffec1d5d4d628f951bf5bf5b9bfc..5126c2c63f79faf9edf26559ea7f7b0ef82e8328 100644 (file)
--- a/test/log/0635
+++ b/test/log/0635
@@ -1,5 +1,5 @@
 1999-03-02 09:44:33 10HmaX-000000005vi-0000 $recipients: "CALLER@the.local.host.name"
-1999-03-02 09:44:33 10HmaX-000000005vi-0000 run-wrapped $recipients: "\n"
+1999-03-02 09:44:33 10HmaX-000000005vi-0000 run-wrapped $recipients: "CALLER@the.local.host.name\n"
 1999-03-02 09:44:33 10HmaX-000000005vi-0000 <= someone@some.domain U=CALLER P=local-smtp S=sss
 1999-03-02 09:44:33 10HmaX-000000005vi-0000 => CALLER <CALLER@the.local.host.name> R=localuser T=local_delivery
 1999-03-02 09:44:33 10HmaX-000000005vi-0000 Completed