[exim-dev] [PATCH 3/3 v2] Check configure file permissions e…

(Bug 1044, CVE-2010-4345)
---
Revamped version; this time we just expose a global 'trusted_config'
variable for readconf.c to use as a flag for whether to check the config
file for inappropriate writeability. It fits in better with the way I
want to handle the trusted config whitelist, and doesn't expose
'internal' logic from exim.c

 doc/doc-txt/ChangeLog |    5 +++++
 src/src/exim.c        |    7 ++++---
 src/src/globals.c     |    1 +
 src/src/globals.h     |    1 +
 src/src/readconf.c    |    6 +++---
 5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 99a6f17..0063c6b 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -78,6 +78,11 @@ DW/22 Bugzilla 1044: CVE-2010-4345 - partial fix: restrict default behaviour
       of CONFIGURE_OWNER and CONFIGURE_GROUP options to no longer allow a
       configuration file which is writeable by the Exim user or group.

+DW/23 Bugzilla 1044: CVE-2010-4345 - part two: extend checks for writeability
+      of configuration files to cover files specified with the -C option if
+      they are going to be used with root privileges, not just the default
+      configuration file.
+

Exim version 4.72
-----------------
diff --git a/src/src/exim.c b/src/src/exim.c
index 50950fa..0d8f244 100644
--- a/src/src/exim.c
+++ b/src/src/exim.c
@@ -1848,6 +1848,7 @@ for (i = 1; i < argc; i++)

       config_main_filelist = argrest;
       config_changed = TRUE;
+      trusted_config = FALSE;
       }
     break;

@@ -3046,7 +3047,7 @@ values (such as the path name). If running in the test harness, pretend that
configuration file changes and macro definitions haven't happened. */

 if ((                                            /* EITHER */
-    (config_changed || macros != NULL) &&        /* Config changed, and */
+    (!trusted_config || 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 */
@@ -3265,7 +3266,7 @@ 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. */

-if (removed_privilege && (config_changed || macros != NULL) &&
+if (removed_privilege && (!trusted_config || macros != NULL) &&
     real_uid == exim_uid)
   {
   #ifdef ALT_CONFIG_ROOT_ONLY
@@ -3277,7 +3278,7 @@ if (removed_privilege && (config_changed || macros != NULL) &&
   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");
+      (int)exim_uid, trusted_config? "-D" : "-C");
   #endif
   }

diff --git a/src/src/globals.c b/src/src/globals.c
index 9b77d87..f77fbcc 100644
--- a/src/src/globals.c
+++ b/src/src/globals.c
@@ -1268,6 +1268,7 @@ tree_node  *tree_nonrecipients = NULL;
 tree_node  *tree_unusable      = NULL;

 BOOL    trusted_caller         = FALSE;
+BOOL    trusted_config         = TRUE;
 gid_t  *trusted_groups         = NULL;
 uid_t  *trusted_users          = NULL;
 uschar *timezone_string        = US TIMEZONE_DEFAULT;
diff --git a/src/src/globals.h b/src/src/globals.h
index d66880e..62dd6b0 100644
--- a/src/src/globals.h
+++ b/src/src/globals.h
@@ -783,6 +783,7 @@ extern tree_node *tree_nonrecipients;  /* Tree of nonrecipient addresses */
 extern tree_node *tree_unusable;       /* Tree of unusable addresses */

 extern BOOL    trusted_caller;         /* Caller is trusted */
+extern BOOL    trusted_config;         /* Configuration file is trusted */
 extern gid_t  *trusted_groups;         /* List of trusted groups */
 extern uid_t  *trusted_users;          /* List of trusted users */
 extern uschar *timezone_string;        /* Required timezone setting */
diff --git a/src/src/readconf.c b/src/src/readconf.c
index 0803058..118ccf5 100644
--- a/src/src/readconf.c
+++ b/src/src/readconf.c
@@ -2874,10 +2874,10 @@ else
       "configuration file %s", filename));
   }

-/* Check the status of the file we have opened, unless it was specified on
-the command line, in which case privilege was given away at the start. */
+/* Check the status of the file we have opened, if we have retained root
+privileges. */

-if (!config_changed)
+if (trusted_config)
   {
   if (fstat(fileno(config_file), &statbuf) != 0)
     log_write(0, LOG_MAIN|LOG_PANIC_DIE, "failed to stat configuration file %s",
-- 
1.7.3.2