Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: pu

CVE-2012-3524 (#689070) is a local root privilege escalation vulnerability
when setuid-root applications use libdbus without first sanitizing their
caller-supplied environment via a whitelist. Applications thought to be
exploitable include Xorg via the setuid /usr/bin/X if using libhal (so for us,
kFreeBSD but not Linux), and perhaps su/sudo if libpam-systemd or
libpam-ck-connector is used. I wasn't able to exploit libpam-ck-connector
under a squeeze VM, but perhaps I'm doing it wrong.

D-Bus upstream consensus is that it is an application bug to use any
non-trivial library in a setuid application without first clearing the
caller-supplied environment; but having said that, hardening libdbus
against applications with this bug seems wise.

When I asked for security team feedback on #689070, they requested that I
send this to stable via s-p-u.

This is basically the same as the t-p-u option in #689148, but with the
patches adjusted for the older libdbus in stable.

May I upload? I have source+i386 ready to go; proposed debdiff attached.

Regards,
    S

-- System Information:
Debian Release: wheezy/sid
  APT prefers testing-proposed-updates
  APT policy: (500, 'testing-proposed-updates'), (500, 'unstable'), (500, 
'testing'), (500, 'stable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.2.0-4-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
diffstat for dbus-1.2.24 dbus-1.2.24

 changelog                                                               |   12 
 patches/0001-CVE-2012-3524-Don-t-access-environment-variables-or-.patch |  213 ++++++++++
 patches/0002-hardening-Ensure-_dbus_check_setuid-is-initialized-t.patch |   36 +
 patches/0003-hardening-Remove-activation-helper-handling-for-DBUS.patch |   57 ++
 patches/0004-activation-helper-Ensure-DBUS_STARTER_ADDRESS-is-set.patch |   66 +++
 patches/series                                                          |    4 
 6 files changed, 388 insertions(+)

diff -Nru dbus-1.2.24/debian/changelog dbus-1.2.24/debian/changelog
--- dbus-1.2.24/debian/changelog	2011-06-14 20:09:38.000000000 +0100
+++ dbus-1.2.24/debian/changelog	2012-10-04 08:47:17.000000000 +0100
@@ -1,3 +1,15 @@
+dbus (1.2.24-4+squeeze2) stable; urgency=low
+
+  * CVE-2012-3524: apply patches from upstream 1.6.6 to avoid arbitrary
+    code execution in setuid/setgid binaries that incorrectly use libdbus
+    without first sanitizing the environment variables inherited from
+    their less-privileged caller (Closes: #689070).
+    - As per upstream 1.6.8, do not check filesystem capabilities for now,
+      only setuid/setgid, fixing regressions in certain configurations of
+      gnome-keyring
+
+ -- Simon McVittie <s...@debian.org>  Thu, 04 Oct 2012 08:47:10 +0100
+
 dbus (1.2.24-4+squeeze1) stable; urgency=low
 
   * Update Vcs-* control fields to reflect the move to git
diff -Nru dbus-1.2.24/debian/patches/0001-CVE-2012-3524-Don-t-access-environment-variables-or-.patch dbus-1.2.24/debian/patches/0001-CVE-2012-3524-Don-t-access-environment-variables-or-.patch
--- dbus-1.2.24/debian/patches/0001-CVE-2012-3524-Don-t-access-environment-variables-or-.patch	1970-01-01 01:00:00.000000000 +0100
+++ dbus-1.2.24/debian/patches/0001-CVE-2012-3524-Don-t-access-environment-variables-or-.patch	2012-10-04 08:47:17.000000000 +0100
@@ -0,0 +1,213 @@
+From: Colin Walters <walt...@verbum.org>
+Date: Wed, 22 Aug 2012 10:03:34 -0400
+Subject: [PATCH 1/5] 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 <krah...@suse.de>
+Signed-off-by: Colin Walters <walt...@verbum.org>
+[backported to 1.2 -smcv]
+Origin: upstream, 1.2.30, commit:083ebe6126a769491c8ef61f8998117440b48861
+[dbus-sysdeps-win changes skipped for Debian: file not in tarball -smcv]
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=52202
+Bug-CVE: CVE-2012-3524
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=689070
+---
+ configure.in             |    2 +-
+ dbus/dbus-keyring.c      |    7 ++++++
+ dbus/dbus-sysdeps-unix.c |   62 +++++++++++++++++++++++++++++++++++++++++++++-
+ dbus/dbus-sysdeps-win.c  |    6 +++++
+ dbus/dbus-sysdeps.c      |    5 ++++
+ dbus/dbus-sysdeps.h      |    1 +
+ 6 files changed, 81 insertions(+), 2 deletions(-)
+
+diff --git a/configure.in b/configure.in
+index b027f86..28c11de 100644
+--- a/configure.in
++++ b/configure.in
+@@ -430,7 +430,7 @@ AC_DEFINE_UNQUOTED(DBUS_HAVE_ATOMIC_INT_COND, [$have_atomic_inc_cond],
+ 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)
++AC_CHECK_FUNCS(vsnprintf vasprintf nanosleep usleep setenv clearenv unsetenv socketpair getgrouplist fpathconf setrlimit poll issetugid getresuid)
+ 
+ #### Check for broken poll; taken from Glib's configure
+ 
+diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c
+index 6dc1e12..0c32a4f 100644
+--- a/dbus/dbus-keyring.c
++++ b/dbus/dbus-keyring.c
+@@ -718,6 +718,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 b58d09a..d4777c5 100644
+--- a/dbus/dbus-sysdeps-unix.c
++++ b/dbus/dbus-sysdeps-unix.c
+@@ -3124,7 +3124,14 @@ _dbus_get_autolaunch_address (DBusString *address,
+   int i;
+   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;
+ 
+@@ -3510,4 +3517,57 @@ _dbus_get_is_errno_eagain_or_ewouldblock (void)
+   return errno == EAGAIN || errno == EWOULDBLOCK;
+ }
+ 
++/**
++ * **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 02f3123..15fb5cb 100644
+#--- a/dbus/dbus-sysdeps-win.c
+#+++ b/dbus/dbus-sysdeps-win.c
+#@@ -3370,6 +3370,12 @@ _dbus_append_keyring_directory_for_credentials (DBusString      *directory,
+#   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 ccd80cc..c3acc17 100644
+--- a/dbus/dbus-sysdeps.c
++++ b/dbus/dbus-sysdeps.c
+@@ -176,6 +176,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 7817e04..3c374d2 100644
+--- a/dbus/dbus-sysdeps.h
++++ b/dbus/dbus-sysdeps.h
+@@ -97,6 +97,7 @@ typedef struct DBusCredentials DBusCredentials;
+ 
+ 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);
+-- 
+1.7.10.4
+
diff -Nru dbus-1.2.24/debian/patches/0002-hardening-Ensure-_dbus_check_setuid-is-initialized-t.patch dbus-1.2.24/debian/patches/0002-hardening-Ensure-_dbus_check_setuid-is-initialized-t.patch
--- dbus-1.2.24/debian/patches/0002-hardening-Ensure-_dbus_check_setuid-is-initialized-t.patch	1970-01-01 01:00:00.000000000 +0100
+++ dbus-1.2.24/debian/patches/0002-hardening-Ensure-_dbus_check_setuid-is-initialized-t.patch	2012-10-04 08:47:17.000000000 +0100
@@ -0,0 +1,36 @@
+From: Colin Walters <walt...@verbum.org>
+Date: Thu, 27 Sep 2012 21:35:22 -0400
+Subject: [PATCH 2/5] hardening: Ensure _dbus_check_setuid() is initialized
+ threadsafe manner
+
+This is a highly theoretical concern, but we might as well.
+
+https://bugs.freedesktop.org/show_bug.cgi?id=52202
+
+Origin: upstream, 1.2.30, commit:f7b2ec1b2e39318766938b53511311a0d0a71680
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=52202
+Bug-CVE: related to CVE-2012-3524
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=689070
+---
+ dbus/dbus-sysdeps-pthread.c |    5 +++++
+ 1 file changed, 5 insertions(+)
+
+diff --git a/dbus/dbus-sysdeps-pthread.c b/dbus/dbus-sysdeps-pthread.c
+index 46e4204..f21af85 100644
+--- a/dbus/dbus-sysdeps-pthread.c
++++ b/dbus/dbus-sysdeps-pthread.c
+@@ -358,6 +358,11 @@ check_monotonic_clock (void)
+ dbus_bool_t
+ _dbus_threads_init_platform_specific (void)
+ {
++  /* These have static variables, and we need to handle both the case
++   * where dbus_threads_init() has been called and when it hasn't;
++   * so initialize them before any threads are allowed to enter.
++   */
+   check_monotonic_clock ();
++  (void) _dbus_check_setuid ();
+   return dbus_threads_init (&pthread_functions);
+ }
+-- 
+1.7.10.4
+
diff -Nru dbus-1.2.24/debian/patches/0003-hardening-Remove-activation-helper-handling-for-DBUS.patch dbus-1.2.24/debian/patches/0003-hardening-Remove-activation-helper-handling-for-DBUS.patch
--- dbus-1.2.24/debian/patches/0003-hardening-Remove-activation-helper-handling-for-DBUS.patch	1970-01-01 01:00:00.000000000 +0100
+++ dbus-1.2.24/debian/patches/0003-hardening-Remove-activation-helper-handling-for-DBUS.patch	2012-10-04 08:47:17.000000000 +0100
@@ -0,0 +1,57 @@
+From: Colin Walters <walt...@verbum.org>
+Date: Fri, 28 Sep 2012 12:01:56 -0400
+Subject: [PATCH 3/5] hardening: Remove activation helper handling for
+ DBUS_VERBOSE
+
+It's not really useful.
+
+See https://bugs.freedesktop.org/show_bug.cgi?id=52202#c17
+
+Origin: upstream, 1.2.30, commit:b3800b7a666fcefc37eeb25a030241d5809a7246
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=52202
+Bug-CVE: related to CVE-2012-3524
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=689070
+---
+ bus/activation-helper.c |   14 +-------------
+ 1 file changed, 1 insertion(+), 13 deletions(-)
+
+diff --git a/bus/activation-helper.c b/bus/activation-helper.c
+index baba8f0..bc5ed07 100644
+--- a/bus/activation-helper.c
++++ b/bus/activation-helper.c
+@@ -140,18 +140,12 @@ out_all:
+   return desktop_file;
+ }
+ 
+-/* Cleares the environment, except for DBUS_VERBOSE and DBUS_STARTER_x */
++/* Clears the environment, except for DBUS_STARTER_x */
+ static dbus_bool_t
+ clear_environment (DBusError *error)
+ {
+-  const char *debug_env = NULL;
+   const char *starter_env = NULL;
+ 
+-#ifdef DBUS_ENABLE_VERBOSE_MODE
+-  /* are we debugging */
+-  debug_env = _dbus_getenv ("DBUS_VERBOSE");
+-#endif
+-
+   /* we save the starter */
+   starter_env = _dbus_getenv ("DBUS_STARTER_ADDRESS");
+ 
+@@ -165,12 +159,6 @@ clear_environment (DBusError *error)
+     }
+ #endif
+ 
+-#ifdef DBUS_ENABLE_VERBOSE_MODE
+-  /* restore the debugging environment setting if set */
+-  if (debug_env)
+-    _dbus_setenv ("DBUS_VERBOSE", debug_env);
+-#endif
+-
+   /* restore the starter */
+   if (starter_env)
+     _dbus_setenv ("DBUS_STARTER_ADDRESS", starter_env);
+-- 
+1.7.10.4
+
diff -Nru dbus-1.2.24/debian/patches/0004-activation-helper-Ensure-DBUS_STARTER_ADDRESS-is-set.patch dbus-1.2.24/debian/patches/0004-activation-helper-Ensure-DBUS_STARTER_ADDRESS-is-set.patch
--- dbus-1.2.24/debian/patches/0004-activation-helper-Ensure-DBUS_STARTER_ADDRESS-is-set.patch	1970-01-01 01:00:00.000000000 +0100
+++ dbus-1.2.24/debian/patches/0004-activation-helper-Ensure-DBUS_STARTER_ADDRESS-is-set.patch	2012-10-04 08:47:17.000000000 +0100
@@ -0,0 +1,66 @@
+From: Geoffrey Thomas <gtho...@mokafive.com>
+Date: Thu, 27 Sep 2012 22:02:06 -0700
+Subject: [PATCH 4/5] activation-helper: Ensure DBUS_STARTER_ADDRESS is set
+ correctly
+
+The fix for CVE-2012-3524 filters out all environment variables if
+libdbus is used from a setuid program, to prevent various spoofing
+attacks.
+
+Unfortunately, the activation helper is a setuid program linking
+libdbus, and this creates a regression for launched programs using
+DBUS_STARTER_ADDRESS, since it will no longer exist.
+
+Fix this by hardcoding the starter address to the default system bus
+address.
+
+Signed-off-by: Geoffrey Thomas <gtho...@mokafive.com>
+Signed-off-by: Colin Walters <walt...@verbum.org>
+Origin: upstream, 1.2.30, commit:c5c747dd7613d777a05ddb663409eeea4e61ec74
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=52202
+Bug-CVE: related to CVE-2012-3524
+Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=689070
+---
+ bus/activation-helper.c |   16 +++++-----------
+ 1 file changed, 5 insertions(+), 11 deletions(-)
+
+diff --git a/bus/activation-helper.c b/bus/activation-helper.c
+index bc5ed07..bfe832e 100644
+--- a/bus/activation-helper.c
++++ b/bus/activation-helper.c
+@@ -140,15 +140,12 @@ out_all:
+   return desktop_file;
+ }
+ 
+-/* Clears the environment, except for DBUS_STARTER_x */
++/* Clears the environment, except for DBUS_STARTER_x,
++ * which we hardcode to the system bus.
++ */
+ static dbus_bool_t
+ clear_environment (DBusError *error)
+ {
+-  const char *starter_env = NULL;
+-
+-  /* we save the starter */
+-  starter_env = _dbus_getenv ("DBUS_STARTER_ADDRESS");
+-
+ #ifndef ACTIVATION_LAUNCHER_TEST
+   /* totally clear the environment */
+   if (!_dbus_clearenv ())
+@@ -159,11 +156,8 @@ clear_environment (DBusError *error)
+     }
+ #endif
+ 
+-  /* restore the starter */
+-  if (starter_env)
+-    _dbus_setenv ("DBUS_STARTER_ADDRESS", starter_env);
+-
+-  /* set the type, which must be system if we got this far */
++  /* Ensure the bus is set to system */
++  _dbus_setenv ("DBUS_STARTER_ADDRESS", DBUS_SYSTEM_BUS_DEFAULT_ADDRESS);
+   _dbus_setenv ("DBUS_STARTER_BUS_TYPE", "system");
+ 
+   return TRUE;
+-- 
+1.7.10.4
+
diff -Nru dbus-1.2.24/debian/patches/series dbus-1.2.24/debian/patches/series
--- dbus-1.2.24/debian/patches/series	2011-06-12 12:51:34.000000000 +0100
+++ dbus-1.2.24/debian/patches/series	2012-10-04 08:47:17.000000000 +0100
@@ -4,3 +4,7 @@
 11-589662-reload-kqueue.patch
 12-CVE-2010-4352-reject-deeply-nested-variants.patch
 13-629938-_dbus_header_byteswap.patch
+0001-CVE-2012-3524-Don-t-access-environment-variables-or-.patch
+0002-hardening-Ensure-_dbus_check_setuid-is-initialized-t.patch
+0003-hardening-Remove-activation-helper-handling-for-DBUS.patch
+0004-activation-helper-Ensure-DBUS_STARTER_ADDRESS-is-set.patch

Reply via email to