Attachment 67345 Details for Bug 52202 – Filter environment variables

[patch] Filter environment variables

0001-CVE-2012-3524-Don-t-access-environment-variables-or-.patch (text/plain), 7.60 KB, created by Colin Walters on 2012-09-18 17:43:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Walters

Created: 2012-09-18 17:43:09 UTC

Size: 7.60 KB

patch

obsolete

>From 450d975046bbd54271da62ce5fcbe50113f2e453 Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>Date: Wed, 22 Aug 2012 10:03:34 -0400
>Subject: [PATCH] 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>
>---
> configure.ac             |  2 +-
> dbus/dbus-keyring.c      |  7 +++++
> dbus/dbus-sysdeps-unix.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
> dbus/dbus-sysdeps-win.c  |  6 ++++
> dbus/dbus-sysdeps.c      |  5 ++++
> dbus/dbus-sysdeps.h      |  1 +
> 6 files changed, 94 insertions(+), 1 deletion(-)
>
>diff --git a/configure.ac b/configure.ac
>index e2c9bdf..b0f2ec2 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -595,7 +595,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 23b9df5..3b9ce31 100644
>--- 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 cef8bd3..b4ecc96 100644
>--- 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 397520a..bc4951b 100644
>--- 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 861bfec..04fb8d7 100644
>--- 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 4052cda..eee9160 100644
>--- 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);
>-- 
>1.7.11.4
>

Actions: View | Splinter Review

Attachments on bug 52202: 67345 | 67803 | 67804 | 67810 | 67977

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.