diff --git a/exim/src/exim.c b/exim/src/exim.c index dd3b5f9..67fbc5c 100644 --- a/exim/src/exim.c +++ b/exim/src/exim.c @@ -1300,7 +1300,7 @@ int arg_error_handling = error_handling; int filter_sfd = -1; int filter_ufd = -1; int group_count; -int i; +int i, rv; int list_queue_option = 0; int msg_action = 0; int msg_action_arg = -1; @@ -1639,8 +1639,20 @@ real_gid = getgid(); if (real_uid == root_uid) { - setgid(real_gid); - setuid(real_uid); + rv = setgid(real_gid); + if (rv) + { + fprintf(stderr, "exim: setgid(%ld) failed: %s\n", + (long int)real_gid, strerror(errno)); + exit(EXIT_FAILURE); + } + rv = setuid(real_uid); + if (rv) + { + fprintf(stderr, "exim: setuid(%ld) failed: %s\n", + (long int)real_uid, strerror(errno)); + exit(EXIT_FAILURE); + } } /* If neither the original real uid nor the original euid was root, Exim is @@ -3862,7 +3874,28 @@ if (!unprivileged && /* originally had root AND */ /* When we are retaining a privileged uid, we still change to the exim gid. */ -else setgid(exim_gid); +else + { + int rv; + rv = setgid(exim_gid); + /* Impact of failure is that some stuff might end up with an incorrect group. + We track this for failures from root, since any attempt to change privilege + by root should succeed and failures should be examined. For non-root, + there's no security risk. For me, it's { exim -bV } on a just-built binary, + no need to complain then. */ + if (rv == -1) + { + if (!unprivileged) + { + fprintf(stderr, + "exim: changing group failed: %s\n", strerror(errno)); + exit(EXIT_FAILURE); + } + else + debug_printf("changing group to %ld failed: %s\n", + (long int)exim_gid, strerror(errno)); + } + } /* Handle a request to scan a file for malware */ if (malware_test_file) diff --git a/exim/src/log.c b/exim/src/log.c index 1d2c8f5..67a3d85 100644 --- a/exim/src/log.c +++ b/exim/src/log.c @@ -361,17 +361,26 @@ are neither exim nor root, creation is not attempted. */ else if (euid == root_uid) { - int status; + int status, rv; pid_t pid = fork(); /* In the subprocess, change uid/gid and do the creation. Return 0 from the - subprocess on success. There doesn't seem much point in testing for setgid - and setuid errors. */ + 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) { - (void)setgid(exim_gid); - (void)setuid(exim_uid); + rv = setgid(exim_gid); + if (rv) + die(US"exim: setgid for log-file creation failed, aborting", + US"Unexpected log failure, please try later"); + rv = setuid(exim_uid); + if (rv) + die(US"exim: setuid for log-file creation failed, aborting", + US"Unexpected log failure, please try later"); _exit((create_log(buffer) < 0)? 1 : 0); }