From 54a2a2a9983913a91ccef3aac107a159434a4714 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sat, 28 Mar 2020 20:01:10 +0000 Subject: [PATCH] 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(-) create mode 100644 test/log/2500 create mode 100644 test/paniclog/2500 create mode 100644 test/rejectlog/2500 create mode 100644 test/stderr/2500 diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index 2a3fb6c51..8605fdc3b 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 d4c01a628..7a441cb86 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 1f058ef72..3aca18326 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 b1380df12..f1053bbda 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 2e5b1e47d..f789c5e2d 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 1eb2924f0..3a0df303b 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 bb75c99cd..e02ce3837 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 0afb72b8c..2d88d6476 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 9d1a10a5a..dc90f53d5 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 446766534..bf82d5e46 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)d_name; int dummy; diff --git a/src/src/transports/appendfile.c b/src/src/transports/appendfile.c index f4baf0c77..426a8c1ed 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 4d5c0c1a9..bcd091b75 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 72bb374ec..0c5ee2f74 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 000000000..4432783ad --- /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@test.ex' +1999-03-02 09:44:33 10HmaX-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 Tainted filename for search: 'TESTSUITE/CALLER@myhost.test.ex' +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 000000000..2a988dbaa --- /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@test.ex' +1999-03-02 09:44:33 10HmaY-0005vi-00 Tainted filename for search: 'TESTSUITE/CALLER@myhost.test.ex' diff --git a/test/rejectlog/2500 b/test/rejectlog/2500 new file mode 100644 index 000000000..8c09f1568 --- /dev/null +++ b/test/rejectlog/2500 @@ -0,0 +1,23 @@ +1999-03-02 09:44:33 10HmaX-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: +P Received: from CALLER by myhost.test.ex with local (Exim x.yz) + (envelope-from ) + id 10HmaX-0005vi-00 + for tainted@test.ex; Tue, 2 Mar 1999 09:44:33 +0000 +I Message-Id: +F From: CALLER_NAME + 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: +P Received: from EXIMUSER by myhost.test.ex with local (Exim x.yz) + id 10HmaY-0005vi-00 + for CALLER@myhost.test.ex; Tue, 2 Mar 1999 09:44:33 +0000 + Auto-Submitted: auto-replied +F From: Mail Delivery System +T To: CALLER@myhost.test.ex + References: + Subject: Mail failure - rejected by local scanning code +I Message-Id: + Date: Tue, 2 Mar 1999 09:44:33 +0000 diff --git a/test/scripts/2500-dsearch/2500 b/test/scripts/2500-dsearch/2500 index 993c361c8..49e2a3761 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@test.ex +**** diff --git a/test/stderr/2500 b/test/stderr/2500 new file mode 100644 index 000000000..2a988dbaa --- /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@test.ex' +1999-03-02 09:44:33 10HmaY-0005vi-00 Tainted filename for search: 'TESTSUITE/CALLER@myhost.test.ex' -- 2.25.1