Attachment 606063 Details for Bug 847402 – glib2: use __secure_getenv, don't run dbus-launch as root

[patch] glib2: use __secure_getenv, don't run dbus-launch as root

0001-Use-__secure_getenv-if-available-don-t-run-dbus-laun.patch (text/plain), 13.36 KB, created by Colin Walters on 2012-08-21 21:06:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Walters

Created: 2012-08-21 21:06:06 UTC

Size: 13.36 KB

patch

obsolete

>From dcf827c413b45649f31e90daad8932e5e6e6260b Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>Date: Tue, 21 Aug 2012 15:03:07 -0400
>Subject: [PATCH] Use __secure_getenv() if available, don't run dbus-launch as
> root
>
>If available, use __secure_getenv() from glibc to avoid trusting
>environment variables.
>
>Second, we should explicitly check whether we're setuid, and if so,
>avoid running dbus-launch.
>
>This hardens GLib somewhat against use in a setuid binary.  However, I
>still wouldn't recommend it.
>---
> configure.ac                 |  4 ++--
> gio/gdbusaddress.c           | 15 ++++++++++-----
> gio/gdbusauthmechanismsha1.c |  5 +++--
> gio/gdbusconnection.c        |  3 ++-
> gio/gdbusprivate.c           |  3 ++-
> gio/giomodule.c              |  5 +++--
> gio/gunionvolumemonitor.c    |  3 ++-
> gio/inotify/inotify-diag.c   |  3 ++-
> glib/genviron.c              | 17 ++++++++++++++++
> glib/glib-private.c          |  5 ++++-
> glib/glib-private.h          |  7 +++++++
> glib/gutils.c                | 46 +++++++++++++++++++++++++++++++++++++++++---
> 12 files changed, 97 insertions(+), 19 deletions(-)
>
>diff --git a/configure.ac b/configure.ac
>index 9e9d714..ad10ba7 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -586,7 +586,7 @@ AC_TRY_COMPILE([#include <dirent.h>], [DIR *dir;],
> # Checks for library functions.
> AC_FUNC_VPRINTF
> AC_FUNC_ALLOCA
>-AC_CHECK_FUNCS(mmap posix_memalign memalign valloc fsync pipe2)
>+AC_CHECK_FUNCS(mmap posix_memalign memalign valloc fsync pipe2 __secure_getenv)
> AC_CHECK_FUNCS(atexit on_exit timegm gmtime_r)
> 
> AC_CHECK_SIZEOF(char)
>@@ -985,7 +985,7 @@ AC_MSG_RESULT(unsigned $glib_size_type)
> 
> # Check for some functions
> AC_CHECK_FUNCS(lstat strerror strsignal memmove vsnprintf stpcpy strcasecmp strncasecmp poll getcwd vasprintf setenv unsetenv getc_unlocked readlink symlink fdwalk memmem)
>-AC_CHECK_FUNCS(chown lchmod lchown fchmod fchown link utimes getgrgid getpwuid)
>+AC_CHECK_FUNCS(chown lchmod lchown fchmod fchown link utimes getgrgid getpwuid getresuid)
> AC_CHECK_FUNCS(getmntent_r setmntent endmntent hasmntopt getfsstat getvfsstat)
> # Check for high-resolution sleep functions
> AC_CHECK_FUNCS(splice)
>diff --git a/gio/gdbusaddress.c b/gio/gdbusaddress.c
>index fac22b7..9ccda8e 100644
>--- a/gio/gdbusaddress.c
>+++ b/gio/gdbusaddress.c
>@@ -37,6 +37,7 @@
> #include "giostream.h"
> #include "gasyncresult.h"
> #include "gsimpleasyncresult.h"
>+#include "glib-private.h"
> #include "gdbusprivate.h"
> #include "giomodule-priv.h"
> #include "gdbusdaemon.h"
>@@ -1014,6 +1015,10 @@ get_session_address_dbus_launch (GError **error)
>   gchar *old_dbus_verbose;
>   gboolean restore_dbus_verbose;
> 
>+  /* Don't run binaries as root if we're setuid. */
>+  if (GLIB_PRIVATE_CALL (g_check_setuid) ())
>+    return NULL;
>+
>   ret = NULL;
>   machine_id = NULL;
>   command_line = NULL;
>@@ -1050,7 +1055,7 @@ get_session_address_dbus_launch (GError **error)
>     {
>       _g_dbus_debug_print_lock ();
>       g_print ("GDBus-debug:Address: Running `%s' to get bus address (possibly autolaunching)\n", command_line);
>-      old_dbus_verbose = g_strdup (g_getenv ("DBUS_VERBOSE"));
>+      old_dbus_verbose = g_strdup (GLIB_PRIVATE_CALL (g_secure_getenv) ("DBUS_VERBOSE"));
>       restore_dbus_verbose = TRUE;
>       g_setenv ("DBUS_VERBOSE", "1", TRUE);
>       _g_dbus_debug_print_unlock ();
>@@ -1482,7 +1487,7 @@ g_dbus_address_get_for_bus_sync (GBusType       bus_type,
>             case 2: k = "DBUS_STARTER_BUS_TYPE"; break;
>             default: g_assert_not_reached ();
>             }
>-          v = g_getenv (k);
>+          v = GLIB_PRIVATE_CALL (g_secure_getenv) (k);
>           g_print ("GDBus-debug:Address: env var %s", k);
>           if (v != NULL)
>             g_print ("=`%s'\n", v);
>@@ -1495,7 +1500,7 @@ g_dbus_address_get_for_bus_sync (GBusType       bus_type,
>   switch (bus_type)
>     {
>     case G_BUS_TYPE_SYSTEM:
>-      ret = g_strdup (g_getenv ("DBUS_SYSTEM_BUS_ADDRESS"));
>+      ret = g_strdup (GLIB_PRIVATE_CALL (g_secure_getenv) ("DBUS_SYSTEM_BUS_ADDRESS"));
>       if (ret == NULL)
>         {
>           ret = g_strdup ("unix:path=/var/run/dbus/system_bus_socket");
>@@ -1503,7 +1508,7 @@ g_dbus_address_get_for_bus_sync (GBusType       bus_type,
>       break;
> 
>     case G_BUS_TYPE_SESSION:
>-      ret = g_strdup (g_getenv ("DBUS_SESSION_BUS_ADDRESS"));
>+      ret = g_strdup (GLIB_PRIVATE_CALL (g_secure_getenv) ("DBUS_SESSION_BUS_ADDRESS"));
>       if (ret == NULL)
>         {
>           ret = get_session_address_platform_specific (&local_error);
>@@ -1511,7 +1516,7 @@ g_dbus_address_get_for_bus_sync (GBusType       bus_type,
>       break;
> 
>     case G_BUS_TYPE_STARTER:
>-      starter_bus = g_getenv ("DBUS_STARTER_BUS_TYPE");
>+      starter_bus = GLIB_PRIVATE_CALL (g_secure_getenv) ("DBUS_STARTER_BUS_TYPE");
>       if (g_strcmp0 (starter_bus, "session") == 0)
>         {
>           ret = g_dbus_address_get_for_bus_sync (G_BUS_TYPE_SESSION, cancellable, &local_error);
>diff --git a/gio/gdbusauthmechanismsha1.c b/gio/gdbusauthmechanismsha1.c
>index 4729208..ababd6c 100644
>--- a/gio/gdbusauthmechanismsha1.c
>+++ b/gio/gdbusauthmechanismsha1.c
>@@ -38,6 +38,7 @@
> #include "gdbusauthmechanismsha1.h"
> #include "gcredentials.h"
> #include "gdbuserror.h"
>+#include "glib-private.h"
> #include "gioenumtypes.h"
> #include "gioerror.h"
> #include "gdbusprivate.h"
>@@ -240,7 +241,7 @@ ensure_keyring_directory (GError **error)
> 
>   g_return_val_if_fail (error == NULL || *error == NULL, NULL);
> 
>-  e = g_getenv ("G_DBUS_COOKIE_SHA1_KEYRING_DIR");
>+  e = GLIB_PRIVATE_CALL (g_secure_getenv) ("G_DBUS_COOKIE_SHA1_KEYRING_DIR");
>   if (e != NULL)
>     {
>       path = g_strdup (e);
>@@ -254,7 +255,7 @@ ensure_keyring_directory (GError **error)
> 
>   if (g_file_test (path, G_FILE_TEST_EXISTS | G_FILE_TEST_IS_DIR))
>     {
>-      if (g_getenv ("G_DBUS_COOKIE_SHA1_KEYRING_DIR_IGNORE_PERMISSION") == NULL)
>+      if (GLIB_PRIVATE_CALL (g_secure_getenv) ("G_DBUS_COOKIE_SHA1_KEYRING_DIR_IGNORE_PERMISSION") == NULL)
>         {
> #ifdef G_OS_UNIX
>           struct stat statbuf;
>diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c
>index afc4ed4..f3b9ff7 100644
>--- a/gio/gdbusconnection.c
>+++ b/gio/gdbusconnection.c
>@@ -120,6 +120,7 @@
> #include "gdbusintrospection.h"
> #include "gdbusmethodinvocation.h"
> #include "gdbusprivate.h"
>+#include "glib-private.h"
> #include "gdbusauthobserver.h"
> #include "ginitable.h"
> #include "gasyncinitable.h"
>@@ -6734,7 +6735,7 @@ message_bus_get_singleton (GBusType   bus_type,
>       break;
> 
>     case G_BUS_TYPE_STARTER:
>-      starter_bus = g_getenv ("DBUS_STARTER_BUS_TYPE");
>+      starter_bus = GLIB_PRIVATE_CALL (g_secure_getenv) ("DBUS_STARTER_BUS_TYPE");
>       if (g_strcmp0 (starter_bus, "session") == 0)
>         {
>           ret = message_bus_get_singleton (G_BUS_TYPE_SESSION, error);
>diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c
>index a3cf9d4..89853d0 100644
>--- a/gio/gdbusprivate.c
>+++ b/gio/gdbusprivate.c
>@@ -37,6 +37,7 @@
> #include "gasyncresult.h"
> #include "gsimpleasyncresult.h"
> #include "ginputstream.h"
>+#include "glib-private.h"
> #include "gmemoryinputstream.h"
> #include "giostream.h"
> #include "gsocketcontrolmessage.h"
>@@ -1924,7 +1925,7 @@ _g_dbus_initialize (void)
>       g_dbus_error_domain = G_DBUS_ERROR;
>       (g_dbus_error_domain); /* To avoid -Wunused-but-set-variable */
> 
>-      debug = g_getenv ("G_DBUS_DEBUG");
>+      debug = GLIB_PRIVATE_CALL (g_secure_getenv) ("G_DBUS_DEBUG");
>       if (debug != NULL)
>         {
>           const GDebugKey keys[] = {
>diff --git a/gio/giomodule.c b/gio/giomodule.c
>index 37a9e70..a954d33 100644
>--- a/gio/giomodule.c
>+++ b/gio/giomodule.c
>@@ -26,6 +26,7 @@
> 
> #include "giomodule.h"
> #include "giomodule-priv.h"
>+#include "glib-private.h"
> #include "glocalfilemonitor.h"
> #include "glocaldirectorymonitor.h"
> #include "gnativevolumemonitor.h"
>@@ -717,7 +718,7 @@ _g_io_module_get_default (const gchar         *extension_point,
>       return NULL;
>     }
> 
>-  use_this = envvar ? g_getenv (envvar) : NULL;
>+  use_this = envvar ? GLIB_PRIVATE_CALL (g_secure_getenv) (envvar) : NULL;
>   if (use_this)
>     {
>       preferred = g_io_extension_point_get_extension_by_name (ep, use_this);
>@@ -887,7 +888,7 @@ _g_io_modules_ensure_loaded (void)
>       scope = g_io_module_scope_new (G_IO_MODULE_SCOPE_BLOCK_DUPLICATES);
> 
>       /* First load any overrides, extras */
>-      module_path = g_getenv ("GIO_EXTRA_MODULES");
>+      module_path = GLIB_PRIVATE_CALL (g_secure_getenv) ("GIO_EXTRA_MODULES");
>       if (module_path)
> 	{
> 	  gchar **paths;
>diff --git a/gio/gunionvolumemonitor.c b/gio/gunionvolumemonitor.c
>index cd814ad..8083045 100644
>--- a/gio/gunionvolumemonitor.c
>+++ b/gio/gunionvolumemonitor.c
>@@ -29,6 +29,7 @@
> 
> #include <glib.h>
> #include "gunionvolumemonitor.h"
>+#include "glib-private.h"
> #include "gmountprivate.h"
> #include "giomodule-priv.h"
> #ifdef G_OS_UNIX
>@@ -433,7 +434,7 @@ get_default_native_class (gpointer data)
> 
>   native_class_out = data;
>   
>-  use_this = g_getenv ("GIO_USE_VOLUME_MONITOR");
>+  use_this = GLIB_PRIVATE_CALL (g_secure_getenv) ("GIO_USE_VOLUME_MONITOR");
>   
>   /* Ensure vfs in modules loaded */
>   _g_io_modules_ensure_loaded ();
>diff --git a/gio/inotify/inotify-diag.c b/gio/inotify/inotify-diag.c
>index 937ebd7..2680ab8 100644
>--- a/gio/inotify/inotify-diag.c
>+++ b/gio/inotify/inotify-diag.c
>@@ -25,6 +25,7 @@
> 
> #include "config.h"
> #include <glib.h>
>+#include "glib-private.h"
> #include <sys/types.h>
> #include <unistd.h>
> #include "inotify-missing.h"
>@@ -67,7 +68,7 @@ id_dump (gpointer userdata)
> void
> _id_startup (void)
> {
>-  if (!g_getenv ("GVFS_INOTIFY_DIAG"))
>+  if (!(GLIB_PRIVATE_CALL (g_secure_getenv) ("GVFS_INOTIFY_DIAG")))
>     return;
> 	
>   g_timeout_add (DIAG_DUMP_TIME, id_dump, NULL);
>diff --git a/glib/genviron.c b/glib/genviron.c
>index aed4b63..4a9f266 100644
>--- a/glib/genviron.c
>+++ b/glib/genviron.c
>@@ -40,6 +40,7 @@
> #include <windows.h>
> #endif
> 
>+#include "glib-private.h"
> #include "gmem.h"
> #include "gmessages.h"
> #include "gstrfuncs.h"
>@@ -708,5 +709,21 @@ g_unsetenv (const gchar *variable)
> 
> #endif  /* G_OS_WIN32 */
> 
>+/* Private API call
>+ *
>+ * Intended to help prevent setuid binaries from being exploited
>+ * indirectly via GLib.
>+ */
>+const char *
>+g_secure_getenv (const char *envname)
>+{
>+#ifdef HAVE___SECURE_GETENV
>+  return __secure_getenv (envname);
>+#else
>+  return g_getenv (envname);
>+#endif
>+}
>+
>+
> /* Epilogue {{{1 */
> /* vim: set foldmethod=marker: */
>diff --git a/glib/glib-private.c b/glib/glib-private.c
>index 3946e77..3be984b 100644
>--- a/glib/glib-private.c
>+++ b/glib/glib-private.c
>@@ -38,7 +38,10 @@ glib__private__ (void)
>     g_wakeup_signal,
>     g_wakeup_acknowledge,
> 
>-    g_get_worker_context
>+    g_get_worker_context,
>+
>+    g_secure_getenv,
>+    g_check_setuid
>   };
> 
>   return &table;
>diff --git a/glib/glib-private.h b/glib/glib-private.h
>index fde0be8..3dff99a 100644
>--- a/glib/glib-private.h
>+++ b/glib/glib-private.h
>@@ -25,6 +25,10 @@
> 
> G_GNUC_INTERNAL
> GMainContext *          g_get_worker_context            (void);
>+G_GNUC_INTERNAL
>+const char *            g_secure_getenv                 (const char *envname);
>+G_GNUC_INTERNAL
>+gboolean                g_check_setuid                  (void);
> 
> #define GLIB_PRIVATE_CALL(symbol) (glib__private__()->symbol)
> 
>@@ -40,6 +44,9 @@ typedef struct {
>   /* See gmain.c */
>   GMainContext *        (* g_get_worker_context)        (void);
>   /* Add other private functions here, initialize them in glib-private.c */
>+
>+  const char *          (* g_secure_getenv)             (const char *envname);
>+  gboolean              (* g_check_setuid)              (void);
> } GLibPrivateVTable;
> 
> GLibPrivateVTable *glib__private__ (void);
>diff --git a/glib/gutils.c b/glib/gutils.c
>index 38b5e44..adf8d21 100644
>--- a/glib/gutils.c
>+++ b/glib/gutils.c
>@@ -60,6 +60,7 @@
> #include "gutils.h"
> 
> #include "glib-init.h"
>+#include "glib-private.h"
> #include "genviron.h"
> #include "gfileutils.h"
> #include "ggettext.h"
>@@ -659,18 +660,18 @@ g_get_any_init_do (void)
> {
>   gchar hostname[100];
> 
>-  g_tmp_dir = g_strdup (g_getenv ("TMPDIR"));
>+  g_tmp_dir = g_strdup (GLIB_PRIVATE_CALL (g_secure_getenv) ("TMPDIR"));
> 
>   if (g_tmp_dir == NULL || *g_tmp_dir == '\0')
>     {
>       g_free (g_tmp_dir);
>-      g_tmp_dir = g_strdup (g_getenv ("TMP"));
>+      g_tmp_dir = g_strdup (GLIB_PRIVATE_CALL (g_secure_getenv) ("TMP"));
>     }
> 
>   if (g_tmp_dir == NULL || *g_tmp_dir == '\0')
>     {
>       g_free (g_tmp_dir);
>-      g_tmp_dir = g_strdup (g_getenv ("TEMP"));
>+      g_tmp_dir = g_strdup (GLIB_PRIVATE_CALL (g_secure_getenv) ("TEMP"));
>     }
> 
> #ifdef G_OS_WIN32
>@@ -2409,3 +2410,42 @@ g_get_tmp_dir (void)
> }
> 
> #endif
>+
>+/* Private API:
>+ *
>+ * Returns %TRUE if the current process was executed as
>+ * setuid.
>+ */ 
>+gboolean
>+g_check_setuid (void)
>+{
>+/* this isn't at all relevant on MS Windows and doesn't compile ... --hb */
>+#ifndef G_OS_WIN32
>+  uid_t ruid, euid, suid; /* Real, effective and saved user ID's */
>+  gid_t rgid, egid, sgid; /* Real, effective and saved group ID's */
>+  
>+#ifdef HAVE_GETRESUID
>+  /* These aren't in the header files, so we prototype them here.
>+   */
>+  int getresuid(uid_t *ruid, uid_t *euid, uid_t *suid);
>+  int getresgid(gid_t *rgid, gid_t *egid, gid_t *sgid);
>+
>+  if (getresuid (&ruid, &euid, &suid) != 0 ||
>+      getresgid (&rgid, &egid, &sgid) != 0)
>+#endif /* HAVE_GETRESUID */
>+    {
>+      suid = ruid = getuid ();
>+      sgid = rgid = getgid ();
>+      euid = geteuid ();
>+      egid = getegid ();
>+    }
>+
>+  if (ruid != euid || ruid != suid ||
>+      rgid != egid || rgid != sgid)
>+    {
>+      return TRUE;
>+    }
>+#endif
>+  return FALSE;
>+}
>+
>-- 
>1.7.11.4
>

Actions: View | Diff

Attachments on bug 847402: 606063 | 606064 | 606084 | 606355 | 606392