From: Jeremy Harris Date: Sat, 11 Jan 2020 21:48:25 +0000 (+0000) Subject: pipe transport: taint-enforce command X-Git-Url: https://git.exim.org/users/jgh/exim.git/commitdiff_plain/9214d2e4dfd9d4f29e9cb7a0eea8a0758ed1b34a pipe transport: taint-enforce command --- diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index bb2ce122c..0e44b119b 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -23816,6 +23816,12 @@ directories are also controllable. See chapter &<>& for details of the local delivery environment and chapter &<>& for a discussion of local delivery batching. +.new +.cindex "tainted data" "in pipe command" +.cindex pipe "tainted data" +Tainted data may not be used for the command name. +.wen + .section "Concurrent delivery" "SECID140" If two messages arrive at almost the same time, and both are routed to a pipe diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 27292954a..c803fdb7e 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -88,7 +88,8 @@ JH/19 Bug 2507: Modules: on handling a dynamic-module (lookups) open failure, information. JH/20 Taint checking: disallow use of tainted data for the appendfile transport - file and directory options. Previously this was permitted. + file and directory options, and for the pipe transport command. + Previously this was permitted. Exim version 4.93 diff --git a/src/src/transports/pipe.c b/src/src/transports/pipe.c index a16a197a4..4ca35aa41 100644 --- a/src/src/transports/pipe.c +++ b/src/src/transports/pipe.c @@ -3,6 +3,7 @@ *************************************************/ /* Copyright (c) University of Cambridge 1995 - 2018 */ +/* Copyright (c) The Exim maintainers 2020 */ /* See the file NOTICE for conditions of use and distribution. */ @@ -586,7 +587,6 @@ symbol. In other cases, the command is supplied as one of the pipe transport's options. */ if (testflag(addr, af_pfr) && addr->local_part[0] == '|') - { if (ob->force_command) { /* Enables expansion of $address_pipe into separate arguments */ @@ -602,7 +602,6 @@ if (testflag(addr, af_pfr) && addr->local_part[0] == '|') expand_arguments = testflag(addr, af_expand_pipe); expand_fail = FAIL; } - } else { cmd = ob->cmd; @@ -615,13 +614,20 @@ else * coming from addr->local_part[0] == '|' */ -if (cmd == NULL || *cmd == '\0') +if (!cmd || !*cmd) { addr->transport_return = DEFER; addr->message = string_sprintf("no command specified for %s transport", tblock->name); return FALSE; } +if (is_tainted(cmd)) + { + addr->message = string_sprintf("Tainted '%s' (command " + "for %s transport) not permitted", cmd, tblock->name); + addr->transport_return = PANIC; + return FALSE; + } /* When a pipe is set up by a filter file, there may be values for $thisaddress and numerical the variables in existence. These are passed in @@ -631,8 +637,8 @@ if (expand_arguments && addr->pipe_expandn) { uschar **ss = addr->pipe_expandn; expand_nmax = -1; - if (*ss != NULL) filter_thisaddress = *ss++; - while (*ss != NULL) + if (*ss) filter_thisaddress = *ss++; + while (*ss) { expand_nstring[++expand_nmax] = *ss; expand_nlength[expand_nmax] = Ustrlen(*ss++); @@ -685,7 +691,6 @@ else if (timezone_string != NULL && timezone_string[0] != 0) /* Add any requested items */ if (envlist) - { if (!(envlist = expand_cstring(envlist))) { addr->transport_return = DEFER; @@ -694,7 +699,6 @@ if (envlist) expand_string_message); return FALSE; } - } while ((ss = string_nextinlist(&envlist, &envsep, big_buffer, big_buffer_size))) { @@ -831,10 +835,10 @@ transport_count = 0; /* First write any configured prefix information */ -if (ob->message_prefix != NULL) +if (ob->message_prefix) { uschar *prefix = expand_string(ob->message_prefix); - if (prefix == NULL) + if (!prefix) { addr->transport_return = f.search_find_defer? DEFER : PANIC; addr->message = string_sprintf("Expansion of \"%s\" (prefix for %s " @@ -951,8 +955,8 @@ above timed out. */ if ((rc = child_close(pid, timeout)) != 0) { - uschar *tmsg = (addr->message == NULL)? US"" : - string_sprintf(" (preceded by %s)", addr->message); + uschar * tmsg = addr->message + ? string_sprintf(" (preceded by %s)", addr->message) : US""; /* The process did not complete in time; kill its process group and fail the delivery. It appears to be necessary to kill the output process too, as @@ -1058,7 +1062,7 @@ if ((rc = child_close(pid, timeout)) != 0) { /* Always handle execve() failure specially if requested to */ - if (ob->freeze_exec_fail && (rc == EX_EXECFAILED)) + if (ob->freeze_exec_fail && rc == EX_EXECFAILED) { addr->transport_return = DEFER; addr->special_action = SPECIAL_FREEZE; @@ -1106,7 +1110,7 @@ if ((rc = child_close(pid, timeout)) != 0) rc-128, os_strsignal(rc-128)) : US os_strexit(rc); - if (*ss != 0) + if (*ss) { g = string_catn(g, US" ", 1); g = string_cat (g, ss); diff --git a/test/confs/0533 b/test/confs/0533 index 7c2ca9dc2..290d5e68a 100644 --- a/test/confs/0533 +++ b/test/confs/0533 @@ -24,7 +24,7 @@ r2: local_part_prefix = pipe- local_part_suffix = =* caseful_local_part = true - data = |${substr_1:$local_part_suffix} + data = |${bless:${substr_1:$local_part_suffix}} pipe_transport = t2 diff --git a/test/confs/0585 b/test/confs/0585 new file mode 100644 index 000000000..a0b35e7cf --- /dev/null +++ b/test/confs/0585 @@ -0,0 +1,33 @@ +# Exim test configuration 0585 + +.include DIR/aux-var/std_conf_prefix + +primary_hostname = myhost.test.ex + +# ----- Main settings ----- + + + +# ----- Routers ----- + +begin routers + +r2: + driver = redirect + local_part_prefix = pipe- + local_part_suffix = =* + caseful_local_part = true + data = |${substr_1:$local_part_suffix} + pipe_transport = t2 + + +# ----- Transports ----- + +begin transports + +t2: + driver = pipe + user = CALLER + batch_max = 10 + +# End diff --git a/test/log/0585 b/test/log/0585 new file mode 100644 index 000000000..db6a92e39 --- /dev/null +++ b/test/log/0585 @@ -0,0 +1,2 @@ +1999-03-02 09:44:33 10HmaX-0005vi-00 <= CALLER@myhost.test.ex U=CALLER P=local S=sss +1999-03-02 09:44:33 10HmaX-0005vi-00 == |TESTSUITE/bin/iefbr14 R=r2 T=t2 defer (0): Tainted 'TESTSUITE/bin/iefbr14' (command for t2 transport) not permitted diff --git a/test/paniclog/0585 b/test/paniclog/0585 new file mode 100644 index 000000000..dc309d8a3 --- /dev/null +++ b/test/paniclog/0585 @@ -0,0 +1 @@ +1999-03-02 09:44:33 10HmaX-0005vi-00 == |TESTSUITE/bin/iefbr14 R=r2 T=t2 defer (0): Tainted 'TESTSUITE/bin/iefbr14' (command for t2 transport) not permitted diff --git a/test/scripts/0000-Basic/0585 b/test/scripts/0000-Basic/0585 new file mode 100644 index 000000000..5fbc9319c --- /dev/null +++ b/test/scripts/0000-Basic/0585 @@ -0,0 +1,4 @@ +# tainted data for pipe transport command +exim -odi pipe-userx=DIR/bin/iefbr14@test.ex +A test message. +**** diff --git a/test/stderr/0585 b/test/stderr/0585 new file mode 100644 index 000000000..dc309d8a3 --- /dev/null +++ b/test/stderr/0585 @@ -0,0 +1 @@ +1999-03-02 09:44:33 10HmaX-0005vi-00 == |TESTSUITE/bin/iefbr14 R=r2 T=t2 defer (0): Tainted 'TESTSUITE/bin/iefbr14' (command for t2 transport) not permitted