[exim-cvs] CVE-2020-28014, CVE-2021-27216: PID file handling

Gitweb: https://git.exim.org/exim.git/commitdiff/c7f4ea442a264b5cb3a9ef0eed641f4778dfb5b7

Commit:     c7f4ea442a264b5cb3a9ef0eed641f4778dfb5b7
Parent:     84dcbc72b968ebc666387874171580463f1944dd
Author:     Heiko Schlittermann (HS12-RIPE) <hs@???>
AuthorDate: Thu Mar 25 22:48:09 2021 +0100
Committer:  Heiko Schlittermann (HS12-RIPE) <hs@???>
CommitDate: Thu May 27 21:30:43 2021 +0200

    CVE-2020-28014, CVE-2021-27216: PID file handling

    Arbitrary PID file creation, clobbering, and deletion.
    Patch provided by Qualys.

    (cherry picked from commit 974f32939a922512b27d9f0a8a1cb5dec60e7d37)
    (cherry picked from commit 43c6f0b83200b7082353c50187ef75de3704580a)
---
 doc/doc-txt/ChangeLog |   5 +-
 src/src/daemon.c      | 169 ++++++++++++++++++++++++++++++++++++++++----------
 src/src/exim.c        |   4 ++
 test/stderr/0433      |  18 ++----
 4 files changed, 149 insertions(+), 47 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 4debef8..adf43bc 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -296,9 +296,12 @@ 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
+QS/01 Creation of (database) files in $spool_dir: only uid=0 or the uid of
       the Exim runtime user are allowed to create files.

+QS/02 PID file creation/deletion: only possible if uid=0 or uid is the Exim
+      runtime user.
+

Exim version 4.94
-----------------
diff --git a/src/src/daemon.c b/src/src/daemon.c
index ed7d30a..f40a58c 100644
--- a/src/src/daemon.c
+++ b/src/src/daemon.c
@@ -932,7 +932,6 @@ while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
}


-
static void
set_pid_file_path(void)
{
@@ -947,34 +946,144 @@ if (pid_file_path[0] != '/')
}


-/* Remove the daemon's pidfile. Note: runs with root privilege,
-as a direct child of the daemon. Does not return. */
+enum pid_op { PID_WRITE, PID_CHECK, PID_DELETE };

-void
-delete_pid_file(void)
+/* Do various pid file operations as safe as possible. Ideally we'd just
+drop the privileges for creation of the pid file and not care at all about removal of
+the file. FIXME.
+Returns: true on success, false + errno==EACCES otherwise
+*/
+static BOOL
+operate_on_pid_file(const enum pid_op operation, const pid_t pid)
{
-uschar * daemon_pid = string_sprintf("%d\n", (int)getppid());
-FILE * f;
+char pid_line[sizeof(int) * 3 + 2];
+const int pid_len = snprintf(pid_line, sizeof(pid_line), "%d\n", (int)pid);
+BOOL lines_match = FALSE;
+
+char * path = NULL;
+char * base = NULL;
+char * dir = NULL;
+
+const int dir_flags = O_RDONLY | O_NONBLOCK;
+const int base_flags = O_NOFOLLOW | O_NONBLOCK;
+const mode_t base_mode = 0644;
+struct stat sb;
+
+int cwd_fd = -1;
+int dir_fd = -1;
+int base_fd = -1;
+
+BOOL success = FALSE;
+errno = EACCES;

 set_pid_file_path();
-if ((f = Ufopen(pid_file_path, "rb")))
+if (!f.running_in_test_harness && real_uid != root_uid && real_uid != exim_uid) goto cleanup;
+if (pid_len < 2 || pid_len >= (int)sizeof(pid_line)) goto cleanup;
+
+path = CS string_copy(pid_file_path);
+if ((base = Ustrrchr(path, '/')) == NULL) /* should not happen, but who knows */
+  log_write(0, LOG_MAIN|LOG_PANIC_DIE, "pid file path \"%s\" does not contain a '/'", pid_file_path);
+
+dir = (base != path) ? path : "/";
+*base++ = '\0';
+
+if (!dir || !*dir || *dir != '/') goto cleanup;
+if (!base || !*base || strchr(base, '/') != NULL) goto cleanup;
+
+cwd_fd = open(".", dir_flags);
+if (cwd_fd < 0 || fstat(cwd_fd, &sb) != 0 || !S_ISDIR(sb.st_mode)) goto cleanup;
+dir_fd = open(dir, dir_flags);
+if (dir_fd < 0 || fstat(dir_fd, &sb) != 0 || !S_ISDIR(sb.st_mode)) goto cleanup;
+
+/* emulate openat */
+if (fchdir(dir_fd) != 0) goto cleanup;
+base_fd = open(base, O_RDONLY | base_flags);
+if (fchdir(cwd_fd) != 0)
+  log_write(0, LOG_MAIN|LOG_PANIC_DIE, "can't return to previous working dir: %s", strerror(errno));
+
+if (base_fd >= 0)
   {
-  if (  fgets(CS big_buffer, big_buffer_size, f)
-    && Ustrcmp(daemon_pid, big_buffer) == 0
-     )
-    if (Uunlink(pid_file_path) == 0)
+  char line[sizeof(pid_line)];
+  ssize_t len = -1;
+
+  if (fstat(base_fd, &sb) != 0 || !S_ISREG(sb.st_mode)) goto cleanup;
+  if ((sb.st_mode & 07777) != base_mode || sb.st_nlink != 1) goto cleanup;
+  if (sb.st_size < 2 || sb.st_size >= (off_t)sizeof(line)) goto cleanup;
+
+  len = read(base_fd, line, sizeof(line));
+  if (len != (ssize_t)sb.st_size) goto cleanup;
+  line[len] = '\0';
+
+  if (strspn(line, "0123456789") != (size_t)len-1) goto cleanup;
+  if (line[len-1] != '\n') goto cleanup;
+  lines_match = (len == pid_len && strcmp(line, pid_line) == 0);
+  }
+
+if (operation == PID_WRITE)
+  {
+  if (!lines_match)
+    {
+    if (base_fd >= 0)
       {
-      DEBUG(D_any)
-    debug_printf("%s unlink: %s\n", pid_file_path, strerror(errno));
-      }
-    else
-      DEBUG(D_any)
-    debug_printf("unlinked %s\n", pid_file_path);
-  fclose(f);
+      int error = -1;
+      /* emulate unlinkat */
+      if (fchdir(dir_fd) != 0) goto cleanup;
+      error = unlink(base);
+      if (fchdir(cwd_fd) != 0)
+        log_write(0, LOG_MAIN|LOG_PANIC_DIE, "can't return to previous working dir: %s", strerror(errno));
+      if (error) goto cleanup;
+      (void)close(base_fd);
+      base_fd = -1;
+     }
+    /* emulate openat */
+    if (fchdir(dir_fd) != 0) goto cleanup;
+    base_fd = open(base, O_WRONLY | O_CREAT | O_EXCL | base_flags, base_mode);
+    if (fchdir(cwd_fd) != 0)
+        log_write(0, LOG_MAIN|LOG_PANIC_DIE, "can't return to previous working dir: %s", strerror(errno));
+    if (base_fd < 0) goto cleanup;
+    if (fchmod(base_fd, base_mode) != 0) goto cleanup;
+    if (write(base_fd, pid_line, pid_len) != pid_len) goto cleanup;
+    DEBUG(D_any) debug_printf("pid written to %s\n", pid_file_path);
+    }
   }
 else
-  DEBUG(D_any)
-    debug_printf("%s\n", string_open_failed("pid file %s", pid_file_path));
+  {
+  if (!lines_match) goto cleanup;
+  if (operation == PID_DELETE)
+    {
+    int error = -1;
+    /* emulate unlinkat */
+    if (fchdir(dir_fd) != 0) goto cleanup;
+    error = unlink(base);
+    if (fchdir(cwd_fd) != 0)
+        log_write(0, LOG_MAIN|LOG_PANIC_DIE, "can't return to previous working dir: %s", strerror(errno));
+    if (error) goto cleanup;
+    }
+  }
+
+success = TRUE;
+errno = 0;
+
+cleanup:
+if (cwd_fd >= 0) (void)close(cwd_fd);
+if (dir_fd >= 0) (void)close(dir_fd);
+if (base_fd >= 0) (void)close(base_fd);
+return success;
+}
+
+
+/* Remove the daemon's pidfile.  Note: runs with root privilege,
+as a direct child of the daemon.  Does not return. */
+
+void
+delete_pid_file(void)
+{
+const BOOL success = operate_on_pid_file(PID_DELETE, getppid());
+
+DEBUG(D_any)
+  debug_printf("delete pid file %s %s: %s\n", pid_file_path,
+    success ? "success" : "failure", strerror(errno));
+
 exim_exit(EXIT_SUCCESS);
 }

@@ -1841,22 +1950,14 @@ The variable daemon_write_pid is used to control this. */

 if (f.running_in_test_harness || write_pid)
   {
-  FILE *f;
-
-  set_pid_file_path();
-  if ((f = modefopen(pid_file_path, "wb", 0644)))
-    {
-    (void)fprintf(f, "%d\n", (int)getpid());
-    (void)fclose(f);
-    DEBUG(D_any) debug_printf("pid written to %s\n", pid_file_path);
-    }
-  else
-    DEBUG(D_any)
-      debug_printf("%s\n", string_open_failed("pid file %s", pid_file_path));
+  const enum pid_op operation = (f.running_in_test_harness
+     || real_uid == root_uid
+     || (real_uid == exim_uid && !override_pid_file_path)) ? PID_WRITE : PID_CHECK;
+  if (!operate_on_pid_file(operation, getpid()))
+    DEBUG(D_any) debug_printf("%s pid file %s: %s\n", (operation == PID_WRITE) ? "write" : "check", pid_file_path, strerror(errno));
   }

/* Set up the handler for SIGHUP, which causes a restart of the daemon. */
-
sighup_seen = FALSE;
signal(SIGHUP, sighup_handler);

diff --git a/src/src/exim.c b/src/src/exim.c
index abb3ba7..49ba9e7 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -3256,6 +3256,10 @@ on the second character (the one after '-'), to save some effort. */
      -oPX:       delete pid file of daemon */

       case 'P':
+    if (!f.running_in_test_harness && real_uid != root_uid && real_uid != exim_uid)
+      exim_fail("exim: only uid=%d or uid=%d can use -oP and -oPX "
+                    "(uid=%d euid=%d | %d)\n",
+                    root_uid, exim_uid, getuid(), geteuid(), real_uid);
     if (!*argrest) override_pid_file_path = argv[++i];
     else if (Ustrcmp(argrest, "X") == 0) delete_pid_file();
     else badarg = TRUE;
diff --git a/test/stderr/0433 b/test/stderr/0433
index b367883..3d0e9e9 100644
--- a/test/stderr/0433
+++ b/test/stderr/0433
@@ -18,9 +18,8 @@ LOG: MAIN
 set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on port 1225
 daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID
 Listening...
-SIGTERM seen
-pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
 SIGTERM/SIGINT seen
+pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
 search_tidyup called

>>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>>
Exim version x.yz ....
@@ -42,9 +41,8 @@ LOG: MAIN
set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on port 1225 port 1226
daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID
Listening...
-SIGTERM seen
-pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
SIGTERM/SIGINT seen
+pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
search_tidyup called
>>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>>
Exim version x.yz ....
@@ -67,9 +65,8 @@ LOG: MAIN
set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on [127.0.0.1]:1228 port 1225 (IPv4) port 1226 (IPv4)
daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID
Listening...
-SIGTERM seen
-pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 127.0.0.1.1228 ; 0.0.0.0 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
SIGTERM/SIGINT seen
+pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 127.0.0.1.1228 ; 0.0.0.0 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
search_tidyup called
>>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>>
Exim version x.yz ....
@@ -92,9 +89,8 @@ LOG: MAIN
set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on port 1225 port 1226 [127.0.0.1]:1228
daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID
Listening...
-SIGTERM seen
-pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 0.0.0.0; 127.0.0.1.1228 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
SIGTERM/SIGINT seen
+pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 0.0.0.0; 127.0.0.1.1228 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
search_tidyup called
>>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>>
Exim version x.yz ....
@@ -118,9 +114,8 @@ LOG: MAIN
set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on [127.0.0.1]:1228 port 1227 (IPv4)
daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID
Listening...
-SIGTERM seen
-pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 127.0.0.1.1228 ; 0.0.0.0 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
SIGTERM/SIGINT seen
+pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 127.0.0.1.1228 ; 0.0.0.0 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
search_tidyup called
>>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>>
Exim version x.yz ....
@@ -144,8 +139,7 @@ LOG: MAIN
set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on port 1225 port 1226
daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID
Listening...
-SIGTERM seen
-pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
SIGTERM/SIGINT seen
+pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX
search_tidyup called
>>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>>