1From 4fb6d745c23581dba78d7c408fdfb14def1161d5 Mon Sep 17 00:00:00 2001 2From: Qualys Security Advisory <qsa@qualys.com> 3Date: Tue, 23 Feb 2021 08:33:03 -0800 4Subject: [PATCH 24/26] CVE-2020-28007: Link attack in Exim's log directory 5 6We patch this vulnerability by opening (instead of just creating) the 7log file in an unprivileged (exim) child process, and by passing this 8file descriptor back to the privileged (root) parent process. The two 9functions log_send_fd() and log_recv_fd() are inspired by OpenSSH's 10functions mm_send_fd() and mm_receive_fd(); thanks! 11 12This patch also fixes: 13 14- a NULL-pointer dereference in usr1_handler() (this signal handler is 15 installed before process_log_path is initialized); 16 17- a file-descriptor leak in dmarc_write_history_file() (two return paths 18 did not close history_file_fd). 19 20Note: the use of log_open_as_exim() in dmarc_write_history_file() should 21be fine because the documentation explicitly states "Make sure the 22directory of this file is writable by the user exim runs as." 23--- 24 src/src/dmarc.c | 26 ++++--- 25 src/src/exim.c | 14 +--- 26 src/src/functions.h | 3 +- 27 src/src/log.c | 214 +++++++++++++++++++++++++++++++++------------------- 28 4 files changed, 154 insertions(+), 103 deletions(-) 29 30diff --git a/src/src/dmarc.c b/src/src/dmarc.c 31index 2e43f84..9f11f58 100644 32--- a/src/src/dmarc.c 33+++ b/src/src/dmarc.c 34@@ -517,8 +517,6 @@ return OK; 35 static int 36 dmarc_write_history_file() 37 { 38-int history_file_fd; 39-ssize_t written_len; 40 int tmp_ans; 41 u_char **rua; /* aggregate report addressees */ 42 uschar *history_buffer = NULL; 43@@ -528,14 +526,6 @@ if (!dmarc_history_file) 44 DEBUG(D_receive) debug_printf("DMARC history file not set\n"); 45 return DMARC_HIST_DISABLED; 46 } 47-history_file_fd = log_create(dmarc_history_file); 48- 49-if (history_file_fd < 0) 50- { 51- log_write(0, LOG_MAIN|LOG_PANIC, "failure to create DMARC history file: %s", 52- dmarc_history_file); 53- return DMARC_HIST_FILE_ERR; 54- } 55 56 /* Generate the contents of the history file */ 57 history_buffer = string_sprintf( 58@@ -587,16 +577,28 @@ if (host_checking || f.running_in_test_harness) 59 } 60 else 61 { 62+ ssize_t written_len; 63+ const int history_file_fd = log_open_as_exim(dmarc_history_file); 64+ 65+ if (history_file_fd < 0) 66+ { 67+ log_write(0, LOG_MAIN|LOG_PANIC, "failure to create DMARC history file: %s", 68+ dmarc_history_file); 69+ return DMARC_HIST_FILE_ERR; 70+ } 71+ 72 written_len = write_to_fd_buf(history_file_fd, 73 history_buffer, 74 Ustrlen(history_buffer)); 75- if (written_len == 0) 76+ 77+ (void)close(history_file_fd); 78+ 79+ if (written_len <= 0) 80 { 81 log_write(0, LOG_MAIN|LOG_PANIC, "failure to write to DMARC history file: %s", 82 dmarc_history_file); 83 return DMARC_HIST_WRITE_ERR; 84 } 85- (void)close(history_file_fd); 86 } 87 return DMARC_HIST_OK; 88 } 89diff --git a/src/src/exim.c b/src/src/exim.c 90index 77b488e..1b4a152 100644 91--- a/src/src/exim.c 92+++ b/src/src/exim.c 93@@ -233,18 +233,8 @@ int fd; 94 95 os_restarting_signal(sig, usr1_handler); 96 97-if ((fd = Uopen(process_log_path, O_APPEND|O_WRONLY, LOG_MODE)) < 0) 98- { 99- /* If we are already running as the Exim user, try to create it in the 100- current process (assuming spool_directory exists). Otherwise, if we are 101- root, do the creation in an exim:exim subprocess. */ 102- 103- int euid = geteuid(); 104- if (euid == exim_uid) 105- fd = Uopen(process_log_path, O_CREAT|O_APPEND|O_WRONLY, LOG_MODE); 106- else if (euid == root_uid) 107- fd = log_create_as_exim(process_log_path); 108- } 109+if (!process_log_path) return; 110+fd = log_open_as_exim(process_log_path); 111 112 /* If we are neither exim nor root, or if we failed to create the log file, 113 give up. There is not much useful we can do with errors, since we don't want 114diff --git a/src/src/functions.h b/src/src/functions.h 115index 51bb17a..9b12b32 100644 116--- a/src/src/functions.h 117+++ b/src/src/functions.h 118@@ -308,8 +308,7 @@ extern int ip_streamsocket(const uschar *, uschar **, int, host_item *); 119 extern int ipv6_nmtoa(int *, uschar *); 120 121 extern uschar *local_part_quote(uschar *); 122-extern int log_create(uschar *); 123-extern int log_create_as_exim(uschar *); 124+extern int log_open_as_exim(uschar *); 125 extern void log_close_all(void); 126 127 extern macro_item * macro_create(const uschar *, const uschar *, BOOL); 128diff --git a/src/src/log.c b/src/src/log.c 129index 07bf2ce..5d36b49 100644 130--- a/src/src/log.c 131+++ b/src/src/log.c 132@@ -264,14 +264,19 @@ overwrite it temporarily if it is necessary to create the directory. 133 Returns: a file descriptor, or < 0 on failure (errno set) 134 */ 135 136-int 137-log_create(uschar *name) 138+static int 139+log_open_already_exim(uschar * const name) 140 { 141-int fd = Uopen(name, 142-#ifdef O_CLOEXEC 143- O_CLOEXEC | 144-#endif 145- O_CREAT|O_APPEND|O_WRONLY, LOG_MODE); 146+int fd = -1; 147+const int flags = O_WRONLY | O_APPEND | O_CREAT | O_NONBLOCK; 148+ 149+if (geteuid() != exim_uid) 150+ { 151+ errno = EACCES; 152+ return -1; 153+ } 154+ 155+fd = Uopen(name, flags, LOG_MODE); 156 157 /* If creation failed, attempt to build a log directory in case that is the 158 problem. */ 159@@ -285,11 +290,7 @@ if (fd < 0 && errno == ENOENT) 160 DEBUG(D_any) debug_printf("%s log directory %s\n", 161 created ? "created" : "failed to create", name); 162 *lastslash = '/'; 163- if (created) fd = Uopen(name, 164-#ifdef O_CLOEXEC 165- O_CLOEXEC | 166-#endif 167- O_CREAT|O_APPEND|O_WRONLY, LOG_MODE); 168+ if (created) fd = Uopen(name, flags, LOG_MODE); 169 } 170 171 return fd; 172@@ -297,6 +298,81 @@ return fd; 173 174 175 176+/* Inspired by OpenSSH's mm_send_fd(). Thanks! */ 177+ 178+static int 179+log_send_fd(const int sock, const int fd) 180+{ 181+struct msghdr msg; 182+union { 183+ struct cmsghdr hdr; 184+ char buf[CMSG_SPACE(sizeof(int))]; 185+} cmsgbuf; 186+struct cmsghdr *cmsg; 187+struct iovec vec; 188+char ch = 'A'; 189+ssize_t n; 190+ 191+memset(&msg, 0, sizeof(msg)); 192+memset(&cmsgbuf, 0, sizeof(cmsgbuf)); 193+msg.msg_control = &cmsgbuf.buf; 194+msg.msg_controllen = sizeof(cmsgbuf.buf); 195+ 196+cmsg = CMSG_FIRSTHDR(&msg); 197+cmsg->cmsg_len = CMSG_LEN(sizeof(int)); 198+cmsg->cmsg_level = SOL_SOCKET; 199+cmsg->cmsg_type = SCM_RIGHTS; 200+*(int *)CMSG_DATA(cmsg) = fd; 201+ 202+vec.iov_base = &ch; 203+vec.iov_len = 1; 204+msg.msg_iov = &vec; 205+msg.msg_iovlen = 1; 206+ 207+while ((n = sendmsg(sock, &msg, 0)) == -1 && errno == EINTR); 208+if (n != 1) return -1; 209+return 0; 210+} 211+ 212+/* Inspired by OpenSSH's mm_receive_fd(). Thanks! */ 213+ 214+static int 215+log_recv_fd(const int sock) 216+{ 217+struct msghdr msg; 218+union { 219+ struct cmsghdr hdr; 220+ char buf[CMSG_SPACE(sizeof(int))]; 221+} cmsgbuf; 222+struct cmsghdr *cmsg; 223+struct iovec vec; 224+ssize_t n; 225+char ch = '\0'; 226+int fd = -1; 227+ 228+memset(&msg, 0, sizeof(msg)); 229+vec.iov_base = &ch; 230+vec.iov_len = 1; 231+msg.msg_iov = &vec; 232+msg.msg_iovlen = 1; 233+ 234+memset(&cmsgbuf, 0, sizeof(cmsgbuf)); 235+msg.msg_control = &cmsgbuf.buf; 236+msg.msg_controllen = sizeof(cmsgbuf.buf); 237+ 238+while ((n = recvmsg(sock, &msg, 0)) == -1 && errno == EINTR); 239+if (n != 1 || ch != 'A') return -1; 240+ 241+cmsg = CMSG_FIRSTHDR(&msg); 242+if (cmsg == NULL) return -1; 243+if (cmsg->cmsg_type != SCM_RIGHTS) return -1; 244+fd = *(const int *)CMSG_DATA(cmsg); 245+if (fd < 0) return -1; 246+return fd; 247+} 248+ 249+ 250+ 251 /************************************************* 252 * Create a log file as the exim user * 253 *************************************************/ 254@@ -312,41 +388,60 @@ Returns: a file descriptor, or < 0 on failure (errno set) 255 */ 256 257 int 258-log_create_as_exim(uschar *name) 259+log_open_as_exim(uschar * const name) 260 { 261-pid_t pid = exim_fork(US"logfile-create"); 262-int status = 1; 263 int fd = -1; 264+const uid_t euid = geteuid(); 265 266-/* In the subprocess, change uid/gid and do the creation. Return 0 from the 267-subprocess on success. If we don't check for setuid failures, then the file 268-can be created as root, so vulnerabilities which cause setuid to fail mean 269-that the Exim user can use symlinks to cause a file to be opened/created as 270-root. We always open for append, so can't nuke existing content but it would 271-still be Rather Bad. */ 272- 273-if (pid == 0) 274+if (euid == exim_uid) 275 { 276- if (setgid(exim_gid) < 0) 277- die(US"exim: setgid for log-file creation failed, aborting", 278- US"Unexpected log failure, please try later"); 279- if (setuid(exim_uid) < 0) 280- die(US"exim: setuid for log-file creation failed, aborting", 281- US"Unexpected log failure, please try later"); 282- _exit((log_create(name) < 0)? 1 : 0); 283+ fd = log_open_already_exim(name); 284 } 285+else if (euid == root_uid) 286+ { 287+ int sock[2]; 288+ if (socketpair(AF_UNIX, SOCK_STREAM, 0, sock) == 0) 289+ { 290+ const pid_t pid = exim_fork(US"logfile-open"); 291+ if (pid == 0) 292+ { 293+ (void)close(sock[0]); 294+ if (setgroups(1, &exim_gid) != 0) _exit(EXIT_FAILURE); 295+ if (setgid(exim_gid) != 0) _exit(EXIT_FAILURE); 296+ if (setuid(exim_uid) != 0) _exit(EXIT_FAILURE); 297+ 298+ if (getuid() != exim_uid || geteuid() != exim_uid) _exit(EXIT_FAILURE); 299+ if (getgid() != exim_gid || getegid() != exim_gid) _exit(EXIT_FAILURE); 300+ 301+ fd = log_open_already_exim(name); 302+ if (fd < 0) _exit(EXIT_FAILURE); 303+ if (log_send_fd(sock[1], fd) != 0) _exit(EXIT_FAILURE); 304+ (void)close(sock[1]); 305+ _exit(EXIT_SUCCESS); 306+ } 307 308-/* If we created a subprocess, wait for it. If it succeeded, try the open. */ 309- 310-while (pid > 0 && waitpid(pid, &status, 0) != pid); 311-if (status == 0) fd = Uopen(name, 312-#ifdef O_CLOEXEC 313- O_CLOEXEC | 314-#endif 315- O_APPEND|O_WRONLY, LOG_MODE); 316+ (void)close(sock[1]); 317+ if (pid > 0) 318+ { 319+ fd = log_recv_fd(sock[0]); 320+ while (waitpid(pid, NULL, 0) == -1 && errno == EINTR); 321+ } 322+ (void)close(sock[0]); 323+ } 324+ } 325 326-/* If we failed to create a subprocess, we are in a bad way. We return 327-with fd still < 0, and errno set, letting the caller handle the error. */ 328+if (fd >= 0) 329+ { 330+ int flags; 331+ flags = fcntl(fd, F_GETFD); 332+ if (flags != -1) (void)fcntl(fd, F_SETFD, flags | FD_CLOEXEC); 333+ flags = fcntl(fd, F_GETFL); 334+ if (flags != -1) (void)fcntl(fd, F_SETFL, flags & ~O_NONBLOCK); 335+ } 336+else 337+ { 338+ errno = EACCES; 339+ } 340 341 return fd; 342 } 343@@ -459,52 +554,17 @@ if (!ok) 344 die(US"exim: log file path too long: aborting", 345 US"Logging failure; please try later"); 346 347-/* We now have the file name. Try to open an existing file. After a successful 348-open, arrange for automatic closure on exec(), and then return. */ 349+/* We now have the file name. After a successful open, return. */ 350 351-*fd = Uopen(buffer, 352-#ifdef O_CLOEXEC 353- O_CLOEXEC | 354-#endif 355- O_APPEND|O_WRONLY, LOG_MODE); 356+*fd = log_open_as_exim(buffer); 357 358 if (*fd >= 0) 359 { 360-#ifndef O_CLOEXEC 361- (void)fcntl(*fd, F_SETFD, fcntl(*fd, F_GETFD) | FD_CLOEXEC); 362-#endif 363 return; 364 } 365 366-/* Open was not successful: try creating the file. If this is a root process, 367-we must do the creating in a subprocess set to exim:exim in order to ensure 368-that the file is created with the right ownership. Otherwise, there can be a 369-race if another Exim process is trying to write to the log at the same time. 370-The use of SIGUSR1 by the exiwhat utility can provoke a lot of simultaneous 371-writing. */ 372- 373 euid = geteuid(); 374 375-/* If we are already running as the Exim user (even if that user is root), 376-we can go ahead and create in the current process. */ 377- 378-if (euid == exim_uid) *fd = log_create(buffer); 379- 380-/* Otherwise, if we are root, do the creation in an exim:exim subprocess. If we 381-are neither exim nor root, creation is not attempted. */ 382- 383-else if (euid == root_uid) *fd = log_create_as_exim(buffer); 384- 385-/* If we now have an open file, set the close-on-exec flag and return. */ 386- 387-if (*fd >= 0) 388- { 389-#ifndef O_CLOEXEC 390- (void)fcntl(*fd, F_SETFD, fcntl(*fd, F_GETFD) | FD_CLOEXEC); 391-#endif 392- return; 393- } 394- 395 /* Creation failed. There are some circumstances in which we get here when 396 the effective uid is not root or exim, which is the problem. (For example, a 397 non-setuid binary with log_arguments set, called in certain ways.) Rather than 398