commit 1670ef10063d7708eb736a482d1ad25b9c59521d Author: Phil Pennock Date: Fri Jan 21 03:56:02 2011 -0500 Check return values of setgid/setuid. CVE-2011-0017 One assertion of the unimportance of checking the return value was wrong, in the event of a compromised exim run-time user. Index: exim-4.72/doc/ChangeLog =================================================================== --- exim-4.72.orig/doc/ChangeLog +++ exim-4.72/doc/ChangeLog @@ -3,6 +3,11 @@ $Cambridge: exim/exim-doc/doc-txt/Change Change log file for Exim from version 4.21 ------------------------------------------- +PP/04 CVE-2011-0017 - check return value of setuid/setgid. This is a + privilege escalation vulnerability whereby the Exim run-time user + can cause root to append content of the attacker's choosing to + arbitrary files. + Exim version 4.72 ----------------- Index: exim-4.72/doc/NewStuff =================================================================== --- exim-4.72.orig/doc/NewStuff +++ exim-4.72/doc/NewStuff @@ -9,6 +9,15 @@ test from the snapshots or the CVS befor the documentation is updated, this file is reduced to a short list. +Version CVE-2011-0017 +--------------------- + + 1. SECURITY FIX: privilege escalation flaw fixed. On Linux (and only Linux) + the flaw permitted the Exim run-time user to cause root to append to + arbitrary files of the attacker's choosing, with the content based + on content supplied by the attacker. + + Version 4.72 ------------ Index: exim-4.72/src/exim.c =================================================================== --- exim-4.72.orig/src/exim.c +++ exim-4.72/src/exim.c @@ -1309,7 +1309,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; @@ -1628,8 +1628,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 @@ -3709,7 +3721,28 @@ if (!unprivileged && /* 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 list the delivery queue */ Index: exim-4.72/src/log.c =================================================================== --- exim-4.72.orig/src/log.c +++ exim-4.72/src/log.c @@ -343,17 +343,26 @@ are neither exim nor root, creation is n 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); }