--- /dev/null
+From 99ae249e9857e80ff4d65b2388bc68c624dcb739 Mon Sep 17 00:00:00 2001
+From: Qualys Security Advisory <qsa@qualys.com>
+Date: Tue, 23 Feb 2021 08:33:03 -0800
+Subject: [PATCH 23/29] 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)
+---
+ src/dmarc.c | 179 ++++++++++++++++++------------------
+ src/exim.c | 14 +--
+ src/functions.h | 3 +-
+ src/log.c | 214 ++++++++++++++++++++++++++++----------------
+ test/stderr/0397 | 6 +-
+ 5 files changed, 234 insertions(+), 182 deletions(-)
+
+diff --git a/src/dmarc.c b/src/dmarc.c
+index f29f7eba6..c5e01c7ee 100644
+--- a/src/dmarc.c
++++ b/src/dmarc.c
+@@ -204,6 +204,97 @@ if ( dmarc_policy == DMARC_POLICY_REJECT && action == DMARC_RESULT_REJECT
+ }
+ }
+
++
++static int
++dmarc_write_history_file()
++{
++int tmp_ans;
++u_char **rua; /* aggregate report addressees */
++uschar *history_buffer = NULL;
++
++if (!dmarc_history_file)
++ {
++ DEBUG(D_receive) debug_printf("DMARC history file not set\n");
++ return DMARC_HIST_DISABLED;
++ }
++
++/* Generate the contents of the history file */
++history_buffer = string_sprintf(
++ "job %s\nreporter %s\nreceived %ld\nipaddr %s\nfrom %s\nmfrom %s\n",
++ message_id, primary_hostname, time(NULL), sender_host_address,
++ header_from_sender, expand_string(US"$sender_address_domain"));
++
++if (spf_response)
++ history_buffer = string_sprintf("%sspf %d\n", history_buffer, dmarc_spf_ares_result);
++ /* history_buffer = string_sprintf("%sspf -1\n", history_buffer); */
++
++history_buffer = string_sprintf(
++ "%s%spdomain %s\npolicy %d\n",
++ history_buffer, dkim_history_buffer, dmarc_used_domain, dmarc_policy);
++
++if ((rua = opendmarc_policy_fetch_rua(dmarc_pctx, NULL, 0, 1)))
++ for (tmp_ans = 0; rua[tmp_ans]; tmp_ans++)
++ history_buffer = string_sprintf("%srua %s\n", history_buffer, rua[tmp_ans]);
++else
++ history_buffer = string_sprintf("%srua -\n", history_buffer);
++
++opendmarc_policy_fetch_pct(dmarc_pctx, &tmp_ans);
++history_buffer = string_sprintf("%spct %d\n", history_buffer, tmp_ans);
++
++opendmarc_policy_fetch_adkim(dmarc_pctx, &tmp_ans);
++history_buffer = string_sprintf("%sadkim %d\n", history_buffer, tmp_ans);
++
++opendmarc_policy_fetch_aspf(dmarc_pctx, &tmp_ans);
++history_buffer = string_sprintf("%saspf %d\n", history_buffer, tmp_ans);
++
++opendmarc_policy_fetch_p(dmarc_pctx, &tmp_ans);
++history_buffer = string_sprintf("%sp %d\n", history_buffer, tmp_ans);
++
++opendmarc_policy_fetch_sp(dmarc_pctx, &tmp_ans);
++history_buffer = string_sprintf("%ssp %d\n", history_buffer, tmp_ans);
++
++history_buffer = string_sprintf(
++ "%salign_dkim %d\nalign_spf %d\naction %d\n",
++ history_buffer, da, sa, action);
++
++/* Write the contents to the history file */
++DEBUG(D_receive)
++ debug_printf("DMARC logging history data for opendmarc reporting%s\n",
++ (host_checking || f.running_in_test_harness) ? " (not really)" : "");
++if (host_checking || f.running_in_test_harness)
++ {
++ DEBUG(D_receive)
++ debug_printf("DMARC history data for debugging:\n%s", history_buffer);
++ }
++else
++ {
++ ssize_t written_len;
++ const int history_file_fd = log_open_as_exim(dmarc_history_file);
++
++ if (history_file_fd < 0)
++ {
++ log_write(0, LOG_MAIN|LOG_PANIC, "failure to create DMARC history file: %s",
++ dmarc_history_file);
++ return DMARC_HIST_FILE_ERR;
++ }
++
++ written_len = write_to_fd_buf(history_file_fd,
++ history_buffer,
++ Ustrlen(history_buffer));
++
++ (void)close(history_file_fd);
++
++ if (written_len <= 0)
++ {
++ log_write(0, LOG_MAIN|LOG_PANIC, "failure to write to DMARC history file: %s",
++ dmarc_history_file);
++ return DMARC_HIST_WRITE_ERR;
++ }
++ }
++return DMARC_HIST_OK;
++}
++
++
+ /* dmarc_process adds the envelope sender address to the existing
+ context (if any), retrieves the result, sets up expansion
+ strings and evaluates the condition outcome. */
+@@ -486,94 +577,6 @@ if (!f.dmarc_disable_verify)
+ return OK;
+ }
+
+-static int
+-dmarc_write_history_file()
+-{
+-int history_file_fd;
+-ssize_t written_len;
+-int tmp_ans;
+-u_char **rua; /* aggregate report addressees */
+-uschar *history_buffer = NULL;
+-
+-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);
+-
+-if (history_file_fd < 0)
+- {
+- log_write(0, LOG_MAIN|LOG_PANIC, "failure to create DMARC history file: %s",
+- dmarc_history_file);
+- return DMARC_HIST_FILE_ERR;
+- }
+-
+-/* Generate the contents of the history file */
+-history_buffer = string_sprintf(
+- "job %s\nreporter %s\nreceived %ld\nipaddr %s\nfrom %s\nmfrom %s\n",
+- message_id, primary_hostname, time(NULL), sender_host_address,
+- header_from_sender, expand_string(US"$sender_address_domain"));
+-
+-if (spf_response)
+- history_buffer = string_sprintf("%sspf %d\n", history_buffer, dmarc_spf_ares_result);
+- /* history_buffer = string_sprintf("%sspf -1\n", history_buffer); */
+-
+-history_buffer = string_sprintf(
+- "%s%spdomain %s\npolicy %d\n",
+- history_buffer, dkim_history_buffer, dmarc_used_domain, dmarc_policy);
+-
+-if ((rua = opendmarc_policy_fetch_rua(dmarc_pctx, NULL, 0, 1)))
+- for (tmp_ans = 0; rua[tmp_ans]; tmp_ans++)
+- history_buffer = string_sprintf("%srua %s\n", history_buffer, rua[tmp_ans]);
+-else
+- history_buffer = string_sprintf("%srua -\n", history_buffer);
+-
+-opendmarc_policy_fetch_pct(dmarc_pctx, &tmp_ans);
+-history_buffer = string_sprintf("%spct %d\n", history_buffer, tmp_ans);
+-
+-opendmarc_policy_fetch_adkim(dmarc_pctx, &tmp_ans);
+-history_buffer = string_sprintf("%sadkim %d\n", history_buffer, tmp_ans);
+-
+-opendmarc_policy_fetch_aspf(dmarc_pctx, &tmp_ans);
+-history_buffer = string_sprintf("%saspf %d\n", history_buffer, tmp_ans);
+-
+-opendmarc_policy_fetch_p(dmarc_pctx, &tmp_ans);
+-history_buffer = string_sprintf("%sp %d\n", history_buffer, tmp_ans);
+-
+-opendmarc_policy_fetch_sp(dmarc_pctx, &tmp_ans);
+-history_buffer = string_sprintf("%ssp %d\n", history_buffer, tmp_ans);
+-
+-history_buffer = string_sprintf(
+- "%salign_dkim %d\nalign_spf %d\naction %d\n",
+- history_buffer, da, sa, action);
+-
+-/* Write the contents to the history file */
+-DEBUG(D_receive)
+- debug_printf("DMARC logging history data for opendmarc reporting%s\n",
+- (host_checking || f.running_in_test_harness) ? " (not really)" : "");
+-if (host_checking || f.running_in_test_harness)
+- {
+- DEBUG(D_receive)
+- debug_printf("DMARC history data for debugging:\n%s", history_buffer);
+- }
+-else
+- {
+- written_len = write_to_fd_buf(history_file_fd,
+- history_buffer,
+- Ustrlen(history_buffer));
+- if (written_len == 0)
+- {
+- log_write(0, LOG_MAIN|LOG_PANIC, "failure to write to DMARC history file: %s",
+- dmarc_history_file);
+- return DMARC_HIST_WRITE_ERR;
+- }
+- (void)close(history_file_fd);
+- }
+-return DMARC_HIST_OK;
+-}
+-
+-
+ uschar *
+ dmarc_exim_expand_query(int what)
+ {
+diff --git a/src/exim.c b/src/exim.c
+index a7dc48c4e..f0a168983 100644
+--- a/src/exim.c
++++ b/src/exim.c
+@@ -227,18 +227,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/functions.h b/src/functions.h
+index cab7a7363..366cb2f26 100644
+--- a/src/functions.h
++++ b/src/functions.h
+@@ -281,8 +281,7 @@ extern int ip_streamsocket(const uschar *, uschar **, int);
+ 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/log.c b/src/log.c
+index c8313890e..15c88c13e 100644
+--- a/src/log.c
++++ b/src/log.c
+@@ -264,14 +264,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. */
+@@ -285,11 +290,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;
+@@ -297,6 +298,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 *
+ *************************************************/
+@@ -312,41 +388,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 = fork();
+-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 = fork();
++ 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;
+ }
+@@ -459,52 +554,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
+--
+2.30.2
+
HCoop / hcoop / debian / exim4.git / blobdiff