X-Git-Url: https://git.exim.org/exim.git/blobdiff_plain/7596f24296bffbb02eeb7e13b6580d67ccaf798e..1d28cc061677bd07d9bed48dd84bd5c590247043:/src/src/transport.c diff --git a/src/src/transport.c b/src/src/transport.c index 37623ea1a..d04ea516a 100644 --- a/src/src/transport.c +++ b/src/src/transport.c @@ -2,9 +2,10 @@ * Exim - an Internet mail transport agent * *************************************************/ +/* Copyright (c) The Exim Maintainers 2020 - 2022 */ /* Copyright (c) University of Cambridge 1995 - 2018 */ -/* Copyright (c) The Exim Maintainers 2020 - 2021 */ /* See the file NOTICE for conditions of use and distribution. */ +/* SPDX-License-Identifier: GPL-2.0-or-later */ /* General functions concerned with transportation, and generic options for all transports. */ @@ -782,7 +783,7 @@ for (header_line * h = header_list; h; h = h->next) if (h->type != htype_old) /* Header removed */ else - DEBUG(D_transport) debug_printf("removed header line:\n%s---\n", h->text); + DEBUG(D_transport) debug_printf("removed header line:\n %s---\n", h->text); } /* Add on any address-specific headers. If there are multiple addresses, @@ -798,8 +799,8 @@ Headers added to an address by a router are guaranteed to end with a newline. if (addr) { - header_line *hprev = addr->prop.extra_headers; - header_line *hnext, * h; + header_line * hprev = addr->prop.extra_headers, * hnext, * h; + for (int i = 0; i < 2; i++) for (h = hprev, hprev = NULL; h; h = hnext) { @@ -810,7 +811,7 @@ if (addr) { if (!sendfn(tctx, h->text, h->slen)) return FALSE; DEBUG(D_transport) - debug_printf("added header line(s):\n%s---\n", h->text); + debug_printf("added header line(s):\n %s---\n", h->text); } } } @@ -838,7 +839,7 @@ if (tblock && (list = CUS tblock->add_headers)) return FALSE; DEBUG(D_transport) { - debug_printf("added header line:\n%s", s); + debug_printf("added header line:\n %s", s); if (s[len-1] != '\n') debug_printf("\n"); debug_printf("---\n"); } @@ -1155,8 +1156,12 @@ f.spool_file_wireformat = FALSE; /* If requested, add a terminating "." line (SMTP output). */ -if (tctx->options & topt_end_dot && !write_chunk(tctx, US".\n", 2)) - return FALSE; +if (tctx->options & topt_end_dot) + { + smtp_debug_cmd(US".", 0); + if (!write_chunk(tctx, US".\n", 2)) + return FALSE; + } /* Write out any remaining data in the buffer before returning. */ @@ -1426,7 +1431,7 @@ if (yield) ? !write_chunk(tctx, US".\n", 2) : !write_chunk(tctx, US"\n.\n", 3) ) ) - yield = FALSE; + { smtp_debug_cmd(US".", 0); yield = FALSE; } /* Write out any remaining data in the buffer. */ @@ -2049,6 +2054,31 @@ else +/* Enforce all args untainted, for consistency with a router-sourced pipe +command, where (because the whole line is passed as one to the tpt) a +tainted arg taints the executable name. It's unclear also that letting an +attacker supply command arguments is wise. */ + +static BOOL +arg_is_tainted(const uschar * s, int argn, address_item * addr, + const uschar * etext, uschar ** errptr) +{ +if (is_tainted(s)) + { + uschar * msg = string_sprintf("Tainted arg %d for %s command: '%s'", + argn, etext, s); + if (addr) + { + addr->transport_return = FAIL; + addr->message = msg; + } + else *errptr = msg; + return TRUE; + } +return FALSE; +} + + /************************************************* * Set up direct (non-shell) command * *************************************************/ @@ -2066,6 +2096,7 @@ Arguments: 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 @@ -2075,15 +2106,12 @@ Returns: TRUE if all went well; otherwise an error will be */ BOOL -transport_set_up_command(const uschar ***argvptr, uschar *cmd, - BOOL expand_arguments, int expand_failed, address_item *addr, - uschar *etext, uschar **errptr) +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) { -const uschar **argv; -uschar *s, *ss; -int address_count = 0; -int argcount = 0; -int max_args; +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 supplied, and allow for that many arguments, plus an additional 60, which @@ -2100,22 +2128,19 @@ trailing space at the start and end. Double-quoted arguments can contain \\ and arguments are verbatim. Copy each argument into a new string. */ s = cmd; -while (isspace(*s)) s++; +Uskip_whitespace(&s); for (; *s && argcount < max_args; argcount++) { if (*s == '\'') { - ss = s + 1; - while (*ss && *ss != '\'') ss++; - argv[argcount] = ss = store_get(ss - s++, cmd); - while (*s && *s != '\'') *ss++ = *s++; - if (*s) s++; - *ss++ = 0; + int n = Ustrcspn(++s, "'"); + argv[argcount] = string_copyn(s, n); + if (*(s += n) == '\'') s++; } else argv[argcount] = string_dequote(CUSS &s); - while (isspace(*s)) s++; + Uskip_whitespace(&s); } argv[argcount] = NULL; @@ -2126,7 +2151,7 @@ if (*s) { uschar *msg = string_sprintf("Too many arguments in command \"%s\" in " "%s", cmd, etext); - if (addr != NULL) + if (addr) { addr->transport_return = FAIL; addr->message = msg; @@ -2165,7 +2190,6 @@ if (expand_arguments) for (int i = 0; argv[i]; i++) { - /* Handle special fudge for passing an address list */ if (addr && @@ -2189,6 +2213,16 @@ if (expand_arguments) for (address_item * ad = addr; ad; ad = ad->next) { + /* $pipe_addresses is spefically not checked for taint, because there is + a testcase (321) depending on it. It's unclear if the exact thing being + done really needs to be legitimate, though I suspect it reflects an + actual use-case that showed up a bug. + This is a hole in the taint-pretection, mitigated only in that + shell-syntax metachars cannot be injected via this route. */ + + DEBUG(D_transport) if (is_tainted(ad->address)) + debug_printf("tainted element '%s' from $pipe_addresses\n", ad->address); + argv[i++] = ad->address; argcount++; } @@ -2229,23 +2263,19 @@ if (expand_arguments) return FALSE; } - while (isspace(*s)) s++; /* strip leading space */ + Uskip_whitespace(&s); /* strip leading space */ while (*s && address_pipe_argcount < address_pipe_max_args) { if (*s == '\'') - { - int n; - for (ss = s + 1; *ss && *ss != '\''; ) ss++; - n = ss - s++; - address_pipe_argv[address_pipe_argcount++] = ss = store_get(n, s); - while (*s && *s != '\'') *ss++ = *s++; - if (*s) s++; - *ss++ = 0; - } - else address_pipe_argv[address_pipe_argcount++] = - string_copy(string_dequote(CUSS &s)); - while (isspace(*s)) s++; /* strip space after arg */ + { + int n = Ustrcspn(++s, "'"); + argv[argcount] = string_copyn(s, n); + if (*(s += n) == '\'') s++; + } + else + address_pipe_argv[address_pipe_argcount++] = string_dequote(CUSS &s); + Uskip_whitespace(&s); /* strip space after arg */ } address_pipe_argv[address_pipe_argcount] = NULL; @@ -2265,8 +2295,9 @@ if (expand_arguments) } /* address_pipe_argcount - 1 - * because we are replacing $address_pipe in the argument list - * with the first thing it expands to */ + because we are replacing $address_pipe in the argument list + with the first thing it expands to */ + if (argcount + address_pipe_argcount - 1 > max_args) { addr->transport_return = FAIL; @@ -2296,9 +2327,13 @@ if (expand_arguments) [argv 0][argv 1][argv 2=pipeargv[0]][argv 3=pipeargv[1]][old argv 3][0] */ for (int address_pipe_i = 0; - address_pipe_argv[address_pipe_i] != US 0; + address_pipe_argv[address_pipe_i]; address_pipe_i++, argcount++) - argv[i++] = address_pipe_argv[address_pipe_i]; + { + uschar * s = address_pipe_argv[address_pipe_i]; + if (arg_is_tainted(s, i, addr, etext, errptr)) return FALSE; + argv[i++] = s; + } /* Subtract one since we replace $address_pipe */ argcount--; @@ -2310,9 +2345,10 @@ if (expand_arguments) else { const uschar *expanded_arg; + BOOL enable_dollar_recipients_g = f.enable_dollar_recipients; f.enable_dollar_recipients = allow_dollar_recipients; expanded_arg = expand_cstring(argv[i]); - f.enable_dollar_recipients = FALSE; + f.enable_dollar_recipients = enable_dollar_recipients_g; if (!expanded_arg) { @@ -2327,6 +2363,17 @@ if (expand_arguments) else *errptr = msg; return FALSE; } + + if ( f.running_in_test_harness && is_tainted(expanded_arg) + && Ustrcmp(etext, "queryprogram router") == 0) + { /* 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 + && arg_is_tainted(expanded_arg, i, addr, etext, errptr)) + return FALSE; argv[i] = expanded_arg; } } @@ -2335,7 +2382,10 @@ if (expand_arguments) { debug_printf("direct command after expansion:\n"); for (int i = 0; argv[i]; i++) - debug_printf(" argv[%d] = %s\n", i, string_printing(argv[i])); + { + debug_printf(" argv[%d] = '%s'\n", i, string_printing(argv[i])); + debug_print_taint(argv[i]); + } } }