Taint enforce: directory open backstops, single-key search filename
authorJeremy Harris <jgh146exb@wizmail.org>
Sat, 28 Mar 2020 20:01:10 +0000 (20:01 +0000)
committerJeremy Harris <jgh146exb@wizmail.org>
Sat, 28 Mar 2020 22:00:09 +0000 (22:00 +0000)
19 files changed:
doc/doc-docbook/spec.xfpt
src/exim_monitor/em_queue.c
src/src/dbfn.c
src/src/drtables.c
src/src/functions.h
src/src/lookups/dsearch.c
src/src/queue.c
src/src/receive.c
src/src/search.c
src/src/sieve.c
src/src/spool_mbox.c
src/src/transports/appendfile.c
src/src/transports/tf_maildir.c
test/confs/2500
test/log/2500 [new file with mode: 0644]
test/paniclog/2500 [new file with mode: 0644]
test/rejectlog/2500 [new file with mode: 0644]
test/scripts/2500-dsearch/2500
test/stderr/2500 [new file with mode: 0644]

index 2a3fb6c51c117a071871445b53256c24a9bc168d..8605fdc3ba73e8a2ad1a21b3e4fb8072464624ff 100644 (file)
@@ -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
index d4c01a628c70dcd75c5f93e3be95cfc641a863c6..7a441cb866eb8359a4649a695411300ff422444f 100644 (file)
@@ -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];
-  }
 }
 
 
index 1f058ef7236621305af047b42c8e9def9d900799..3aca1832650440aeed61e6abf16e27f7c60fe791 100644 (file)
@@ -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);
   }
index b1380df12fb13874edefb77d60bf86254aeb8c0b..f1053bbdae1cdacd775756d70ee25f3cd1054437 100644 (file)
@@ -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);
index 2e5b1e47d43048f623bdd65919abe3a7c6f67278..f789c5e2dee17098f8068efc974f304584ff14ef 100644 (file)
@@ -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;
 }
index 1eb2924f01cd6a1677c821c9924dbf38acf9f186..3a0df303b6fe124316788842397c1406420e414b 100644 (file)
@@ -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;
index bb75c99cd13c393006327958057d01b162ce91ab..e02ce3837a59613f1453f22e99765e60794ef8e2 100644 (file)
@@ -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);
index 0afb72b8cfd315c9a409f8dd38aea4904faa82fa..2d88d6476e355f489f2253c35b6e11edb6e5c5b4 100644 (file)
@@ -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);
index 9d1a10a5a00f2f8f45c9903fac2c69917cedf806..dc90f53d5c6ed23868264c19880ed40fc2b277fd 100644 (file)
@@ -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. */
index 4467665340030a48a6fee47290e98efc2a1cbfd3..bf82d5e4643d496854e1b7f97a0d487622138447 100644 (file)
@@ -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);
     }
   }
index 7b6a796079c535d519753eb38346eaa83a0a30bf..8996f443a16c5b33d938b414fa68dcc8a53a6997 100644 (file)
@@ -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;
index f4baf0c771406444e70331a218b2ee861213d333..426a8c1ed0cd8d3d6635e21a632614730f88a4be 100644 (file)
@@ -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;
 
index 4d5c0c1a9a1960c0aa42e0c2c34f7cb5b6edd689..bcd091b758b9922e07f51147884ee9591a4a3c1a 100644 (file)
@@ -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;
 
index 72bb374ec41cb126a0a301d63cd1ab4a12d6ba91..0c5ee2f740a11c5d269cfaac2c3c3345b041dc89 100644 (file)
@@ -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 (file)
index 0000000..4432783
--- /dev/null
@@ -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=<CALLER@myhost.test.ex> 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 (file)
index 0000000..2a988db
--- /dev/null
@@ -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 (file)
index 0000000..8c09f15
--- /dev/null
@@ -0,0 +1,23 @@
+1999-03-02 09:44:33 10HmaX-0005vi-00 F=<CALLER@myhost.test.ex> rejected by non-SMTP ACL: failed to expand ACL string "accept set acl_m0 =      ${lookup {key} dsearch {TESTSUITE/$recipients}}": NULL
+Envelope-from: <CALLER@myhost.test.ex>
+Envelope-to: <tainted@test.ex>
+P Received: from CALLER by myhost.test.ex with local (Exim x.yz)
+       (envelope-from <CALLER@myhost.test.ex>)
+       id 10HmaX-0005vi-00
+       for tainted@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
+I Message-Id: <E10HmaX-0005vi-00@myhost.test.ex>
+F From: CALLER_NAME <CALLER@myhost.test.ex>
+  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@myhost.test.ex>
+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 <Mailer-Daemon@myhost.test.ex>
+T To: CALLER@myhost.test.ex
+  References: <E10HmaX-0005vi-00@myhost.test.ex>
+  Subject: Mail failure - rejected by local scanning code
+I Message-Id: <E10HmaY-0005vi-00@myhost.test.ex>
+  Date: Tue, 2 Mar 1999 09:44:33 +0000
index 993c361c848d028d7a7d366acc6fd132bcca6efe..49e2a37619c70e82d366e34cf2040af6930ba8ba 100644 (file)
@@ -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 (file)
index 0000000..2a988db
--- /dev/null
@@ -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'