Gitweb:
https://git.exim.org/exim.git/commitdiff/9214d2e4dfd9d4f29e9cb7a0eea8a0758ed1b34a
Commit: 9214d2e4dfd9d4f29e9cb7a0eea8a0758ed1b34a
Parent: 0d2e392e281e96d9f9f2f3dd438affe3f2563c57
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Sat Jan 11 21:48:25 2020 +0000
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Sat Jan 11 21:48:25 2020 +0000
pipe transport: taint-enforce command
---
doc/doc-docbook/spec.xfpt | 6 ++++++
doc/doc-txt/ChangeLog | 3 ++-
src/src/transports/pipe.c | 30 +++++++++++++++++-------------
test/confs/0533 | 2 +-
test/confs/{0533 => 0585} | 15 +--------------
test/log/0585 | 2 ++
test/paniclog/0585 | 1 +
test/scripts/0000-Basic/0585 | 4 ++++
test/stderr/0585 | 1 +
9 files changed, 35 insertions(+), 29 deletions(-)
diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index bb2ce12..0e44b11 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -23816,6 +23816,12 @@ directories are also controllable. See chapter &<<CHAPenvironment>>& for
details of the local delivery environment and chapter &<<CHAPbatching>>&
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 2729295..c803fdb 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 a16a197..4ca35aa 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 7c2ca9d..290d5e6 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/0533 b/test/confs/0585
similarity index 60%
copy from test/confs/0533
copy to test/confs/0585
index 7c2ca9d..a0b35e7 100644
--- a/test/confs/0533
+++ b/test/confs/0585
@@ -1,4 +1,4 @@
-# Exim test configuration 0533
+# Exim test configuration 0585
.include DIR/aux-var/std_conf_prefix
@@ -12,13 +12,6 @@ primary_hostname = myhost.test.ex
begin routers
-r1:
- driver = redirect
- local_part_prefix = file-
- local_part_suffix = =*
- data = DIR/test-mail/${bless:${substr_1:$local_part_suffix}}
- file_transport = t1
-
r2:
driver = redirect
local_part_prefix = pipe-
@@ -32,12 +25,6 @@ r2:
begin transports
-t1:
- driver = appendfile
- envelope_to_add
- user = CALLER
- batch_max = 10
-
t2:
driver = pipe
user = CALLER
diff --git a/test/log/0585 b/test/log/0585
new file mode 100644
index 0000000..db6a92e
--- /dev/null
+++ b/test/log/0585
@@ -0,0 +1,2 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 <= CALLER@??? U=CALLER P=local S=sss
+1999-03-02 09:44:33 10HmaX-0005vi-00 == |TESTSUITE/bin/iefbr14 <pipe-userx=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 0000000..dc309d8
--- /dev/null
+++ b/test/paniclog/0585
@@ -0,0 +1 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 == |TESTSUITE/bin/iefbr14 <pipe-userx=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 0000000..5fbc931
--- /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@???
+A test message.
+****
diff --git a/test/stderr/0585 b/test/stderr/0585
new file mode 100644
index 0000000..dc309d8
--- /dev/null
+++ b/test/stderr/0585
@@ -0,0 +1 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 == |TESTSUITE/bin/iefbr14 <pipe-userx=TESTSUITE/bin/iefbr14@???> R=r2 T=t2 defer (0): Tainted 'TESTSUITE/bin/iefbr14' (command for t2 transport) not permitted
