CVE-2012-3524: Don't access environment variables or run dbus-launch when setuid

This matches a corresponding change in GLib.  See
glib/gutils.c:g_check_setuid().

Some programs attempt to use libdbus when setuid; notably the X.org
server is shipped in such a configuration. libdbus never had an
explicit policy about its use in setuid programs.

I'm not sure whether we should advertise such support.  However, given
that there are real-world programs that do this currently, we can make
them safer with not too much effort.

Better to fix a problem caused by an interaction between two
components in *both* places if possible.

How to determine whether or not we're running in a privilege-escalated
path is operating system specific.  Note that GTK+'s code to check
euid versus uid worked historically on Unix, more modern systems have
filesystem capabilities and SELinux domain transitions, neither of
which are captured by the uid comparison.

On Linux/glibc, the way this works is that the kernel sets an
AT_SECURE flag in the ELF auxiliary vector, and glibc looks for it on
startup.  If found, then glibc sets a public-but-undocumented
__libc_enable_secure variable which we can use.  Unfortunately, while
it *previously* worked to check this variable, a combination of newer
binutils and RPM break it:
http://www.openwall.com/lists/owl-dev/2012/08/14/1

So for now on Linux/glibc, we fall back to the historical Unix version
until we get glibc fixed.

On some BSD variants, there is a issetugid() function.  On other Unix
variants, we fall back to what GTK+ has been doing.

Reported-by: Sebastian Krahmer <krahmer@suse.de>
Signed-off-by: Colin Walters <walters@verbum.org>

diff --git a/configure.ac b/configure.ac
index 2e34f565305b0d77850d70976a80d31615994033..df9098565037fbc4ab964eed8e1108fff8431b10 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -596,7 +596,7 @@ AC_DEFINE_UNQUOTED([DBUS_USE_SYNC], [$have_sync], [Use the gcc __sync extension]
 AC_SEARCH_LIBS(socket,[socket network])
 AC_CHECK_FUNC(gethostbyname,,[AC_CHECK_LIB(nsl,gethostbyname)])
 
-AC_CHECK_FUNCS(vsnprintf vasprintf nanosleep usleep setenv clearenv unsetenv socketpair getgrouplist fpathconf setrlimit poll setlocale localeconv strtoll strtoull)
+AC_CHECK_FUNCS(vsnprintf vasprintf nanosleep usleep setenv clearenv unsetenv socketpair getgrouplist fpathconf setrlimit poll setlocale localeconv strtoll strtoull issetugid getresuid)
 
 AC_CHECK_HEADERS([syslog.h])
 if test "x$ac_cv_header_syslog_h" = "xyes"; then

diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c
index 23b9df5a0afa396490100a5dc13c89ba2756cde8..3b9ce3150e61f7112d057a4bf8437cdbe134eb17 100644 (file)
--- a/dbus/dbus-keyring.c
+++ b/dbus/dbus-keyring.c
@@ -717,6 +717,13 @@ _dbus_keyring_new_for_credentials (DBusCredentials  *credentials,
   DBusCredentials *our_credentials;
   
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
+
+  if (_dbus_check_setuid ())
+    {
+      dbus_set_error_const (error, DBUS_ERROR_NOT_SUPPORTED,
+                            "Unable to create DBus keyring when setuid");
+      return NULL;
+    }
   
   keyring = NULL;
   error_set = FALSE;

diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index cef8bd310aaea7a1a998ef7c4ea702367060bcd9..b4ecc96e78ddd477ecb19828f1e114db443e4a0e 100644 (file)
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -3434,6 +3434,13 @@ _dbus_get_autolaunch_address (const char *scope,
   DBusString uuid;
   dbus_bool_t retval;
 
+  if (_dbus_check_setuid ())
+    {
+      dbus_set_error_const (error, DBUS_ERROR_NOT_SUPPORTED,
+                            "Unable to autolaunch when setuid");
+      return FALSE;
+    }
+
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
   retval = FALSE;
 
@@ -3551,6 +3558,13 @@ _dbus_lookup_launchd_socket (DBusString *socket_path,
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
+  if (_dbus_check_setuid ())
+    {
+      dbus_set_error_const (error, DBUS_ERROR_NOT_SUPPORTED,
+                            "Unable to find launchd socket when setuid");
+      return FALSE;
+    }
+
   i = 0;
   argv[i] = "launchctl";
   ++i;
@@ -3591,6 +3605,13 @@ _dbus_lookup_session_address_launchd (DBusString *address, DBusError  *error)
   dbus_bool_t valid_socket;
   DBusString socket_path;
 
+  if (_dbus_check_setuid ())
+    {
+      dbus_set_error_const (error, DBUS_ERROR_NOT_SUPPORTED,
+                            "Unable to find launchd socket when setuid");
+      return FALSE;
+    }
+
   if (!_dbus_string_init (&socket_path))
     {
       _DBUS_SET_OOM (error);
@@ -4086,4 +4107,57 @@ _dbus_close_all (void)
     close (i);
 }
 
+/**
+ * **NOTE**: If you modify this function, please also consider making
+ * the corresponding change in GLib.  See
+ * glib/gutils.c:g_check_setuid().
+ *
+ * Returns TRUE if the current process was executed as setuid (or an
+ * equivalent __libc_enable_secure is available).  See:
+ * http://osdir.com/ml/linux.lfs.hardened/2007-04/msg00032.html
+ */
+dbus_bool_t
+_dbus_check_setuid (void)
+{
+  /* TODO: get __libc_enable_secure exported from glibc.
+   * See http://www.openwall.com/lists/owl-dev/2012/08/14/1
+   */
+#if 0 && defined(HAVE_LIBC_ENABLE_SECURE)
+  {
+    /* See glibc/include/unistd.h */
+    extern int __libc_enable_secure;
+    return __libc_enable_secure;
+  }
+#elif defined(HAVE_ISSETUGID)
+  /* BSD: http://www.freebsd.org/cgi/man.cgi?query=issetugid&sektion=2 */
+  return issetugid ();
+#else
+  uid_t ruid, euid, suid; /* Real, effective and saved user ID's */
+  gid_t rgid, egid, sgid; /* Real, effective and saved group ID's */
+
+  static dbus_bool_t check_setuid_initialised;
+  static dbus_bool_t is_setuid;
+
+  if (_DBUS_UNLIKELY (!check_setuid_initialised))
+    {
+#ifdef HAVE_GETRESUID
+      if (getresuid (&ruid, &euid, &suid) != 0 ||
+          getresgid (&rgid, &egid, &sgid) != 0)
+#endif /* HAVE_GETRESUID */
+        {
+          suid = ruid = getuid ();
+          sgid = rgid = getgid ();
+          euid = geteuid ();
+          egid = getegid ();
+        }
+
+      check_setuid_initialised = TRUE;
+      is_setuid = (ruid != euid || ruid != suid ||
+                   rgid != egid || rgid != sgid);
+
+    }
+  return is_setuid;
+#endif
+}
+
 /* tests in dbus-sysdeps-util.c */

diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
index 397520af7d9cd10fdf1320666150a6f6666bec97..bc4951b5cff22c22002f3a75139f40d62cf8dd3c 100644 (file)
--- a/dbus/dbus-sysdeps-win.c
+++ b/dbus/dbus-sysdeps-win.c
@@ -3632,6 +3632,12 @@ _dbus_path_is_absolute (const DBusString *filename)
     return FALSE;
 }
 
+dbus_bool_t
+_dbus_check_setuid (void)
+{
+  return FALSE;
+}
+
 /** @} end of sysdeps-win */
 /* tests in dbus-sysdeps-util.c */
 

diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c
index 861bfec9e2990557d136a9af021e9f7ec28d82d2..04fb8d7636d0eed2ee607b46bef2e226e24e3d6f 100644 (file)
--- a/dbus/dbus-sysdeps.c
+++ b/dbus/dbus-sysdeps.c
@@ -182,6 +182,11 @@ _dbus_setenv (const char *varname,
 const char*
 _dbus_getenv (const char *varname)
 {  
+  /* Don't respect any environment variables if the current process is
+   * setuid.  This is the equivalent of glibc's __secure_getenv().
+   */
+  if (_dbus_check_setuid ())
+    return NULL;
   return getenv (varname);
 }
 

diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index 4052cda9c3a100ffdb415188c37cb6843c2249ef..eee916083b86d793d85dc342659324776da8f373 100644 (file)
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -87,6 +87,7 @@ typedef struct DBusPipe DBusPipe;
 
 void _dbus_abort (void) _DBUS_GNUC_NORETURN;
 
+dbus_bool_t _dbus_check_setuid (void);
 const char* _dbus_getenv (const char *varname);
 dbus_bool_t _dbus_setenv (const char *varname,
                          const char *value);