exim/CVE-2011-0017.diff

commit 1670ef10063d7708eb736a482d1ad25b9c59521d
Author: Phil Pennock <pdp@exim.org>
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);
     }