Gitweb:
https://git.exim.org/exim.git/commitdiff/b6c1434e4765d1a53efa2f3046bfb20ba682b5d2
Commit: b6c1434e4765d1a53efa2f3046bfb20ba682b5d2
Parent: f83d4a2b3fedd9a8a0e7367db82a68a719f08e30
Author: Qualys Security Advisory <qsa@???>
AuthorDate: Tue Feb 23 08:33:03 2021 -0800
Committer: Heiko Schlittermann (HS12-RIPE) <hs@???>
CommitDate: Thu May 27 21:30:58 2021 +0200
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/src/dmarc.c | 6 +-
src/src/exim.c | 14 +---
src/src/functions.h | 3 +-
src/src/log.c | 214 +++++++++++++++++++++++++++++++++-------------------
test/stderr/0397 | 6 +-
5 files changed, 146 insertions(+), 97 deletions(-)
diff --git a/src/src/dmarc.c b/src/src/dmarc.c
index 599ecc4..750f919 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 0865d64..a5f0bd5 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 459a707..84eb873 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 6e35ff9..bb6902e 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 82f1437..4d9dcaa 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
