CVE-2020-28008: Assorted attacks in Exim's spool directory
authorHeiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>
Sun, 14 Mar 2021 11:16:57 +0000 (12:16 +0100)
committerHeiko Schlittermann (HS12-RIPE) <hs@schlittermann.de>
Tue, 27 Apr 2021 22:40:35 +0000 (00:40 +0200)
We patch dbfn_open() by introducing two functions priv_drop_temp() and
priv_restore() (inspired by OpenSSH's functions temporarily_use_uid()
and restore_uid()), which temporarily drop and restore root privileges
thanks to seteuid(). This goes against Exim's developers' wishes ("Exim
(the project) doesn't trust seteuid to work reliably") but, to the best
of our knowledge, seteuid() works everywhere and is the only way to
securely fix dbfn_open().

(cherry picked from commit 18da59151dbafa89be61c63580bdb295db36e374)

doc/doc-txt/ChangeLog
src/src/dbfn.c
test/stderr/0275
test/stderr/0278
test/stderr/0386
test/stderr/0388
test/stderr/0402
test/stderr/0403
test/stderr/0404
test/stderr/0408
test/stderr/0487

index 98c1b05e26929715ccf60c5028e883b554888ac2..636fdf71e058ba74fa1bf5eff7892f2343b6ea84 100644 (file)
@@ -207,6 +207,9 @@ PP/11 Fix security issue in BDAT state confusion.
 
 HS/03 Die on "/../" in msglog file names
 
+QS/01 Creation of (database) files in $spool_dir: only uid=0 or the euid of
+      the Exim runtime user are allowed to create files.
+
 
 Exim version 4.94
 -----------------
index bbf20a1d5ba12ac602b7105b106142a0e2db35ae..dceb5f140395c001b4ee8c05da3ecdaee5bc2f0e 100644 (file)
@@ -60,6 +60,66 @@ log_write(0, LOG_MAIN, "Berkeley DB error: %s", msg);
 
 
 
+static enum {
+  PRIV_DROPPING, PRIV_DROPPED,
+  PRIV_RESTORING, PRIV_RESTORED
+} priv_state = PRIV_RESTORED;
+
+static uid_t priv_euid;
+static gid_t priv_egid;
+static gid_t priv_groups[EXIM_GROUPLIST_SIZE + 1];
+static int priv_ngroups;
+
+/* Inspired by OpenSSH's temporarily_use_uid(). Thanks! */
+
+static void
+priv_drop_temp(const uid_t temp_uid, const gid_t temp_gid)
+{
+if (priv_state != PRIV_RESTORED) _exit(EXIT_FAILURE);
+priv_state = PRIV_DROPPING;
+
+priv_euid = geteuid();
+if (priv_euid == root_uid)
+  {
+  priv_egid = getegid();
+  priv_ngroups = getgroups(nelem(priv_groups), priv_groups);
+  if (priv_ngroups < 0) _exit(EXIT_FAILURE);
+
+  if (priv_ngroups > 0 && setgroups(1, &temp_gid) != 0) _exit(EXIT_FAILURE);
+  if (setegid(temp_gid) != 0) _exit(EXIT_FAILURE);
+  if (seteuid(temp_uid) != 0) _exit(EXIT_FAILURE);
+
+  if (geteuid() != temp_uid) _exit(EXIT_FAILURE);
+  if (getegid() != temp_gid) _exit(EXIT_FAILURE);
+  }
+
+priv_state = PRIV_DROPPED;
+}
+
+/* Inspired by OpenSSH's restore_uid(). Thanks! */
+
+static void
+priv_restore(void)
+{
+if (priv_state != PRIV_DROPPED) _exit(EXIT_FAILURE);
+priv_state = PRIV_RESTORING;
+
+if (priv_euid == root_uid)
+  {
+  if (seteuid(priv_euid) != 0) _exit(EXIT_FAILURE);
+  if (setegid(priv_egid) != 0) _exit(EXIT_FAILURE);
+  if (priv_ngroups > 0 && setgroups(priv_ngroups, priv_groups) != 0) _exit(EXIT_FAILURE);
+
+  if (geteuid() != priv_euid) _exit(EXIT_FAILURE);
+  if (getegid() != priv_egid) _exit(EXIT_FAILURE);
+  }
+
+priv_state = PRIV_RESTORED;
+}
+
+
+
+
 /*************************************************
 *          Open and lock a database file         *
 *************************************************/
@@ -91,7 +151,6 @@ dbfn_open(uschar *name, int flags, open_db *dbblock, BOOL lof, BOOL panic)
 {
 int rc, save_errno;
 BOOL read_only = flags == O_RDONLY;
-BOOL created = FALSE;
 flock_t lock_data;
 uschar dirname[256], filename[256];
 
@@ -113,12 +172,13 @@ exists, there is no error. */
 snprintf(CS dirname, sizeof(dirname), "%s/db", spool_directory);
 snprintf(CS filename, sizeof(filename), "%s/%s.lockfile", dirname, name);
 
+priv_drop_temp(exim_uid, exim_gid);
 if ((dbblock->lockfd = Uopen(filename, O_RDWR, EXIMDB_LOCKFILE_MODE)) < 0)
   {
-  created = TRUE;
   (void)directory_make(spool_directory, US"db", EXIMDB_DIRECTORY_MODE, panic);
   dbblock->lockfd = Uopen(filename, O_RDWR|O_CREAT, EXIMDB_LOCKFILE_MODE);
   }
+priv_restore();
 
 if (dbblock->lockfd < 0)
   {
@@ -167,60 +227,17 @@ it easy to pin this down, there are now debug statements on either side of the
 open call. */
 
 snprintf(CS filename, sizeof(filename), "%s/%s", dirname, name);
-EXIM_DBOPEN(filename, dirname, flags, EXIMDB_MODE, &(dbblock->dbptr));
 
+priv_drop_temp(exim_uid, exim_gid);
+EXIM_DBOPEN(filename, dirname, flags, EXIMDB_MODE, &(dbblock->dbptr));
 if (!dbblock->dbptr && errno == ENOENT && flags == O_RDWR)
   {
   DEBUG(D_hints_lookup)
     debug_printf_indent("%s appears not to exist: trying to create\n", filename);
-  created = TRUE;
   EXIM_DBOPEN(filename, dirname, flags|O_CREAT, EXIMDB_MODE, &(dbblock->dbptr));
   }
-
 save_errno = errno;
-
-/* If we are running as root and this is the first access to the database, its
-files will be owned by root. We want them to be owned by exim. We detect this
-situation by noting above when we had to create the lock file or the database
-itself. Because the different dbm libraries use different extensions for their
-files, I don't know of any easier way of arranging this than scanning the
-directory for files with the appropriate base name. At least this deals with
-the lock file at the same time. Also, the directory will typically have only
-half a dozen files, so the scan will be quick.
-
-This code is placed here, before the test for successful opening, because there
-was a case when a file was created, but the DBM library still returned NULL
-because of some problem. It also sorts out the lock file if that was created
-but creation of the database file failed. */
-
-if (created && geteuid() == root_uid)
-  {
-  DIR * dd;
-  uschar *lastname = Ustrrchr(filename, '/') + 1;
-  int namelen = Ustrlen(name);
-
-  *lastname = 0;
-
-  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);
-  }
+priv_restore();
 
 /* If the open has failed, return NULL, leaving errno set. If lof is TRUE,
 log the event - also for debugging - but debug only if the file just doesn't
index 35f1e4442c095b340ccdf4061615250f50b9d7b9..47c9c9e2870910950e86093fd38d9c548f5541ad 100644 (file)
@@ -172,7 +172,7 @@ Delivery address list:
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: userx@test.ex
index 23b0ba9c66efad6e2bc284ed92b4eed882a04cc5..c09920b94a5b58f144b7d65e1dbb30c1ae3b87eb 100644 (file)
@@ -131,7 +131,7 @@ Delivery address list:
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: CALLER@test.ex
index b136548b7bb41488aef1c859611733960a05436c..7bc56d9b7619fff95409e473dd49805ea2d7576e 100644 (file)
@@ -273,7 +273,7 @@ Delivery address list:
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: 2@b
index 468eae32d10dcc5122b78ca2e7e9c9a4911efa7e..7819fe907a28a36beeecb75959f22bfb5f5eb5ef 100644 (file)
@@ -11,7 +11,7 @@ set_process_info: pppp delivering 10HmaX-0005vi-00
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: x@y
index 1f6bf19c782701f0a9169680c70c0c75397b3f4a..ec4b9ea725ececa78e1146fa35f280c5feba01bf 100644 (file)
@@ -214,7 +214,7 @@ Delivery address list:
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: CALLER@test.ex
index 0c22cf8f598c5cb185c3a09cdaacccdad9be1a83..9d759694c3e6b0ad836cc6f6fd9269977f0ceb94 100644 (file)
@@ -71,7 +71,7 @@ Delivery address list:
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: userx@test.ex
index b1a78c9fa97cc276d750fe8a0438ff3148b93155..f0c1c355b445364683385ed7b2e20080a1ab2f8a 100644 (file)
@@ -172,7 +172,7 @@ Delivery address list:
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: userx@test.ex
index 3fbd0f2a6991a03004a2aef18a305fed5a5609ad..50b85318343a03cb6c42f121b1b09831e0018fcc 100644 (file)
@@ -71,7 +71,7 @@ Delivery address list:
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: userx@test.ex
index 83d243a54f552822fb46b66dfa9279eaadf6ad8f..579b1bd5b8b034d59fc593c53fc221fe92c7fd63 100644 (file)
@@ -99,7 +99,7 @@ Delivery address list:
  EXIM_DBOPEN: file <TESTSUITE/spool/db/retry> dir <TESTSUITE/spool/db> flags=O_RDONLY
  returned from EXIM_DBOPEN: (nil)
  ensuring TESTSUITE/spool/db/retry.lockfile is owned by exim
- failed to open DB file TESTSUITE/spool/db/retry.lockfile: No such file or directory
+ failed to open DB file TESTSUITE/spool/db/retry: No such file or directory
 no retry data available
 >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 Considering: userx@test.ex