pipe transport: taint-enforce command

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index bb2ce122c9af82d65c0b5ac4025c0c57c0a87e07..0e44b119bca1e7af29c51edca5af04c774b5e983 100644 (file)
--- 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 27292954ac22fc97a3655b63740e1c09fcef3233..c803fdb7edb1765250ad89de76afa56146096ec6 100644 (file)
--- 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 a16a197a4cfa571dc4eb7d2ea034bf0cd0aa69b6..4ca35aa41532b7b591238599f4f62496988e55be 100644 (file)
--- 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 7c2ca9dc2890b2fa40498e577258635734678549..290d5e68a8822734345e2bb08e98a94f8869df4c 100644 (file)
--- 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 (file)
index 0000000..a0b35e7
--- /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 (file)
index 0000000..db6a92e
--- /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 <pipe-userx=TESTSUITE/bin/iefbr14@test.ex> 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 (file)
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@test.ex> 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 (file)
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@test.ex
+A test message.
+****

diff --git a/test/stderr/0585 b/test/stderr/0585
new file mode 100644 (file)
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@test.ex> R=r2 T=t2 defer (0): Tainted 'TESTSUITE/bin/iefbr14' (command for t2 transport) not permitted