CVE-2020-28007: Link attack in Exim's log directory

We patch this vulnerability by opening (instead of just creating) the
log file in an unprivileged (exim) child process, and by passing this
file descriptor back to the privileged (root) parent process. The two
functions log_send_fd() and log_recv_fd() are inspired by OpenSSH's
functions mm_send_fd() and mm_receive_fd(); thanks!

This patch also fixes:

- a NULL-pointer dereference in usr1_handler() (this signal handler is
  installed before process_log_path is initialized);

- a file-descriptor leak in dmarc_write_history_file() (two return paths
  did not close history_file_fd).

Note: the use of log_open_as_exim() in dmarc_write_history_file() should
be fine because the documentation explicitly states "Make sure the
directory of this file is writable by the user exim runs as."

(cherry picked from commit 2502cc41d1d92c1413eca6a4ba035c21162662bd)
(cherry picked from commit 93e9a18fbf09deb59bd133986f4c89aeb2d2d86a)

4 files changed, 143 insertions, 94 deletions

diff --git a/src/src/dmarc.c b/src/src/dmarc.c
index 599ecc454..750f9197f 100644
--- a/src/src/dmarc.c
+++ b/src/src/dmarc.c
@@ -53,9 +53,9 @@ static dmarc_exim_p dmarc_policy_description[] = {
 };
 
 
-void 
+void
 dmarc_version_report(FILE *f)
-{ 
+{
 const char *implementation, *version;
 
 fprintf(f, "Library version: dmarc: Compile: %d.%d.%d.%d\n",
@@ -254,7 +254,7 @@ if (!dmarc_history_file)
   DEBUG(D_receive) debug_printf("DMARC history file not set\n");
   return DMARC_HIST_DISABLED;
   }
-history_file_fd = log_create(dmarc_history_file);
+history_file_fd = log_create_as_exim(dmarc_history_file);
 
 if (history_file_fd < 0)
   {
diff --git a/src/src/exim.c b/src/src/exim.c
index 0865d642d..a5f0bd578 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -233,18 +233,8 @@ int fd;
 
 os_restarting_signal(sig, usr1_handler);
 
-if ((fd = Uopen(process_log_path, O_APPEND|O_WRONLY, LOG_MODE)) < 0)
-  {
-  /* If we are already running as the Exim user, try to create it in the
-  current process (assuming spool_directory exists). Otherwise, if we are
-  root, do the creation in an exim:exim subprocess. */
-
-  int euid = geteuid();
-  if (euid == exim_uid)
-    fd = Uopen(process_log_path, O_CREAT|O_APPEND|O_WRONLY, LOG_MODE);
-  else if (euid == root_uid)
-    fd = log_create_as_exim(process_log_path);
-  }
+if (!process_log_path) return;
+fd = log_open_as_exim(process_log_path);
 
 /* If we are neither exim nor root, or if we failed to create the log file,
 give up. There is not much useful we can do with errors, since we don't want
diff --git a/src/src/functions.h b/src/src/functions.h
index 459a707a1..84eb87397 100644
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -316,8 +316,7 @@ extern int     ip_streamsocket(const uschar *, uschar **, int, host_item *);
 extern int     ipv6_nmtoa(int *, uschar *);
 
 extern uschar *local_part_quote(uschar *);
-extern int     log_create(uschar *);
-extern int     log_create_as_exim(uschar *);
+extern int     log_open_as_exim(uschar *);
 extern void    log_close_all(void);
 
 extern macro_item * macro_create(const uschar *, const uschar *, BOOL);
diff --git a/src/src/log.c b/src/src/log.c
index 6e35ff9a7..bb6902ea0 100644
--- a/src/src/log.c
+++ b/src/src/log.c
@@ -265,14 +265,19 @@ overwrite it temporarily if it is necessary to create the directory.
 Returns:       a file descriptor, or < 0 on failure (errno set)
 */
 
-int
-log_create(uschar *name)
+static int
+log_open_already_exim(uschar * const name)
 {
-int fd = Uopen(name,
-#ifdef O_CLOEXEC
-	O_CLOEXEC |
-#endif
-	O_CREAT|O_APPEND|O_WRONLY, LOG_MODE);
+int fd = -1;
+const int flags = O_WRONLY | O_APPEND | O_CREAT | O_NONBLOCK;
+
+if (geteuid() != exim_uid)
+  {
+  errno = EACCES;
+  return -1;
+  }
+
+fd = Uopen(name, flags, LOG_MODE);
 
 /* If creation failed, attempt to build a log directory in case that is the
 problem. */
@@ -286,11 +291,7 @@ if (fd < 0 && errno == ENOENT)
   DEBUG(D_any) debug_printf("%s log directory %s\n",
     created ? "created" : "failed to create", name);
   *lastslash = '/';
-  if (created) fd = Uopen(name,
-#ifdef O_CLOEXEC
-			O_CLOEXEC |
-#endif
-		       	O_CREAT|O_APPEND|O_WRONLY, LOG_MODE);
+  if (created) fd = Uopen(name, flags, LOG_MODE);
   }
 
 return fd;
@@ -298,6 +299,81 @@ return fd;
 
 
 
+/* Inspired by OpenSSH's mm_send_fd(). Thanks! */
+
+static int
+log_send_fd(const int sock, const int fd)
+{
+struct msghdr msg;
+union {
+  struct cmsghdr hdr;
+  char buf[CMSG_SPACE(sizeof(int))];
+} cmsgbuf;
+struct cmsghdr *cmsg;
+struct iovec vec;
+char ch = 'A';
+ssize_t n;
+
+memset(&msg, 0, sizeof(msg));
+memset(&cmsgbuf, 0, sizeof(cmsgbuf));
+msg.msg_control = &cmsgbuf.buf;
+msg.msg_controllen = sizeof(cmsgbuf.buf);
+
+cmsg = CMSG_FIRSTHDR(&msg);
+cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+cmsg->cmsg_level = SOL_SOCKET;
+cmsg->cmsg_type = SCM_RIGHTS;
+*(int *)CMSG_DATA(cmsg) = fd;
+
+vec.iov_base = &ch;
+vec.iov_len = 1;
+msg.msg_iov = &vec;
+msg.msg_iovlen = 1;
+
+while ((n = sendmsg(sock, &msg, 0)) == -1 && errno == EINTR);
+if (n != 1) return -1;
+return 0;
+}
+
+/* Inspired by OpenSSH's mm_receive_fd(). Thanks! */
+
+static int
+log_recv_fd(const int sock)
+{
+struct msghdr msg;
+union {
+  struct cmsghdr hdr;
+  char buf[CMSG_SPACE(sizeof(int))];
+} cmsgbuf;
+struct cmsghdr *cmsg;
+struct iovec vec;
+ssize_t n;
+char ch = '\0';
+int fd = -1;
+
+memset(&msg, 0, sizeof(msg));
+vec.iov_base = &ch;
+vec.iov_len = 1;
+msg.msg_iov = &vec;
+msg.msg_iovlen = 1;
+
+memset(&cmsgbuf, 0, sizeof(cmsgbuf));
+msg.msg_control = &cmsgbuf.buf;
+msg.msg_controllen = sizeof(cmsgbuf.buf);
+
+while ((n = recvmsg(sock, &msg, 0)) == -1 && errno == EINTR);
+if (n != 1 || ch != 'A') return -1;
+
+cmsg = CMSG_FIRSTHDR(&msg);
+if (cmsg == NULL) return -1;
+if (cmsg->cmsg_type != SCM_RIGHTS) return -1;
+fd = *(const int *)CMSG_DATA(cmsg);
+if (fd < 0) return -1;
+return fd;
+}
+
+
+
 /*************************************************
 *     Create a log file as the exim user         *
 *************************************************/
@@ -313,41 +389,60 @@ Returns:       a file descriptor, or < 0 on failure (errno set)
 */
 
 int
-log_create_as_exim(uschar *name)
+log_open_as_exim(uschar * const name)
 {
-pid_t pid = exim_fork(US"logfile-create");
-int status = 1;
 int fd = -1;
+const uid_t euid = geteuid();
 
-/* In the subprocess, change uid/gid and do the creation. Return 0 from the
-subprocess on success. If we don't check for setuid failures, then the file
-can be created as root, so vulnerabilities which cause setuid to fail mean
-that the Exim user can use symlinks to cause a file to be opened/created as
-root. We always open for append, so can't nuke existing content but it would
-still be Rather Bad. */
-
-if (pid == 0)
+if (euid == exim_uid)
   {
-  if (setgid(exim_gid) < 0)
-    die(US"exim: setgid for log-file creation failed, aborting",
-      US"Unexpected log failure, please try later");
-  if (setuid(exim_uid) < 0)
-    die(US"exim: setuid for log-file creation failed, aborting",
-      US"Unexpected log failure, please try later");
-  _exit((log_create(name) < 0)? 1 : 0);
+  fd = log_open_already_exim(name);
   }
+else if (euid == root_uid)
+  {
+  int sock[2];
+  if (socketpair(AF_UNIX, SOCK_STREAM, 0, sock) == 0)
+    {
+    const pid_t pid = exim_fork(US"logfile-open");
+    if (pid == 0)
+      {
+      (void)close(sock[0]);
+      if (setgroups(1, &exim_gid) != 0) _exit(EXIT_FAILURE);
+      if (setgid(exim_gid) != 0) _exit(EXIT_FAILURE);
+      if (setuid(exim_uid) != 0) _exit(EXIT_FAILURE);
+
+      if (getuid() != exim_uid || geteuid() != exim_uid) _exit(EXIT_FAILURE);
+      if (getgid() != exim_gid || getegid() != exim_gid) _exit(EXIT_FAILURE);
+
+      fd = log_open_already_exim(name);
+      if (fd < 0) _exit(EXIT_FAILURE);
+      if (log_send_fd(sock[1], fd) != 0) _exit(EXIT_FAILURE);
+      (void)close(sock[1]);
+      _exit(EXIT_SUCCESS);
+      }
 
-/* If we created a subprocess, wait for it. If it succeeded, try the open. */
-
-while (pid > 0 && waitpid(pid, &status, 0) != pid);
-if (status == 0) fd = Uopen(name,
-#ifdef O_CLOEXEC
-			O_CLOEXEC |
-#endif
-		       	O_APPEND|O_WRONLY, LOG_MODE);
+    (void)close(sock[1]);
+    if (pid > 0)
+      {
+      fd = log_recv_fd(sock[0]);
+      while (waitpid(pid, NULL, 0) == -1 && errno == EINTR);
+      }
+    (void)close(sock[0]);
+    }
+  }
 
-/* If we failed to create a subprocess, we are in a bad way. We return
-with fd still < 0, and errno set, letting the caller handle the error. */
+if (fd >= 0)
+  {
+  int flags;
+  flags = fcntl(fd, F_GETFD);
+  if (flags != -1) (void)fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
+  flags = fcntl(fd, F_GETFL);
+  if (flags != -1) (void)fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);
+  }
+else
+  {
+  errno = EACCES;
+  }
 
 return fd;
 }
@@ -462,52 +557,17 @@ if (!ok)
   die(US"exim: log file path too long: aborting",
       US"Logging failure; please try later");
 
-/* We now have the file name. Try to open an existing file. After a successful
-open, arrange for automatic closure on exec(), and then return. */
+/* We now have the file name. After a successful open, return. */
 
-*fd = Uopen(buffer,
-#ifdef O_CLOEXEC
-		O_CLOEXEC |
-#endif
-	       	O_APPEND|O_WRONLY, LOG_MODE);
+*fd = log_open_as_exim(buffer);
 
 if (*fd >= 0)
   {
-#ifndef O_CLOEXEC
-  (void)fcntl(*fd, F_SETFD, fcntl(*fd, F_GETFD) | FD_CLOEXEC);
-#endif
   return;
   }
 
-/* Open was not successful: try creating the file. If this is a root process,
-we must do the creating in a subprocess set to exim:exim in order to ensure
-that the file is created with the right ownership. Otherwise, there can be a
-race if another Exim process is trying to write to the log at the same time.
-The use of SIGUSR1 by the exiwhat utility can provoke a lot of simultaneous
-writing. */
-
 euid = geteuid();
 
-/* If we are already running as the Exim user (even if that user is root),
-we can go ahead and create in the current process. */
-
-if (euid == exim_uid) *fd = log_create(buffer);
-
-/* Otherwise, if we are root, do the creation in an exim:exim subprocess. If we
-are neither exim nor root, creation is not attempted. */
-
-else if (euid == root_uid) *fd = log_create_as_exim(buffer);
-
-/* If we now have an open file, set the close-on-exec flag and return. */
-
-if (*fd >= 0)
-  {
-#ifndef O_CLOEXEC
-  (void)fcntl(*fd, F_SETFD, fcntl(*fd, F_GETFD) | FD_CLOEXEC);
-#endif
-  return;
-  }
-
 /* Creation failed. There are some circumstances in which we get here when
 the effective uid is not root or exim, which is the problem. (For example, a
 non-setuid binary with log_arguments set, called in certain ways.) Rather than