From 36b600bb776d082be8cdd15c18daed3c29cfda7f Mon Sep 17 00:00:00 2001
From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Sun, 22 Mar 2020 00:55:59 +0000
Subject: [PATCH] Taint: fix dsearch result to be untainted

---
 doc/doc-docbook/spec.xfpt | 17 ++++++++++++++---
 doc/doc-txt/ChangeLog     |  4 ++++
 src/src/lookups/dsearch.c | 10 ++++++----
 test/confs/0153           |  7 +++----
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index f91d51792..cf15942db 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -6762,7 +6762,12 @@ by default, but has an option to omit them (see section &<<SECTdbmbuild>>&).
 whose name is the key by calling the &[lstat()]& function. The key may not
 contain any forward slash characters. If &[lstat()]& succeeds, the result of
 the lookup is the name of the entry, which may be a file, directory,
-symbolic link, or any other kind of directory entry. An example of how this
+symbolic link, or any other kind of directory entry.
+.new
+.cindex "tainted data" "dsearch result"
+It is regarded as untainted.
+.wen
+An example of how this
 lookup can be used to support virtual domains is given in section
 &<<SECTvirtualdomains>>&.
 .next
@@ -36771,12 +36776,18 @@ to a router of this form:
 virtual:
   driver = redirect
   domains = dsearch;/etc/mail/virtual
-  data = ${lookup{$local_part}lsearch{/etc/mail/virtual/$domain}}
+  data = ${lookup{$local_part}lsearch{/etc/mail/virtual/$domain_data}}
   no_more
 .endd
+.new
 The &%domains%& option specifies that the router is to be skipped, unless there
 is a file in the &_/etc/mail/virtual_& directory whose name is the same as the
-domain that is being processed. When the router runs, it looks up the local
+domain that is being processed.
+The &(dsearch)& lookup used results in an untainted version of &$domain$&
+being placed into the &$domain_data$& variable.
+.wen
+
+When the router runs, it looks up the local
 part in the file to find a new address (or list of addresses). The &%no_more%&
 setting ensures that if the lookup fails (leading to &%data%& being an empty
 string), Exim gives up on the address without trying any subsequent routers.
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 74568ce3f..be07ba625 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -152,6 +152,10 @@ JH/31 Fix spurious detection of timeout while writing to transport filter.
 JH/32 Bug 2541: Fix segfault on bad cmdline -f (sender) argument.  Previously
       an attempt to copy the string was made before checking it.
 
+JH/33 Fix the dsearch lookup to return an untainted result.  Previously the
+      taint of the lookup key was maintained; we now regard the presence in the
+      filesystem as sufficient validation.
+
 
 Exim version 4.93
 -----------------
diff --git a/src/src/lookups/dsearch.c b/src/src/lookups/dsearch.c
index c27f5d6e6..1eb2924f0 100644
--- a/src/src/lookups/dsearch.c
+++ b/src/src/lookups/dsearch.c
@@ -28,7 +28,7 @@ static void *
 dsearch_open(uschar *dirname, uschar **errmsg)
 {
 DIR *dp = opendir(CS dirname);
-if (dp == NULL)
+if (!dp)
   {
   int save_errno = errno;
   *errmsg = string_open_failed(errno, "%s for directory search", dirname);
@@ -47,8 +47,8 @@ return (void *)(-1);
 /* The handle will always be (void *)(-1), but don't try casting it to an
 integer as this gives warnings on 64-bit systems. */
 
-BOOL
-static dsearch_check(void *handle, uschar *filename, int modemask, uid_t *owners,
+static BOOL
+dsearch_check(void *handle, uschar *filename, int modemask, uid_t *owners,
   gid_t *owngroups, uschar **errmsg)
 {
 handle = handle;
@@ -87,7 +87,9 @@ if (Ustrchr(keystring, '/') != 0)
 filename = string_sprintf("%s/%s", dirname, keystring);
 if (Ulstat(filename, &statbuf) >= 0)
   {
-  *result = string_copy(keystring);
+  /* Since the filename exists in the filesystem, we can return a
+  non-tainted result. */
+  *result = string_copy_taint(keystring, FALSE);
   return OK;
   }
 
diff --git a/test/confs/0153 b/test/confs/0153
index 35a004a45..24f499b41 100644
--- a/test/confs/0153
+++ b/test/confs/0153
@@ -13,10 +13,9 @@ begin routers
 virtual:
   driver = redirect
   domains = *.virt.test.ex
-  address_data = ${if match{$domain}{^(.*)\\.virt\\.test\\.ex\$}{${bless:$1}}}
-  data = ${if exists{DIR/aux-fixed/TESTNUM.alias.$address_data} \
-           {${lookup{$local_part}lsearch{DIR/aux-fixed/TESTNUM.alias.$address_data}}} \
-          fail}
+  address_data = ${lookup {TESTNUM.alias.${extract {1}{.}{$domain}}} \
+			dsearch{DIR/aux-fixed} {$value}fail}
+  data = ${lookup{$local_part}lsearch{DIR/aux-fixed/$address_data}}
   no_more
 
 list:
-- 
2.30.2