Gitweb:
https://git.exim.org/exim.git/commitdiff/54a2a2a9983913a91ccef3aac107a159434a4714
Commit: 54a2a2a9983913a91ccef3aac107a159434a4714
Parent: ef8ef6c168a552e61ecde1d8d2cd816f2e87614b
Author: Jeremy Harris <jgh146exb@???>
AuthorDate: Sat Mar 28 20:01:10 2020 +0000
Committer: Jeremy Harris <jgh146exb@???>
CommitDate: Sat Mar 28 22:00:09 2020 +0000
Taint enforce: directory open backstops, single-key search filename
---
doc/doc-docbook/spec.xfpt | 4 ++++
src/exim_monitor/em_queue.c | 37 +++++++++++++++++--------------------
src/src/dbfn.c | 34 ++++++++++++++++++----------------
src/src/drtables.c | 2 +-
src/src/functions.h | 19 ++++++++++++++-----
src/src/lookups/dsearch.c | 2 +-
src/src/queue.c | 7 +++----
src/src/receive.c | 2 +-
src/src/search.c | 12 ++++++++----
src/src/sieve.c | 15 ++++++---------
src/src/spool_mbox.c | 5 ++---
src/src/transports/appendfile.c | 7 +++----
src/src/transports/tf_maildir.c | 7 +++----
test/confs/2500 | 2 ++
test/log/2500 | 6 ++++++
test/paniclog/2500 | 2 ++
test/rejectlog/2500 | 23 +++++++++++++++++++++++
test/scripts/2500-dsearch/2500 | 4 ++++
test/stderr/2500 | 2 ++
19 files changed, 120 insertions(+), 72 deletions(-)
diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 2a3fb6c..8605fdc 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -6675,6 +6675,10 @@ Two different types of data lookup are implemented:
The &'single-key'& type requires the specification of a file in which to look,
and a single key to search for. The key must be a non-empty string for the
lookup to succeed. The lookup type determines how the file is searched.
+.new
+.cindex "tainted data" "single-key lookups"
+The file string may not be tainted
+.wen
.next
.cindex "query-style lookup" "definition of"
The &'query-style'& type accepts a generalized database query. No particular
diff --git a/src/exim_monitor/em_queue.c b/src/exim_monitor/em_queue.c
index d4c01a6..7a441cb 100644
--- a/src/exim_monitor/em_queue.c
+++ b/src/exim_monitor/em_queue.c
@@ -463,7 +463,8 @@ code knows to look for them. We count the entries to set the value for the
queue stripchart, and set up data for the queue display window if the "full"
option is given. */
-void scan_spool_input(int full)
+void
+scan_spool_input(int full)
{
int i;
int subptr;
@@ -471,8 +472,6 @@ int subdir_max = 1;
int count = 0;
int indexptr = 1;
queue_item *p;
-struct dirent *ent;
-DIR *dd;
uschar input_dir[256];
uschar subdirs[64];
@@ -490,16 +489,18 @@ there is progress, output a dot for each one to the standard output. */
for (i = 0; i < subdir_max; i++)
{
int subdirchar = subdirs[i]; /* 0 for main directory */
+ DIR *dd;
+ struct dirent *ent;
+
if (subdirchar != 0)
{
input_dir[subptr] = '/';
input_dir[subptr+1] = subdirchar;
}
- dd = opendir(CS input_dir);
- if (dd == NULL) continue;
+ if (!(dd = exim_opendir(input_dir))) continue;
- while ((ent = readdir(dd)) != NULL)
+ while ((ent = readdir(dd)))
{
uschar *name = US ent->d_name;
int len = Ustrlen(name);
@@ -543,22 +544,23 @@ removing items, the total that we are comparing against isn't actually correct,
but in a long queue it won't make much difference, and in a short queue it
doesn't matter anyway!*/
-p = queue_index[0];
-while (p != NULL)
- {
+for (p = queue_index[0]; p; )
if (!p->seen)
{
- queue_item *next = p->next;
- if (p->prev == NULL) queue_index[0] = next;
- else p->prev->next = next;
- if (next == NULL)
+ queue_item * next = p->next;
+ if (p->prev)
+ p->prev->next = next;
+ else
+ queue_index[0] = next;
+ if (next)
+ next->prev = p->prev;
+ else
{
int i;
- queue_item *q = queue_index[queue_index_size-1];
+ queue_item * q = queue_index[queue_index_size-1];
for (i = queue_index_size - 1; i >= 0; i--)
if (queue_index[i] == q) queue_index[i] = p->prev;
}
- else next->prev = p->prev;
clean_up(p);
queue_total--;
p = next;
@@ -566,22 +568,17 @@ while (p != NULL)
else
{
if (++count > (queue_total * indexptr)/(queue_index_size-1))
- {
queue_index[indexptr++] = p;
- }
p->seen = FALSE; /* for next time */
p = p->next;
}
- }
/* If a lot of messages have been removed at the bottom, we may not
have got the index all filled in yet. Make sure all the pointers
are legal. */
while (indexptr < queue_index_size - 1)
- {
queue_index[indexptr++] = queue_index[queue_index_size-1];
- }
}
diff --git a/src/src/dbfn.c b/src/src/dbfn.c
index 1f058ef..3aca183 100644
--- a/src/src/dbfn.c
+++ b/src/src/dbfn.c
@@ -194,27 +194,29 @@ but creation of the database file failed. */
if (created && geteuid() == root_uid)
{
- DIR *dd;
- struct dirent *ent;
+ DIR * dd;
uschar *lastname = Ustrrchr(filename, '/') + 1;
int namelen = Ustrlen(name);
*lastname = 0;
- dd = opendir(CS filename);
- while ((ent = readdir(dd)))
- if (Ustrncmp(ent->d_name, name, namelen) == 0)
- {
- struct stat statbuf;
- /* Filenames from readdir() are trusted, so use a taint-nonchecking copy */
- strcpy(CS lastname, CCS ent->d_name);
- if (Ustat(filename, &statbuf) >= 0 && statbuf.st_uid != exim_uid)
- {
- DEBUG(D_hints_lookup) debug_printf_indent("ensuring %s is owned by exim\n", filename);
- if (exim_chown(filename, exim_uid, exim_gid))
- DEBUG(D_hints_lookup) debug_printf_indent("failed setting %s to owned by exim\n", filename);
- }
- }
+ if ((dd = exim_opendir(filename)))
+ for (struct dirent *ent; ent = readdir(dd); )
+ if (Ustrncmp(ent->d_name, name, namelen) == 0)
+ {
+ struct stat statbuf;
+ /* Filenames from readdir() are trusted,
+ so use a taint-nonchecking copy */
+ strcpy(CS lastname, CCS ent->d_name);
+ if (Ustat(filename, &statbuf) >= 0 && statbuf.st_uid != exim_uid)
+ {
+ DEBUG(D_hints_lookup)
+ debug_printf_indent("ensuring %s is owned by exim\n", filename);
+ if (exim_chown(filename, exim_uid, exim_gid))
+ DEBUG(D_hints_lookup)
+ debug_printf_indent("failed setting %s to owned by exim\n", filename);
+ }
+ }
closedir(dd);
}
diff --git a/src/src/drtables.c b/src/src/drtables.c
index b1380df..f1053bb 100644
--- a/src/src/drtables.c
+++ b/src/src/drtables.c
@@ -718,7 +718,7 @@ addlookupmodule(NULL, &whoson_lookup_module_info);
#endif
#ifdef LOOKUP_MODULE_DIR
-if (!(dd = opendir(LOOKUP_MODULE_DIR)))
+if (!(dd = exim_opendir(LOOKUP_MODULE_DIR)))
{
DEBUG(D_lookup) debug_printf("Couldn't open %s: not loading lookup modules\n", LOOKUP_MODULE_DIR);
log_write(0, LOG_MAIN, "Couldn't open %s: not loading lookup modules\n", LOOKUP_MODULE_DIR);
diff --git a/src/src/functions.h b/src/src/functions.h
index 2e5b1e4..f789c5e 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -1066,7 +1066,7 @@ static inline int
exim_open2(const char *pathname, int flags)
{
if (!is_tainted(pathname)) return open(pathname, flags);
-log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'\n", pathname);
+log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'", pathname);
errno = EACCES;
return -1;
}
@@ -1074,7 +1074,7 @@ static inline int
exim_open(const char *pathname, int flags, mode_t mode)
{
if (!is_tainted(pathname)) return open(pathname, flags, mode);
-log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'\n", pathname);
+log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'", pathname);
errno = EACCES;
return -1;
}
@@ -1082,7 +1082,7 @@ static inline int
exim_openat(int dirfd, const char *pathname, int flags)
{
if (!is_tainted(pathname)) return openat(dirfd, pathname, flags);
-log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'\n", pathname);
+log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'", pathname);
errno = EACCES;
return -1;
}
@@ -1090,7 +1090,7 @@ static inline int
exim_openat4(int dirfd, const char *pathname, int flags, mode_t mode)
{
if (!is_tainted(pathname)) return openat(dirfd, pathname, flags, mode);
-log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'\n", pathname);
+log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'", pathname);
errno = EACCES;
return -1;
}
@@ -1099,7 +1099,16 @@ static inline FILE *
exim_fopen(const char *pathname, const char *mode)
{
if (!is_tainted(pathname)) return fopen(pathname, mode);
-log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'\n", pathname);
+log_write(0, LOG_MAIN|LOG_PANIC, "Tainted filename '%s'", pathname);
+errno = EACCES;
+return NULL;
+}
+
+static inline DIR *
+exim_opendir(const uschar * name)
+{
+if (!is_tainted(name)) return opendir(CCS name);
+log_write(0, LOG_MAIN|LOG_PANIC, "Tainted dirname '%s'", name);
errno = EACCES;
return NULL;
}
diff --git a/src/src/lookups/dsearch.c b/src/src/lookups/dsearch.c
index 1eb2924..3a0df30 100644
--- a/src/src/lookups/dsearch.c
+++ b/src/src/lookups/dsearch.c
@@ -27,7 +27,7 @@ actually scanning through the list of files. */
static void *
dsearch_open(uschar *dirname, uschar **errmsg)
{
-DIR *dp = opendir(CS dirname);
+DIR * dp = exim_opendir(dirname);
if (!dp)
{
int save_errno = errno;
diff --git a/src/src/queue.c b/src/src/queue.c
index bb75c99..e02ce38 100644
--- a/src/src/queue.c
+++ b/src/src/queue.c
@@ -125,8 +125,6 @@ int resetflags = -1;
int subptr;
queue_filename *yield = NULL;
queue_filename *last = NULL;
-struct dirent *ent;
-DIR *dd;
uschar buffer[256];
queue_filename *root[LOG2_MAXNODES];
@@ -171,6 +169,7 @@ for (; i <= *subcount; i++)
{
int count = 0;
int subdirchar = subdirs[i]; /* 0 for main directory */
+ DIR *dd;
if (subdirchar != 0)
{
@@ -179,12 +178,12 @@ for (; i <= *subcount; i++)
}
DEBUG(D_queue_run) debug_printf("looking in %s\n", buffer);
- if (!(dd = opendir(CS buffer)))
+ if (!(dd = exim_opendir(buffer)))
continue;
/* Now scan the directory. */
- while ((ent = readdir(dd)))
+ for (struct dirent *ent; ent = readdir(dd); )
{
uschar *name = US ent->d_name;
int len = Ustrlen(name);
diff --git a/src/src/receive.c b/src/src/receive.c
index 0afb72b..2d88d64 100644
--- a/src/src/receive.c
+++ b/src/src/receive.c
@@ -1453,7 +1453,7 @@ if (rc == OK)
struct dirent * entry;
DIR * tempdir;
- for (tempdir = opendir(CS scandir); entry = readdir(tempdir); )
+ for (tempdir = exim_opendir(scandir); entry = readdir(tempdir); )
if (strncmpic(US entry->d_name, US"__rfc822_", 9) == 0)
{
rfc822_file_path = string_sprintf("%s/%s", scandir, entry->d_name);
diff --git a/src/src/search.c b/src/src/search.c
index 9d1a10a..dc90f53 100644
--- a/src/src/search.c
+++ b/src/src/search.c
@@ -335,6 +335,13 @@ lookup_info *lk = lookup_list[search_type];
uschar keybuffer[256];
int old_pool = store_pool;
+if (filename && is_tainted(filename))
+ {
+ log_write(0, LOG_MAIN|LOG_PANIC,
+ "Tainted filename for search: '%s'", filename);
+ return NULL;
+ }
+
/* Change to the search store pool and remember our reset point */
store_pool = POOL_SEARCH;
@@ -353,8 +360,7 @@ sprintf(CS keybuffer, "%c%.254s", search_type + '0',
if ((t = tree_search(search_tree, keybuffer)))
{
- c = (search_cache *)(t->data.ptr);
- if (c->handle)
+ if ((c = (search_cache *)t->data.ptr)->handle)
{
DEBUG(D_lookup) debug_printf_indent(" cached open\n");
store_pool = old_pool;
@@ -370,7 +376,6 @@ we are holding open in the cache. If the limit is reached, close the least
recently used one. */
if (lk->type == lookup_absfile && open_filecount >= lookup_open_max)
- {
if (!open_bot)
log_write(0, LOG_MAIN|LOG_PANIC, "too many lookups open, but can't find "
"one to close");
@@ -387,7 +392,6 @@ if (lk->type == lookup_absfile && open_filecount >= lookup_open_max)
c->handle = NULL;
open_filecount--;
}
- }
/* If opening is successful, call the file-checking function if there is one,
and if all is still well, enter the open database into the tree. */
diff --git a/src/src/sieve.c b/src/src/sieve.c
index 4467665..bf82d5e 100644
--- a/src/src/sieve.c
+++ b/src/src/sieve.c
@@ -3445,27 +3445,24 @@ if (exec && filter->vacation_directory != NULL && filter_test == FTEST_NONE)
/* clean up old vacation log databases */
- oncelogdir=opendir(CS filter->vacation_directory);
-
- if (oncelogdir ==(DIR*)0 && errno != ENOENT)
+ if ( !(oncelogdir = exim_opendir(filter->vacation_directory))
+ && errno != ENOENT)
{
- filter->errmsg=CUS "unable to open vacation directory";
+ filter->errmsg = CUS "unable to open vacation directory";
return -1;
}
- if (oncelogdir != NULL)
+ if (oncelogdir)
{
time(&now);
- while ((oncelog=readdir(oncelogdir))!=(struct dirent*)0)
- {
+ while ((oncelog = readdir(oncelogdir)))
if (strlen(oncelog->d_name)==32)
{
- uschar *s=string_sprintf("%s/%s",filter->vacation_directory,oncelog->d_name);
+ uschar *s = string_sprintf("%s/%s",filter->vacation_directory,oncelog->d_name);
if (Ustat(s,&properties)==0 && (properties.st_mtime+VACATION_MAX_DAYS*86400)<now)
Uunlink(s);
}
- }
closedir(oncelogdir);
}
}
diff --git a/src/src/spool_mbox.c b/src/src/spool_mbox.c
index 7b6a796..8996f44 100644
--- a/src/src/spool_mbox.c
+++ b/src/src/spool_mbox.c
@@ -211,12 +211,11 @@ malware_ok = 0;
if (spool_mbox_ok && !f.no_mbox_unspool)
{
uschar *file_path;
- struct dirent *entry;
DIR *tempdir;
rmark reset_point = store_mark();
uschar * mbox_path = string_sprintf("%s/scan/%s", spool_directory, spooled_message_id);
- if (!(tempdir = opendir(CS mbox_path)))
+ if (!(tempdir = exim_opendir(mbox_path)))
{
debug_printf("Unable to opendir(%s): %s\n", mbox_path, strerror(errno));
/* Just in case we still can: */
@@ -224,7 +223,7 @@ if (spool_mbox_ok && !f.no_mbox_unspool)
return;
}
/* loop thru dir & delete entries */
- while((entry = readdir(tempdir)))
+ for (struct dirent *entry; entry = readdir(tempdir); )
{
uschar *name = US entry->d_name;
int dummy;
diff --git a/src/src/transports/appendfile.c b/src/src/transports/appendfile.c
index f4baf0c..426a8c1 100644
--- a/src/src/transports/appendfile.c
+++ b/src/src/transports/appendfile.c
@@ -714,14 +714,13 @@ check_dir_size(const uschar * dirname, int *countptr, const pcre *regex)
DIR *dir;
off_t sum = 0;
int count = *countptr;
-struct dirent *ent;
-struct stat statbuf;
-if (!(dir = opendir(CS dirname))) return 0;
+if (!(dir = exim_opendir(dirname))) return 0;
-while ((ent = readdir(dir)))
+for (struct dirent *ent; ent = readdir(dir); )
{
uschar * path, * name = US ent->d_name;
+ struct stat statbuf;
if (Ustrcmp(name, ".") == 0 || Ustrcmp(name, "..") == 0) continue;
diff --git a/src/src/transports/tf_maildir.c b/src/src/transports/tf_maildir.c
index 4d5c0c1..bcd091b 100644
--- a/src/src/transports/tf_maildir.c
+++ b/src/src/transports/tf_maildir.c
@@ -253,15 +253,14 @@ maildir_compute_size(uschar *path, int *filecount, time_t *latest,
{
DIR *dir;
off_t sum = 0;
-struct dirent *ent;
-struct stat statbuf;
-if (!(dir = opendir(CS path)))
+if (!(dir = exim_opendir(path)))
return 0;
-while ((ent = readdir(dir)))
+for (struct dirent *ent; ent = readdir(dir); )
{
uschar * s, * name = US ent->d_name;
+ struct stat statbuf;
if (Ustrcmp(name, ".") == 0 || Ustrcmp(name, "..") == 0) continue;
diff --git a/test/confs/2500 b/test/confs/2500
index 72bb374..0c5ee2f 100644
--- a/test/confs/2500
+++ b/test/confs/2500
@@ -6,4 +6,6 @@ primary_hostname = myhost.test.ex
# ----- Main settings -----
+acl_not_smtp = accept set acl_m0 = ${lookup {key} dsearch {DIR/$recipients}}
+
# End
diff --git a/test/log/2500 b/test/log/2500
new file mode 100644
index 0000000..4432783
--- /dev/null
+++ b/test/log/2500
@@ -0,0 +1,6 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 Tainted filename for search: 'TESTSUITE/tainted@???'
+1999-03-02 09:44:33 10HmaX-0005vi-00 F=<CALLER@???> rejected by non-SMTP ACL: failed to expand ACL string "accept set acl_m0 = ${lookup {key} dsearch {TESTSUITE/$recipients}}": NULL
+1999-03-02 09:44:33 10HmaY-0005vi-00 Tainted filename for search: 'TESTSUITE/CALLER@???'
+1999-03-02 09:44:33 10HmaY-0005vi-00 F=<> rejected by non-SMTP ACL: failed to expand ACL string "accept set acl_m0 = ${lookup {key} dsearch {TESTSUITE/$recipients}}": NULL
+1999-03-02 09:44:33 10HmaY-0005vi-00 Error while reading message with no usable sender address (R=10HmaX-0005vi-00): rejected by non-SMTP ACL: local configuration problem
+1999-03-02 09:44:33 10HmaX-0005vi-00 Child mail process returned status 1
diff --git a/test/paniclog/2500 b/test/paniclog/2500
new file mode 100644
index 0000000..2a988db
--- /dev/null
+++ b/test/paniclog/2500
@@ -0,0 +1,2 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 Tainted filename for search: 'TESTSUITE/tainted@???'
+1999-03-02 09:44:33 10HmaY-0005vi-00 Tainted filename for search: 'TESTSUITE/CALLER@???'
diff --git a/test/rejectlog/2500 b/test/rejectlog/2500
new file mode 100644
index 0000000..8c09f15
--- /dev/null
+++ b/test/rejectlog/2500
@@ -0,0 +1,23 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 F=<CALLER@???> rejected by non-SMTP ACL: failed to expand ACL string "accept set acl_m0 = ${lookup {key} dsearch {TESTSUITE/$recipients}}": NULL
+Envelope-from: <CALLER@???>
+Envelope-to: <tainted@???>
+P Received: from CALLER by myhost.test.ex with local (Exim x.yz)
+ (envelope-from <CALLER@???>)
+ id 10HmaX-0005vi-00
+ for tainted@???; Tue, 2 Mar 1999 09:44:33 +0000
+I Message-Id: <E10HmaX-0005vi-00@???>
+F From: CALLER_NAME <CALLER@???>
+ Date: Tue, 2 Mar 1999 09:44:33 +0000
+1999-03-02 09:44:33 10HmaY-0005vi-00 F=<> rejected by non-SMTP ACL: failed to expand ACL string "accept set acl_m0 = ${lookup {key} dsearch {TESTSUITE/$recipients}}": NULL
+Envelope-from: <>
+Envelope-to: <CALLER@???>
+P Received: from EXIMUSER by myhost.test.ex with local (Exim x.yz)
+ id 10HmaY-0005vi-00
+ for CALLER@???; Tue, 2 Mar 1999 09:44:33 +0000
+ Auto-Submitted: auto-replied
+F From: Mail Delivery System <Mailer-Daemon@???>
+T To: CALLER@???
+ References: <E10HmaX-0005vi-00@???>
+ Subject: Mail failure - rejected by local scanning code
+I Message-Id: <E10HmaY-0005vi-00@???>
+ Date: Tue, 2 Mar 1999 09:44:33 +0000
diff --git a/test/scripts/2500-dsearch/2500 b/test/scripts/2500-dsearch/2500
index 993c361..49e2a37 100644
--- a/test/scripts/2500-dsearch/2500
+++ b/test/scripts/2500-dsearch/2500
@@ -7,3 +7,7 @@ fail: ${lookup{TESTNUM.tst} dsearch{DIR/dir_not_here}{$value}{FAIL}}
fail(case): ${lookup{TESTNUM.TST} dsearch{DIR/aux-fixed}{$value}{FAIL}}
fail(case): ${lookup{TESTNUM.TST} dsearch{DIR/AUX-fixed}{$value}{FAIL}}
****
+#
+1
+exim tainted@???
+****
diff --git a/test/stderr/2500 b/test/stderr/2500
new file mode 100644
index 0000000..2a988db
--- /dev/null
+++ b/test/stderr/2500
@@ -0,0 +1,2 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 Tainted filename for search: 'TESTSUITE/tainted@???'
+1999-03-02 09:44:33 10HmaY-0005vi-00 Tainted filename for search: 'TESTSUITE/CALLER@???'
