exim/CVE-2010-4345.diff

Bugzilla: bnc#658731
Author: David Woodhouse
Date: 2010-12-09 17:22 UTC
To: exim-dev
Subject: Re: [exim-dev] [Exim-maintainers] Remote root vulnerability in Exim
[Resend to exim-dev for comment, since exim-maintainers is quiet]

On Thu, 2010-12-09 at 10:18 +0000, David Woodhouse wrote:
> In the meantime, I'm looking at the second bug; the privilege
> escalation. I think the best answer to that is to kill the
> ALT_CONFIG_ROOT_ONLY option completely, and *always* allow the trusted
> user to use an alternative config but *only* if that config is listed in
> a file such as /etc/exim/allowed-configs. Does that seem sane?

Hm, we already have ALT_CONFIG_PREFIX which *almost* does this. It's
just that when defined, it refuses to run at all with a config that
doesn't match the prefix; rather than simply running without privileges.

I don't really see the point in refusing to run at all; the user can
always build their *own* Exim binary and run it without privileges.

So I've come up with the patch below. It kills the ALT_CONFIG_ROOT_ONLY
option, by making it effectively always set and never allowing even the
trusted Exim user to use arbitrary config files.

It repurposes the existing ALT_CONFIG_PREFIX, rather than adding a new
option and making things more complex. Instead of refusing to run with
config files that don't match the prefix, Exim will simply run without
root privileges. And it *will* run with root privileges when invoked
with the -C option for a file which *does* match the prefix.

Important user-visible changes would be:

- If you previously had ALT_CONFIG_ROOT_ONLY unset (the default), then
you must set ALT_CONFIG_PREFIX and use matching files if you want
to use alternative config files.

- If you previously had ALT_CONFIG_PREFIX set, then you should be aware
that matching config files will now be invoked with root privileges
regardless of the uid of the invoking user.

We could change the latter so that non-root and non-exim users invoking
config files in ALT_CONFIG_PREFIX are *never* granted root privs, but
I'm not sure we should. Comments?

Index: exim-4.71/src/exim.c
===================================================================
--- exim-4.71.orig/src/exim.c
+++ exim-4.71/src/exim.c
@@ -3158,12 +3158,9 @@ if (setgroups(0, NULL) != 0)
   }
 
 /* If the configuration file name has been altered by an argument on the
-command line (either a new file name or a macro definition) and the caller is
-not root or the exim user, or if this is a filter testing run, remove any
-setuid privilege the program has, and run as the underlying user.
-
-If ALT_CONFIG_ROOT_ONLY is defined, the exim user is locked out of this, which
-severely restricts the use of -C for some purposes.
+command line (either a new file name not matching ALT_CONFIG_PREFIX or a macro
+definition) and the caller is not root, or if this is a filter testing run,
+remove any setuid privilege the program has, and run as the underlying user.
 
 Otherwise, set the real ids to the effective values (should be root unless run
 from inetd, which it can either be root or the exim uid, if one is configured).
@@ -3177,9 +3174,6 @@ configuration file changes and macro def
 if ((                                            /* EITHER */
     (config_changed || macros != NULL) &&        /* Config changed, and */
     real_uid != root_uid &&                      /* Not root, and */
-    #ifndef ALT_CONFIG_ROOT_ONLY                 /* (when not locked out) */
-    real_uid != exim_uid &&                      /* Not exim, and */
-    #endif
     !running_in_test_harness                     /* Not fudged */
     ) ||                                         /*   OR   */
     expansion_test                               /* expansion testing */
@@ -3367,47 +3361,16 @@ else
   }
 
 /* Handle the case when we have removed the setuid privilege because of -C or
--D. This means that the caller of Exim was not root, and, provided that
-ALT_CONFIG_ROOT_ONLY is not defined, was not the Exim user that is built into
-the binary.
-
-If ALT_CONFIG_ROOT_ONLY is not defined, there is a problem if it turns out we
-were running as the exim user defined in the configuration file (different to
-the one in the binary). The sysadmin may expect this case to retain privilege
-because "the binary was called by the Exim user", but it hasn't, because of the
-order in which it handles this stuff. There are two possibilities:
-
-  (1) If deliver_drop_privilege is set, Exim is not going to re-exec in order
-      to do message deliveries. Thus, the fact that it is running as a
-      non-privileged user is plausible, and might be wanted in some special
-      configurations. However, really_exim will have been set false when
-      privilege was dropped, to stop Exim trying to write to its normal log
-      files. Therefore, re-enable normal log processing, assuming the sysadmin
-      has set up the log directory correctly.
-
-  (2) If deliver_drop_privilege is not set, the configuration won't work as
-      apparently intended, and so we log a panic message. In order to retain
-      root for -C or -D, the caller must either be root or the Exim user
-      defined in the binary (when deliver_drop_ privilege is false).
-
-If ALT_CONFIG_ROOT_ONLY is defined, we don't know whether we were called by the
-built-in exim user or one defined in the configuration. In either event,
-re-enable log processing, assuming the sysadmin knows what they are doing. */
+-D. This means that the caller of Exim was not root.
+
+We don't know whether we were called by the built-in exim user or one defined
+in the configuration. In either event, re-enable log processing, assuming the
+sysadmin knows what they are doing. */
 
 if (removed_privilege && (config_changed || macros != NULL) &&
     real_uid == exim_uid)
   {
-  #ifdef ALT_CONFIG_ROOT_ONLY
   really_exim = TRUE;   /* let logging work normally */
-  #else
-
-  if (deliver_drop_privilege)
-    really_exim = TRUE; /* let logging work normally */
-  else
-    log_write(0, LOG_MAIN|LOG_PANIC,
-      "exim user (uid=%d) is defined only at runtime; privilege lost for %s",
-      (int)exim_uid, config_changed? "-C" : "-D");
-  #endif
   }
 
 /* Start up Perl interpreter if Perl support is configured and there is a