diff options
author | Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de> | 2021-03-25 22:48:09 +0100 |
---|---|---|
committer | Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de> | 2021-05-27 21:30:43 +0200 |
commit | c7f4ea442a264b5cb3a9ef0eed641f4778dfb5b7 (patch) | |
tree | 2bdd021e3da3fb7792cab94d5c1587bc5e64cf4f | |
parent | 84dcbc72b968ebc666387874171580463f1944dd (diff) |
CVE-2020-28014, CVE-2021-27216: PID file handling
Arbitrary PID file creation, clobbering, and deletion.
Patch provided by Qualys.
(cherry picked from commit 974f32939a922512b27d9f0a8a1cb5dec60e7d37)
(cherry picked from commit 43c6f0b83200b7082353c50187ef75de3704580a)
-rw-r--r-- | doc/doc-txt/ChangeLog | 5 | ||||
-rw-r--r-- | src/src/daemon.c | 169 | ||||
-rw-r--r-- | src/src/exim.c | 4 | ||||
-rw-r--r-- | test/stderr/0433 | 18 |
4 files changed, 149 insertions, 47 deletions
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 4debef807..adf43bc4b 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -296,9 +296,12 @@ PP/11 Fix security issue in BDAT state confusion. HS/03 Die on "/../" in msglog file names -QS/01 Creation of (database) files in $spool_dir: only uid=0 or the euid of +QS/01 Creation of (database) files in $spool_dir: only uid=0 or the uid of the Exim runtime user are allowed to create files. +QS/02 PID file creation/deletion: only possible if uid=0 or uid is the Exim + runtime user. + Exim version 4.94 ----------------- diff --git a/src/src/daemon.c b/src/src/daemon.c index ed7d30a16..f40a58c75 100644 --- a/src/src/daemon.c +++ b/src/src/daemon.c @@ -932,7 +932,6 @@ while ((pid = waitpid(-1, &status, WNOHANG)) > 0) } - static void set_pid_file_path(void) { @@ -947,34 +946,144 @@ if (pid_file_path[0] != '/') } -/* Remove the daemon's pidfile. Note: runs with root privilege, -as a direct child of the daemon. Does not return. */ +enum pid_op { PID_WRITE, PID_CHECK, PID_DELETE }; -void -delete_pid_file(void) +/* Do various pid file operations as safe as possible. Ideally we'd just +drop the privileges for creation of the pid file and not care at all about removal of +the file. FIXME. +Returns: true on success, false + errno==EACCES otherwise +*/ +static BOOL +operate_on_pid_file(const enum pid_op operation, const pid_t pid) { -uschar * daemon_pid = string_sprintf("%d\n", (int)getppid()); -FILE * f; +char pid_line[sizeof(int) * 3 + 2]; +const int pid_len = snprintf(pid_line, sizeof(pid_line), "%d\n", (int)pid); +BOOL lines_match = FALSE; + +char * path = NULL; +char * base = NULL; +char * dir = NULL; + +const int dir_flags = O_RDONLY | O_NONBLOCK; +const int base_flags = O_NOFOLLOW | O_NONBLOCK; +const mode_t base_mode = 0644; +struct stat sb; + +int cwd_fd = -1; +int dir_fd = -1; +int base_fd = -1; + +BOOL success = FALSE; +errno = EACCES; set_pid_file_path(); -if ((f = Ufopen(pid_file_path, "rb"))) +if (!f.running_in_test_harness && real_uid != root_uid && real_uid != exim_uid) goto cleanup; +if (pid_len < 2 || pid_len >= (int)sizeof(pid_line)) goto cleanup; + +path = CS string_copy(pid_file_path); +if ((base = Ustrrchr(path, '/')) == NULL) /* should not happen, but who knows */ + log_write(0, LOG_MAIN|LOG_PANIC_DIE, "pid file path \"%s\" does not contain a '/'", pid_file_path); + +dir = (base != path) ? path : "/"; +*base++ = '\0'; + +if (!dir || !*dir || *dir != '/') goto cleanup; +if (!base || !*base || strchr(base, '/') != NULL) goto cleanup; + +cwd_fd = open(".", dir_flags); +if (cwd_fd < 0 || fstat(cwd_fd, &sb) != 0 || !S_ISDIR(sb.st_mode)) goto cleanup; +dir_fd = open(dir, dir_flags); +if (dir_fd < 0 || fstat(dir_fd, &sb) != 0 || !S_ISDIR(sb.st_mode)) goto cleanup; + +/* emulate openat */ +if (fchdir(dir_fd) != 0) goto cleanup; +base_fd = open(base, O_RDONLY | base_flags); +if (fchdir(cwd_fd) != 0) + log_write(0, LOG_MAIN|LOG_PANIC_DIE, "can't return to previous working dir: %s", strerror(errno)); + +if (base_fd >= 0) { - if ( fgets(CS big_buffer, big_buffer_size, f) - && Ustrcmp(daemon_pid, big_buffer) == 0 - ) - if (Uunlink(pid_file_path) == 0) + char line[sizeof(pid_line)]; + ssize_t len = -1; + + if (fstat(base_fd, &sb) != 0 || !S_ISREG(sb.st_mode)) goto cleanup; + if ((sb.st_mode & 07777) != base_mode || sb.st_nlink != 1) goto cleanup; + if (sb.st_size < 2 || sb.st_size >= (off_t)sizeof(line)) goto cleanup; + + len = read(base_fd, line, sizeof(line)); + if (len != (ssize_t)sb.st_size) goto cleanup; + line[len] = '\0'; + + if (strspn(line, "0123456789") != (size_t)len-1) goto cleanup; + if (line[len-1] != '\n') goto cleanup; + lines_match = (len == pid_len && strcmp(line, pid_line) == 0); + } + +if (operation == PID_WRITE) + { + if (!lines_match) + { + if (base_fd >= 0) { - DEBUG(D_any) - debug_printf("%s unlink: %s\n", pid_file_path, strerror(errno)); - } - else - DEBUG(D_any) - debug_printf("unlinked %s\n", pid_file_path); - fclose(f); + int error = -1; + /* emulate unlinkat */ + if (fchdir(dir_fd) != 0) goto cleanup; + error = unlink(base); + if (fchdir(cwd_fd) != 0) + log_write(0, LOG_MAIN|LOG_PANIC_DIE, "can't return to previous working dir: %s", strerror(errno)); + if (error) goto cleanup; + (void)close(base_fd); + base_fd = -1; + } + /* emulate openat */ + if (fchdir(dir_fd) != 0) goto cleanup; + base_fd = open(base, O_WRONLY | O_CREAT | O_EXCL | base_flags, base_mode); + if (fchdir(cwd_fd) != 0) + log_write(0, LOG_MAIN|LOG_PANIC_DIE, "can't return to previous working dir: %s", strerror(errno)); + if (base_fd < 0) goto cleanup; + if (fchmod(base_fd, base_mode) != 0) goto cleanup; + if (write(base_fd, pid_line, pid_len) != pid_len) goto cleanup; + DEBUG(D_any) debug_printf("pid written to %s\n", pid_file_path); + } } else - DEBUG(D_any) - debug_printf("%s\n", string_open_failed("pid file %s", pid_file_path)); + { + if (!lines_match) goto cleanup; + if (operation == PID_DELETE) + { + int error = -1; + /* emulate unlinkat */ + if (fchdir(dir_fd) != 0) goto cleanup; + error = unlink(base); + if (fchdir(cwd_fd) != 0) + log_write(0, LOG_MAIN|LOG_PANIC_DIE, "can't return to previous working dir: %s", strerror(errno)); + if (error) goto cleanup; + } + } + +success = TRUE; +errno = 0; + +cleanup: +if (cwd_fd >= 0) (void)close(cwd_fd); +if (dir_fd >= 0) (void)close(dir_fd); +if (base_fd >= 0) (void)close(base_fd); +return success; +} + + +/* Remove the daemon's pidfile. Note: runs with root privilege, +as a direct child of the daemon. Does not return. */ + +void +delete_pid_file(void) +{ +const BOOL success = operate_on_pid_file(PID_DELETE, getppid()); + +DEBUG(D_any) + debug_printf("delete pid file %s %s: %s\n", pid_file_path, + success ? "success" : "failure", strerror(errno)); + exim_exit(EXIT_SUCCESS); } @@ -1841,22 +1950,14 @@ The variable daemon_write_pid is used to control this. */ if (f.running_in_test_harness || write_pid) { - FILE *f; - - set_pid_file_path(); - if ((f = modefopen(pid_file_path, "wb", 0644))) - { - (void)fprintf(f, "%d\n", (int)getpid()); - (void)fclose(f); - DEBUG(D_any) debug_printf("pid written to %s\n", pid_file_path); - } - else - DEBUG(D_any) - debug_printf("%s\n", string_open_failed("pid file %s", pid_file_path)); + const enum pid_op operation = (f.running_in_test_harness + || real_uid == root_uid + || (real_uid == exim_uid && !override_pid_file_path)) ? PID_WRITE : PID_CHECK; + if (!operate_on_pid_file(operation, getpid())) + DEBUG(D_any) debug_printf("%s pid file %s: %s\n", (operation == PID_WRITE) ? "write" : "check", pid_file_path, strerror(errno)); } /* Set up the handler for SIGHUP, which causes a restart of the daemon. */ - sighup_seen = FALSE; signal(SIGHUP, sighup_handler); diff --git a/src/src/exim.c b/src/src/exim.c index abb3ba7c0..49ba9e728 100644 --- a/src/src/exim.c +++ b/src/src/exim.c @@ -3256,6 +3256,10 @@ on the second character (the one after '-'), to save some effort. */ -oPX: delete pid file of daemon */ case 'P': + if (!f.running_in_test_harness && real_uid != root_uid && real_uid != exim_uid) + exim_fail("exim: only uid=%d or uid=%d can use -oP and -oPX " + "(uid=%d euid=%d | %d)\n", + root_uid, exim_uid, getuid(), geteuid(), real_uid); if (!*argrest) override_pid_file_path = argv[++i]; else if (Ustrcmp(argrest, "X") == 0) delete_pid_file(); else badarg = TRUE; diff --git a/test/stderr/0433 b/test/stderr/0433 index b36788373..3d0e9e9f8 100644 --- a/test/stderr/0433 +++ b/test/stderr/0433 @@ -18,9 +18,8 @@ LOG: MAIN set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on port 1225 daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID Listening... -SIGTERM seen -pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX SIGTERM/SIGINT seen +pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX search_tidyup called >>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>> Exim version x.yz .... @@ -42,9 +41,8 @@ LOG: MAIN set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on port 1225 port 1226 daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID Listening... -SIGTERM seen -pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX SIGTERM/SIGINT seen +pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX search_tidyup called >>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>> Exim version x.yz .... @@ -67,9 +65,8 @@ LOG: MAIN set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on [127.0.0.1]:1228 port 1225 (IPv4) port 1226 (IPv4) daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID Listening... -SIGTERM seen -pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 127.0.0.1.1228 ; 0.0.0.0 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX SIGTERM/SIGINT seen +pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 127.0.0.1.1228 ; 0.0.0.0 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX search_tidyup called >>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>> Exim version x.yz .... @@ -92,9 +89,8 @@ LOG: MAIN set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on port 1225 port 1226 [127.0.0.1]:1228 daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID Listening... -SIGTERM seen -pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 0.0.0.0; 127.0.0.1.1228 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX SIGTERM/SIGINT seen +pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 0.0.0.0; 127.0.0.1.1228 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX search_tidyup called >>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>> Exim version x.yz .... @@ -118,9 +114,8 @@ LOG: MAIN set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on [127.0.0.1]:1228 port 1227 (IPv4) daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID Listening... -SIGTERM seen -pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 127.0.0.1.1228 ; 0.0.0.0 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX SIGTERM/SIGINT seen +pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -DPORT=daemon_smtp_port=1225:1226 -DIFACE=local_interfaces = <; 127.0.0.1.1228 ; 0.0.0.0 -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX search_tidyup called >>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>> Exim version x.yz .... @@ -144,8 +139,7 @@ LOG: MAIN set_process_info: pppp daemon(x.yz): no queue runs, listening for SMTP on port 1225 port 1226 daemon running with uid=EXIM_UID gid=EXIM_GID euid=EXIM_UID egid=EXIM_GID Listening... -SIGTERM seen -pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX SIGTERM/SIGINT seen +pppp exec TESTSUITE/eximdir/exim -DEXIM_PATH=TESTSUITE/eximdir/exim -DSERVER=server -C TESTSUITE/test-config -d=0xf7795cfd -MCd daemon-del-pidfile -oP TESTSUITE/spool/exim-daemon.pid -oPX search_tidyup called >>>>>>>>>>>>>>>> Exim pid=pppp (daemon) terminating with rc=0 >>>>>>>>>>>>>>>> |