Colin Walters b64d575
From 450d975046bbd54271da62ce5fcbe50113f2e453 Mon Sep 17 00:00:00 2001
Colin Walters b64d575
From: Colin Walters <walters@verbum.org>
Colin Walters b64d575
Date: Wed, 22 Aug 2012 10:03:34 -0400
Colin Walters b64d575
Subject: [PATCH] CVE-2012-3524: Don't access environment variables or run
Colin Walters b64d575
 dbus-launch when setuid
Colin Walters b64d575
Colin Walters b64d575
This matches a corresponding change in GLib.  See
Colin Walters b64d575
glib/gutils.c:g_check_setuid().
Colin Walters b64d575
Colin Walters b64d575
Some programs attempt to use libdbus when setuid; notably the X.org
Colin Walters b64d575
server is shipped in such a configuration. libdbus never had an
Colin Walters b64d575
explicit policy about its use in setuid programs.
Colin Walters b64d575
Colin Walters b64d575
I'm not sure whether we should advertise such support.  However, given
Colin Walters b64d575
that there are real-world programs that do this currently, we can make
Colin Walters b64d575
them safer with not too much effort.
Colin Walters b64d575
Colin Walters b64d575
Better to fix a problem caused by an interaction between two
Colin Walters b64d575
components in *both* places if possible.
Colin Walters b64d575
Colin Walters b64d575
How to determine whether or not we're running in a privilege-escalated
Colin Walters b64d575
path is operating system specific.  Note that GTK+'s code to check
Colin Walters b64d575
euid versus uid worked historically on Unix, more modern systems have
Colin Walters b64d575
filesystem capabilities and SELinux domain transitions, neither of
Colin Walters b64d575
which are captured by the uid comparison.
Colin Walters b64d575
Colin Walters b64d575
On Linux/glibc, the way this works is that the kernel sets an
Colin Walters b64d575
AT_SECURE flag in the ELF auxiliary vector, and glibc looks for it on
Colin Walters b64d575
startup.  If found, then glibc sets a public-but-undocumented
Colin Walters b64d575
__libc_enable_secure variable which we can use.  Unfortunately, while
Colin Walters b64d575
it *previously* worked to check this variable, a combination of newer
Colin Walters b64d575
binutils and RPM break it:
Colin Walters b64d575
http://www.openwall.com/lists/owl-dev/2012/08/14/1
Colin Walters b64d575
Colin Walters b64d575
So for now on Linux/glibc, we fall back to the historical Unix version
Colin Walters b64d575
until we get glibc fixed.
Colin Walters b64d575
Colin Walters b64d575
On some BSD variants, there is a issetugid() function.  On other Unix
Colin Walters b64d575
variants, we fall back to what GTK+ has been doing.
Colin Walters b64d575
Colin Walters b64d575
Reported-by: Sebastian Krahmer <krahmer@suse.de>
Colin Walters b64d575
Signed-off-by: Colin Walters <walters@verbum.org>
Colin Walters b64d575
---
Colin Walters b64d575
 configure.ac             |  2 +-
Colin Walters b64d575
 dbus/dbus-keyring.c      |  7 +++++
Colin Walters b64d575
 dbus/dbus-sysdeps-unix.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
Colin Walters b64d575
 dbus/dbus-sysdeps-win.c  |  6 ++++
Colin Walters b64d575
 dbus/dbus-sysdeps.c      |  5 ++++
Colin Walters b64d575
 dbus/dbus-sysdeps.h      |  1 +
Colin Walters b64d575
 6 files changed, 94 insertions(+), 1 deletion(-)
Colin Walters b64d575
Colin Walters b64d575
diff --git a/configure.ac b/configure.ac
Colin Walters b64d575
index e2c9bdf..b0f2ec2 100644
Colin Walters b64d575
--- a/configure.ac
Colin Walters b64d575
+++ b/configure.ac
Colin Walters b64d575
@@ -595,7 +595,7 @@ AC_DEFINE_UNQUOTED([DBUS_USE_SYNC], [$have_sync], [Use the gcc __sync extension]
Colin Walters b64d575
 AC_SEARCH_LIBS(socket,[socket network])
Colin Walters b64d575
 AC_CHECK_FUNC(gethostbyname,,[AC_CHECK_LIB(nsl,gethostbyname)])
Colin Walters b64d575
 
Colin Walters b64d575
-AC_CHECK_FUNCS(vsnprintf vasprintf nanosleep usleep setenv clearenv unsetenv socketpair getgrouplist fpathconf setrlimit poll setlocale localeconv strtoll strtoull)
Colin Walters b64d575
+AC_CHECK_FUNCS(vsnprintf vasprintf nanosleep usleep setenv clearenv unsetenv socketpair getgrouplist fpathconf setrlimit poll setlocale localeconv strtoll strtoull issetugid getresuid)
Colin Walters b64d575
 
Colin Walters b64d575
 AC_CHECK_HEADERS([syslog.h])
Colin Walters b64d575
 if test "x$ac_cv_header_syslog_h" = "xyes"; then
Colin Walters b64d575
diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c
Colin Walters b64d575
index 23b9df5..3b9ce31 100644
Colin Walters b64d575
--- a/dbus/dbus-keyring.c
Colin Walters b64d575
+++ b/dbus/dbus-keyring.c
Colin Walters b64d575
@@ -717,6 +717,13 @@ _dbus_keyring_new_for_credentials (DBusCredentials  *credentials,
Colin Walters b64d575
   DBusCredentials *our_credentials;
Colin Walters b64d575
   
Colin Walters b64d575
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
Colin Walters b64d575
+
Colin Walters b64d575
+  if (_dbus_check_setuid ())
Colin Walters b64d575
+    {
Colin Walters b64d575
+      dbus_set_error_const (error, DBUS_ERROR_NOT_SUPPORTED,
Colin Walters b64d575
+                            "Unable to create DBus keyring when setuid");
Colin Walters b64d575
+      return NULL;
Colin Walters b64d575
+    }
Colin Walters b64d575
   
Colin Walters b64d575
   keyring = NULL;
Colin Walters b64d575
   error_set = FALSE;
Colin Walters b64d575
diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
Colin Walters b64d575
index cef8bd3..b4ecc96 100644
Colin Walters b64d575
--- a/dbus/dbus-sysdeps-unix.c
Colin Walters b64d575
+++ b/dbus/dbus-sysdeps-unix.c
Colin Walters b64d575
@@ -3434,6 +3434,13 @@ _dbus_get_autolaunch_address (const char *scope,
Colin Walters b64d575
   DBusString uuid;
Colin Walters b64d575
   dbus_bool_t retval;
Colin Walters b64d575
 
Colin Walters b64d575
+  if (_dbus_check_setuid ())
Colin Walters b64d575
+    {
Colin Walters b64d575
+      dbus_set_error_const (error, DBUS_ERROR_NOT_SUPPORTED,
Colin Walters b64d575
+                            "Unable to autolaunch when setuid");
Colin Walters b64d575
+      return FALSE;
Colin Walters b64d575
+    }
Colin Walters b64d575
+
Colin Walters b64d575
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
Colin Walters b64d575
   retval = FALSE;
Colin Walters b64d575
 
Colin Walters b64d575
@@ -3551,6 +3558,13 @@ _dbus_lookup_launchd_socket (DBusString *socket_path,
Colin Walters b64d575
 
Colin Walters b64d575
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
Colin Walters b64d575
 
Colin Walters b64d575
+  if (_dbus_check_setuid ())
Colin Walters b64d575
+    {
Colin Walters b64d575
+      dbus_set_error_const (error, DBUS_ERROR_NOT_SUPPORTED,
Colin Walters b64d575
+                            "Unable to find launchd socket when setuid");
Colin Walters b64d575
+      return FALSE;
Colin Walters b64d575
+    }
Colin Walters b64d575
+
Colin Walters b64d575
   i = 0;
Colin Walters b64d575
   argv[i] = "launchctl";
Colin Walters b64d575
   ++i;
Colin Walters b64d575
@@ -3591,6 +3605,13 @@ _dbus_lookup_session_address_launchd (DBusString *address, DBusError  *error)
Colin Walters b64d575
   dbus_bool_t valid_socket;
Colin Walters b64d575
   DBusString socket_path;
Colin Walters b64d575
 
Colin Walters b64d575
+  if (_dbus_check_setuid ())
Colin Walters b64d575
+    {
Colin Walters b64d575
+      dbus_set_error_const (error, DBUS_ERROR_NOT_SUPPORTED,
Colin Walters b64d575
+                            "Unable to find launchd socket when setuid");
Colin Walters b64d575
+      return FALSE;
Colin Walters b64d575
+    }
Colin Walters b64d575
+
Colin Walters b64d575
   if (!_dbus_string_init (&socket_path))
Colin Walters b64d575
     {
Colin Walters b64d575
       _DBUS_SET_OOM (error);
Colin Walters b64d575
@@ -4086,4 +4107,57 @@ _dbus_close_all (void)
Colin Walters b64d575
     close (i);
Colin Walters b64d575
 }
Colin Walters b64d575
 
Colin Walters b64d575
+/**
Colin Walters b64d575
+ * **NOTE**: If you modify this function, please also consider making
Colin Walters b64d575
+ * the corresponding change in GLib.  See
Colin Walters b64d575
+ * glib/gutils.c:g_check_setuid().
Colin Walters b64d575
+ *
Colin Walters b64d575
+ * Returns TRUE if the current process was executed as setuid (or an
Colin Walters b64d575
+ * equivalent __libc_enable_secure is available).  See:
Colin Walters b64d575
+ * http://osdir.com/ml/linux.lfs.hardened/2007-04/msg00032.html
Colin Walters b64d575
+ */
Colin Walters b64d575
+dbus_bool_t
Colin Walters b64d575
+_dbus_check_setuid (void)
Colin Walters b64d575
+{
Colin Walters b64d575
+  /* TODO: get __libc_enable_secure exported from glibc.
Colin Walters b64d575
+   * See http://www.openwall.com/lists/owl-dev/2012/08/14/1
Colin Walters b64d575
+   */
Colin Walters b64d575
+#if 0 && defined(HAVE_LIBC_ENABLE_SECURE)
Colin Walters b64d575
+  {
Colin Walters b64d575
+    /* See glibc/include/unistd.h */
Colin Walters b64d575
+    extern int __libc_enable_secure;
Colin Walters b64d575
+    return __libc_enable_secure;
Colin Walters b64d575
+  }
Colin Walters b64d575
+#elif defined(HAVE_ISSETUGID)
Colin Walters b64d575
+  /* BSD: http://www.freebsd.org/cgi/man.cgi?query=issetugid&sektion=2 */
Colin Walters b64d575
+  return issetugid ();
Colin Walters b64d575
+#else
Colin Walters b64d575
+  uid_t ruid, euid, suid; /* Real, effective and saved user ID's */
Colin Walters b64d575
+  gid_t rgid, egid, sgid; /* Real, effective and saved group ID's */
Colin Walters b64d575
+
Colin Walters b64d575
+  static dbus_bool_t check_setuid_initialised;
Colin Walters b64d575
+  static dbus_bool_t is_setuid;
Colin Walters b64d575
+
Colin Walters b64d575
+  if (_DBUS_UNLIKELY (!check_setuid_initialised))
Colin Walters b64d575
+    {
Colin Walters b64d575
+#ifdef HAVE_GETRESUID
Colin Walters b64d575
+      if (getresuid (&ruid, &euid, &suid) != 0 ||
Colin Walters b64d575
+          getresgid (&rgid, &egid, &sgid) != 0)
Colin Walters b64d575
+#endif /* HAVE_GETRESUID */
Colin Walters b64d575
+        {
Colin Walters b64d575
+          suid = ruid = getuid ();
Colin Walters b64d575
+          sgid = rgid = getgid ();
Colin Walters b64d575
+          euid = geteuid ();
Colin Walters b64d575
+          egid = getegid ();
Colin Walters b64d575
+        }
Colin Walters b64d575
+
Colin Walters b64d575
+      check_setuid_initialised = TRUE;
Colin Walters b64d575
+      is_setuid = (ruid != euid || ruid != suid ||
Colin Walters b64d575
+                   rgid != egid || rgid != sgid);
Colin Walters b64d575
+
Colin Walters b64d575
+    }
Colin Walters b64d575
+  return is_setuid;
Colin Walters b64d575
+#endif
Colin Walters b64d575
+}
Colin Walters b64d575
+
Colin Walters b64d575
 /* tests in dbus-sysdeps-util.c */
Colin Walters b64d575
diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
Colin Walters b64d575
index 397520a..bc4951b 100644
Colin Walters b64d575
--- a/dbus/dbus-sysdeps-win.c
Colin Walters b64d575
+++ b/dbus/dbus-sysdeps-win.c
Colin Walters b64d575
@@ -3632,6 +3632,12 @@ _dbus_path_is_absolute (const DBusString *filename)
Colin Walters b64d575
     return FALSE;
Colin Walters b64d575
 }
Colin Walters b64d575
 
Colin Walters b64d575
+dbus_bool_t
Colin Walters b64d575
+_dbus_check_setuid (void)
Colin Walters b64d575
+{
Colin Walters b64d575
+  return FALSE;
Colin Walters b64d575
+}
Colin Walters b64d575
+
Colin Walters b64d575
 /** @} end of sysdeps-win */
Colin Walters b64d575
 /* tests in dbus-sysdeps-util.c */
Colin Walters b64d575
 
Colin Walters b64d575
diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c
Colin Walters b64d575
index 861bfec..04fb8d7 100644
Colin Walters b64d575
--- a/dbus/dbus-sysdeps.c
Colin Walters b64d575
+++ b/dbus/dbus-sysdeps.c
Colin Walters b64d575
@@ -182,6 +182,11 @@ _dbus_setenv (const char *varname,
Colin Walters b64d575
 const char*
Colin Walters b64d575
 _dbus_getenv (const char *varname)
Colin Walters b64d575
 {  
Colin Walters b64d575
+  /* Don't respect any environment variables if the current process is
Colin Walters b64d575
+   * setuid.  This is the equivalent of glibc's __secure_getenv().
Colin Walters b64d575
+   */
Colin Walters b64d575
+  if (_dbus_check_setuid ())
Colin Walters b64d575
+    return NULL;
Colin Walters b64d575
   return getenv (varname);
Colin Walters b64d575
 }
Colin Walters b64d575
 
Colin Walters b64d575
diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
Colin Walters b64d575
index 4052cda..eee9160 100644
Colin Walters b64d575
--- a/dbus/dbus-sysdeps.h
Colin Walters b64d575
+++ b/dbus/dbus-sysdeps.h
Colin Walters b64d575
@@ -87,6 +87,7 @@ typedef struct DBusPipe DBusPipe;
Colin Walters b64d575
 
Colin Walters b64d575
 void _dbus_abort (void) _DBUS_GNUC_NORETURN;
Colin Walters b64d575
 
Colin Walters b64d575
+dbus_bool_t _dbus_check_setuid (void);
Colin Walters b64d575
 const char* _dbus_getenv (const char *varname);
Colin Walters b64d575
 dbus_bool_t _dbus_setenv (const char *varname,
Colin Walters b64d575
 			  const char *value);
Colin Walters b64d575
-- 
Colin Walters b64d575
1.7.11.4
Colin Walters b64d575