From 6707bfa9fb78858de938a1abca2846c820c5ded7 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 3 Aug 2023 18:40:42 +0100 Subject: [PATCH 2/2] Fix $recipients expansion when used within ${run...}. Bug 3013 Broken-by: cfe6acff2ddc --- doc/ChangeLog | 3 +++ src/deliver.c | 2 +- src/expand.c | 5 ++--- src/functions.h | 2 +- src/macros.h | 5 +++++ src/routers/queryprogram.c | 3 +-- src/smtp_in.c | 4 ++-- src/transport.c | 18 +++++++++--------- src/transports/lmtp.c | 4 ++-- src/transports/pipe.c | 11 ++++++----- src/transports/smtp.c | 2 +- test/log/0635 | 2 +- 12 files changed, 34 insertions(+), 27 deletions(-) --- a/src/deliver.c +++ b/src/deliver.c @@ -2382,11 +2382,11 @@ if ((pid = exim_fork(US"delivery-local") if (tp->filter_command) { 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; if (ok) --- a/src/expand.c +++ b/src/expand.c @@ -5527,11 +5527,11 @@ while (*s) case EITEM_RUN: { FILE * f; const uschar * arg, ** argv; - BOOL late_expand = TRUE; + unsigned late_expand = TSUC_EXPAND_ARGS | TSUC_ALLOW_TAINTED_ARGS | TSUC_ALLOW_RECIPIENTS; if (expand_forbid & RDO_RUN) { expand_string_message = US"running a command is not permitted"; goto EXPAND_FAILED; @@ -5540,11 +5540,11 @@ while (*s) /* Handle options to the "run" */ while (*s == ',') { if (Ustrncmp(++s, "preexpand", 9) == 0) - { late_expand = FALSE; s += 9; } + { late_expand = 0; s += 9; } else { const uschar * t = s; while (isalpha(*++t)) ; expand_string_message = string_sprintf("bad option '%.*s' for run", @@ -5599,11 +5599,10 @@ while (*s) if (!transport_set_up_command(&argv, /* anchor for arg list */ arg, /* raw command */ 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; /* Create the child process, making it a group leader. */ --- a/src/functions.h +++ b/src/functions.h @@ -616,11 +616,11 @@ extern BOOL transport_pass_socket(con , unsigned, unsigned, unsigned #endif ); 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); extern BOOL transport_write_string(int, const char *, ...); extern BOOL transport_headers_send(transport_ctx *, --- a/src/macros.h +++ b/src/macros.h @@ -1112,6 +1112,11 @@ should not be one active. */ #define NOTIFIER_SOCKET_NAME "exim_daemon_notify" #define NOTIFY_MSG_QRUN 1 /* Notify message types */ #define NOTIFY_QUEUE_SIZE_REQ 2 +/* 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 */ --- a/src/routers/queryprogram.c +++ b/src/routers/queryprogram.c @@ -286,14 +286,13 @@ if (curr_uid != root_uid && (uid != curr /* Set up the command to run */ 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; /* Create the child process, making it a group leader. */ --- a/src/smtp_in.c +++ b/src/smtp_in.c @@ -5840,12 +5840,12 @@ while (done <= 0) { uschar *error; 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) { log_write(0, LOG_MAIN|LOG_PANIC, "failed to set up ETRN command: %s", error); --- a/src/transport.c +++ b/src/transport.c @@ -2082,34 +2082,34 @@ return FALSE; * Set up direct (non-shell) command * *************************************************/ /* 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 Returns: TRUE if all went well; otherwise an error will be set in the first address and FALSE returned */ 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; /* Get store in which to build an argument list. Count the number of addresses @@ -2180,14 +2180,14 @@ DEBUG(D_transport) debug_printf("direct command:\n"); for (int i = 0; argv[i]; i++) 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++) { DEBUG(D_expand) debug_printf_indent("arg %d\n", i); @@ -2370,11 +2370,11 @@ if (expand_arguments) { /* hack, would be good to not need it */ DEBUG(D_transport) 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; } } --- a/src/transports/lmtp.c +++ b/src/transports/lmtp.c @@ -487,12 +487,12 @@ argument list and expanding the items. * 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. */ if (f.dont_deliver) goto MINUS_N; --- a/src/transports/pipe.c +++ b/src/transports/pipe.c @@ -289,24 +289,25 @@ Arguments: Returns: TRUE if all went well; otherwise an error will be set in the first address and FALSE returned */ 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; /* Set up "transport " to be put in any error messages, and then 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. */ argv = *argvptr; --- a/src/transports/smtp.c +++ b/src/transports/smtp.c @@ -3803,11 +3803,11 @@ if (tblock->filter_command) /* On failure, copy the error to all addresses, abandon the SMTP call, and 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, FALSE, &sx->delivery_start); yield = ERROR; --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -76,10 +76,12 @@ JH/31 Bug 2998: Fix ${utf8clean:...} to editor insists on emitting only valid UTF-8. JH/32 Fix "tls_dhparam = none" under GnuTLS. At least with 3.7.9 this gave a null-indireciton SIGSEGV for the receive process. +JH/34 Bug 3013: Fix use of $recipients within arguments for ${run...}. + In 4.96 this would expand to empty. Exim version 4.96 ----------------- JH/01 Move the wait-for-next-tick (needed for unique message IDs) from