From 6707bfa9fb78858de938a1abca2846c820c5ded7 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 3 Aug 2023 18:40:42 +0100 Subject: [PATCH] Fix $recipients expansion when used within ${run...}. Bug 3013 Broken-by: cfe6acff2ddc --- doc/doc-txt/ChangeLog | 3 +++ src/src/deliver.c | 2 +- src/src/expand.c | 5 ++--- src/src/functions.h | 2 +- src/src/macros.h | 5 +++++ src/src/routers/queryprogram.c | 3 +-- src/src/smtp_in.c | 4 ++-- src/src/transport.c | 18 +++++++++--------- src/src/transports/lmtp.c | 4 ++-- src/src/transports/pipe.c | 11 ++++++----- src/src/transports/smtp.c | 2 +- test/log/0635 | 2 +- 12 files changed, 34 insertions(+), 27 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index ecb4aadec..efdc228b6 100644 --- 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 bea38c5d1..52270368e 100644 --- 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 e0c571ade..259d463a4 100644 --- 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 b5829a54c..0b030e4fe 100644 --- 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 ed7a259aa..941c4f00c 100644 --- 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 51fdad229..ae33682e2 100644 --- 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 e6f9808dd..765d33bf4 100644 --- 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 c125cc7c3..1e8bb4aa7 100644 --- 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 776c40e05..2dd0f328b 100644 --- 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 c3547eefe..18f9fd84e 100644 --- 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 c502d7365..df94eebde 100644 --- 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 a8ccbcfbe..5126c2c63 100644 --- 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 R=localuser T=local_delivery 1999-03-02 09:44:33 10HmaX-000000005vi-0000 Completed -- 2.30.2