diff options
Diffstat (limited to 'debian/patches/84_23-CVE-2020-28007-Link-attack-in-Exim-s-log-directory.patch')
| -rw-r--r-- | debian/patches/84_23-CVE-2020-28007-Link-attack-in-Exim-s-log-directory.patch | 542 |
1 files changed, 542 insertions, 0 deletions
diff --git a/debian/patches/84_23-CVE-2020-28007-Link-attack-in-Exim-s-log-directory.patch b/debian/patches/84_23-CVE-2020-28007-Link-attack-in-Exim-s-log-directory.patch new file mode 100644 index 0000000..211abe3 --- /dev/null +++ b/debian/patches/84_23-CVE-2020-28007-Link-attack-in-Exim-s-log-directory.patch @@ -0,0 +1,542 @@ +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 + |