From: Qualys Security Advisory Date: Tue, 23 Feb 2021 16:33:03 +0000 (-0800) Subject: CVE-2020-28007: Link attack in Exim's log directory X-Git-Tag: exim-4.95-RC0~51^2~6 X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/b6c1434e4765d1a53efa2f3046bfb20ba682b5d2 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) --- 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 diff --git a/test/stderr/0397 b/test/stderr/0397 index 82f143787..4d9dcaa47 100644 --- a/test/stderr/0397 +++ b/test/stderr/0397 @@ -1,7 +1,7 @@ -1999-03-02 09:44:33 Cannot open main log file "/non/existent/path/to/force/failure/main": No such file or directory: euid=uuuu egid=EXIM_GID +1999-03-02 09:44:33 Cannot open main log file "/non/existent/path/to/force/failure/main": Permission denied: euid=uuuu egid=EXIM_GID 1999-03-02 09:44:33 Start queue run: pid=pppp -1999-03-02 09:44:33 Cannot open main log file "/non/existent/path/to/force/failure/main": No such file or directory: euid=uuuu egid=EXIM_GID +1999-03-02 09:44:33 Cannot open main log file "/non/existent/path/to/force/failure/main": Permission denied: euid=uuuu egid=EXIM_GID SYSLOG: '2017-07-30 18:51:05 Start queue run: pid=pppp' -SYSLOG: '2017-07-30 18:51:05 Cannot open main log file "/non/existent/path/to/force/failure/main": No such file or directory: euid=uuuu egid=EXIM_GID' +SYSLOG: '2017-07-30 18:51:05 Cannot open main log file "/non/existent/path/to/force/failure/main": Permission denied: euid=uuuu egid=EXIM_GID' SYSLOG: 'exim: could not open panic log - aborting: see message(s) above' exim: could not open panic log - aborting: see message(s) above