Taint enforce: directory open backstops, single-key search filename

12 files changed, 77 insertions, 72 deletions

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)<now)
           Uunlink(s);
         }
-      }
     closedir(oncelogdir);
     }
   }
diff --git a/src/src/spool_mbox.c b/src/src/spool_mbox.c
index 7b6a79607..8996f443a 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 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;