summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhil Pennock <pdp@exim.org>2011-01-21 03:56:02 -0500
committerPhil Pennock <pdp@exim.org>2011-01-21 03:56:02 -0500
commit1670ef10063d7708eb736a482d1ad25b9c59521d (patch)
treecc8ad240887f3dfa0f4f56b228e6d6bbcb376de3
parent6545de78cb822ab5db97a2f16fe7a42cc9488bd8 (diff)
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.
-rw-r--r--doc/doc-txt/ChangeLog5
-rw-r--r--doc/doc-txt/NewStuff7
-rw-r--r--src/src/exim.c41
-rw-r--r--src/src/log.c19
4 files changed, 62 insertions, 10 deletions
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index ff375d398..a1bd4e7fc 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -32,6 +32,11 @@ PP/03 Report version information for many libraries, including
version.h, now support a version extension string for distributors
who patch heavily. Dynamic module ABI change.
+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.73
-----------------
diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff
index 8c8aeaa50..3a3ad5de5 100644
--- a/doc/doc-txt/NewStuff
+++ b/doc/doc-txt/NewStuff
@@ -12,7 +12,12 @@ the documentation is updated, this file is reduced to a short list.
Version 4.74
------------
- 1. Exim now supports loading some lookup types at run-time, using your
+ 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.
+
+ 2. Exim now supports loading some lookup types at run-time, using your
platform's dlopen() functionality. This has limited platform support
and the intention is not to support every variant, it's limited to
dlopen(). This permits the main Exim binary to not be linked against
diff --git a/src/src/exim.c b/src/src/exim.c
index dd3b5f9e7..67fbc5cf7 100644
--- a/src/src/exim.c
+++ b/src/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/src/src/log.c b/src/src/log.c
index 1d2c8f5e0..67a3d8543 100644
--- a/src/src/log.c
+++ b/src/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);
}