Merge tag 'dbus-1.8.14' into dbus-1.8

dbus-1.8.14

6 files changed, 196 insertions, 6 deletions

diff --git a/NEWS b/NEWS
index 250aedb1..4fc8c0ff 100644
--- a/NEWS
+++ b/NEWS
@@ -1,7 +1,33 @@
-D-Bus 1.8.14 (UNRELEASED)
+D-Bus 1.8.14 (2015-01-05)
 ==
 
-...
+The “40lb of roofing nails” release.
+
+Security hardening:
+
+• Do not allow calls to UpdateActivationEnvironment from uids other than
+  the uid of the dbus-daemon. If a system service installs unsafe
+  security policy rules that allow arbitrary method calls
+  (such as CVE-2014-8148) then this prevents memory consumption and
+  possible privilege escalation via UpdateActivationEnvironment.
+
+  We believe that in practice, privilege escalation here is avoided
+  by dbus-daemon-launch-helper sanitizing its environment; but
+  it seems better to be safe.
+
+• Do not allow calls to UpdateActivationEnvironment or the Stats interface
+  on object paths other than /org/freedesktop/DBus. Some system services
+  install unsafe security policy rules that allow arbitrary method calls
+  to any destination, method and interface with a specified object path;
+  while less bad than allowing arbitrary method calls, these security
+  policies are still harmful, since dbus-daemon normally offers the
+  same API on all object paths and other system services might behave
+  similarly.
+
+Other fixes:
+
+• Add missing initialization so GetExtendedTcpTable doesn't crash on
+  Windows Vista SP0 (fd.o #77008, Илья А. Ткаченко)
 
 D-Bus 1.8.12 (2014-11-24)
 ==
diff --git a/bus/driver.c b/bus/driver.c
index e95a79d9..f5d3ebe2 100644
--- a/bus/driver.c
+++ b/bus/driver.c
@@ -878,6 +878,44 @@ bus_driver_handle_update_activation_environment (DBusConnection *connection,
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
+  if (!bus_driver_check_message_is_for_us (message, error))
+    return FALSE;
+
+#ifdef DBUS_UNIX
+    {
+      /* UpdateActivationEnvironment is basically a recipe for privilege
+      * escalation so let's be extra-careful: do not allow the sysadmin
+      * to shoot themselves in the foot. */
+      unsigned long uid;
+
+      if (!dbus_connection_get_unix_user (connection, &uid))
+        {
+          bus_context_log (bus_transaction_get_context (transaction),
+              DBUS_SYSTEM_LOG_SECURITY,
+              "rejected attempt to call UpdateActivationEnvironment by "
+              "unknown uid");
+          dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
+              "rejected attempt to call UpdateActivationEnvironment by "
+              "unknown uid");
+          return FALSE;
+        }
+
+      /* On the system bus, we could in principle allow uid 0 to call
+       * UpdateActivationEnvironment; but they should know better anyway,
+       * and our default system.conf has always forbidden it */
+      if (!_dbus_unix_user_is_process_owner (uid))
+        {
+          bus_context_log (bus_transaction_get_context (transaction),
+              DBUS_SYSTEM_LOG_SECURITY,
+              "rejected attempt to call UpdateActivationEnvironment by uid %lu",
+              uid);
+          dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
+              "rejected attempt to call UpdateActivationEnvironment");
+          return FALSE;
+        }
+    }
+#endif
+
   activation = bus_connection_get_activation (connection);
 
   dbus_message_iter_init (message, &iter);
@@ -1965,6 +2003,38 @@ bus_driver_handle_introspect (DBusConnection *connection,
   return FALSE;
 }
 
+/*
+ * Set @error and return FALSE if the message is not directed to the
+ * dbus-daemon by its canonical object path. This is hardening against
+ * system services with poorly-written security policy files, which
+ * might allow sending dangerously broad equivalence classes of messages
+ * such as "anything with this assumed-to-be-safe object path".
+ *
+ * dbus-daemon is unusual in that it normally ignores the object path
+ * of incoming messages; we need to keep that behaviour for the "read"
+ * read-only method calls like GetConnectionUnixUser for backwards
+ * compatibility, but it seems safer to be more restrictive for things
+ * intended to be root-only or privileged-developers-only.
+ *
+ * It is possible that there are other system services with the same
+ * quirk as dbus-daemon.
+ */
+dbus_bool_t
+bus_driver_check_message_is_for_us (DBusMessage *message,
+                                    DBusError   *error)
+{
+  if (!dbus_message_has_path (message, DBUS_PATH_DBUS))
+    {
+      dbus_set_error (error, DBUS_ERROR_ACCESS_DENIED,
+          "Method '%s' is only available at the canonical object path '%s'",
+          dbus_message_get_member (message), DBUS_PATH_DBUS);
+
+      return FALSE;
+    }
+
+  return TRUE;
+}
+
 dbus_bool_t
 bus_driver_handle_message (DBusConnection *connection,
                            BusTransaction *transaction,
diff --git a/bus/driver.h b/bus/driver.h
index 713b2764..201709c4 100644
--- a/bus/driver.h
+++ b/bus/driver.h
@@ -46,7 +46,7 @@ dbus_bool_t bus_driver_send_service_owner_changed  (const char     *service_name
 						    BusTransaction *transaction,
 						    DBusError      *error);
 dbus_bool_t bus_driver_generate_introspect_string  (DBusString *xml);
-
-
+dbus_bool_t bus_driver_check_message_is_for_us     (DBusMessage *message,
+                                                    DBusError   *error);
 
 #endif /* BUS_DRIVER_H */
diff --git a/bus/stats.c b/bus/stats.c
index 24308eb5..20321e5e 100644
--- a/bus/stats.c
+++ b/bus/stats.c
@@ -29,6 +29,7 @@
 #include <dbus/dbus-connection-internal.h>
 
 #include "connection.h"
+#include "driver.h"
 #include "services.h"
 #include "utils.h"
 
@@ -49,6 +50,9 @@ bus_stats_handle_get_stats (DBusConnection *connection,
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
+  if (!bus_driver_check_message_is_for_us (message, error))
+    return FALSE;
+
   context = bus_transaction_get_context (transaction);
   connections = bus_context_get_connections (context);
 
@@ -131,6 +135,9 @@ bus_stats_handle_get_connection_stats (DBusConnection *caller_connection,
 
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
+  if (!bus_driver_check_message_is_for_us (message, error))
+    return FALSE;
+
   registry = bus_connection_get_registry (caller_connection);
 
   if (! dbus_message_get_args (message, error,
diff --git a/configure.ac b/configure.ac
index 05b58f76..4bd17b7b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3,7 +3,7 @@ AC_PREREQ([2.63])
 
 m4_define([dbus_major_version], [1])
 m4_define([dbus_minor_version], [8])
-m4_define([dbus_micro_version], [13])
+m4_define([dbus_micro_version], [14])
 m4_define([dbus_version],
           [dbus_major_version.dbus_minor_version.dbus_micro_version])
 AC_INIT([dbus],[dbus_version],[https://bugs.freedesktop.org/enter_bug.cgi?product=dbus],[dbus])
@@ -37,7 +37,7 @@ LT_CURRENT=11
 
 ## increment any time the source changes; set to
 ##  0 if you increment CURRENT
-LT_REVISION=9
+LT_REVISION=10
 
 ## increment if any interfaces have been added; set to 0
 ## if any interfaces have been changed or removed. removal has
diff --git a/test/dbus-daemon.c b/test/dbus-daemon.c
index 4b3b61e5..dc0f1317 100644
--- a/test/dbus-daemon.c
+++ b/test/dbus-daemon.c
@@ -458,6 +458,91 @@ test_creds (Fixture *f,
 }
 
 static void
+test_canonical_path_uae (Fixture *f,
+    gconstpointer context)
+{
+  DBusMessage *m = dbus_message_new_method_call (DBUS_SERVICE_DBUS,
+      DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "UpdateActivationEnvironment");
+  DBusPendingCall *pc;
+  DBusMessageIter args_iter;
+  DBusMessageIter arr_iter;
+
+  if (m == NULL)
+    g_error ("OOM");
+
+  dbus_message_iter_init_append (m, &args_iter);
+
+  /* Append an empty a{ss} (string => string dictionary). */
+  if (!dbus_message_iter_open_container (&args_iter, DBUS_TYPE_ARRAY,
+        "{ss}", &arr_iter) ||
+      !dbus_message_iter_close_container (&args_iter, &arr_iter))
+    g_error ("OOM");
+
+  if (!dbus_connection_send_with_reply (f->left_conn, m, &pc,
+                                        DBUS_TIMEOUT_USE_DEFAULT) ||
+      pc == NULL)
+    g_error ("OOM");
+
+  dbus_message_unref (m);
+  m = NULL;
+
+  if (dbus_pending_call_get_completed (pc))
+    pending_call_store_reply (pc, &m);
+  else if (!dbus_pending_call_set_notify (pc, pending_call_store_reply,
+                                          &m, NULL))
+    g_error ("OOM");
+
+  while (m == NULL)
+    test_main_context_iterate (f->ctx, TRUE);
+
+  /* it succeeds */
+  g_assert_cmpint (dbus_message_get_type (m), ==,
+      DBUS_MESSAGE_TYPE_METHOD_RETURN);
+
+  dbus_message_unref (m);
+
+  /* Now try with the wrong object path */
+  m = dbus_message_new_method_call (DBUS_SERVICE_DBUS,
+      "/com/example/Wrong", DBUS_INTERFACE_DBUS, "UpdateActivationEnvironment");
+
+  if (m == NULL)
+    g_error ("OOM");
+
+  dbus_message_iter_init_append (m, &args_iter);
+
+  /* Append an empty a{ss} (string => string dictionary). */
+  if (!dbus_message_iter_open_container (&args_iter, DBUS_TYPE_ARRAY,
+        "{ss}", &arr_iter) ||
+      !dbus_message_iter_close_container (&args_iter, &arr_iter))
+    g_error ("OOM");
+
+  if (!dbus_connection_send_with_reply (f->left_conn, m, &pc,
+                                        DBUS_TIMEOUT_USE_DEFAULT) ||
+      pc == NULL)
+    g_error ("OOM");
+
+  dbus_message_unref (m);
+  m = NULL;
+
+  if (dbus_pending_call_get_completed (pc))
+    pending_call_store_reply (pc, &m);
+  else if (!dbus_pending_call_set_notify (pc, pending_call_store_reply,
+                                          &m, NULL))
+    g_error ("OOM");
+
+  while (m == NULL)
+    test_main_context_iterate (f->ctx, TRUE);
+
+  /* it fails, yielding an error message with one string argument */
+  g_assert_cmpint (dbus_message_get_type (m), ==, DBUS_MESSAGE_TYPE_ERROR);
+  g_assert_cmpstr (dbus_message_get_error_name (m), ==,
+      DBUS_ERROR_ACCESS_DENIED);
+  g_assert_cmpstr (dbus_message_get_signature (m), ==, "s");
+
+  dbus_message_unref (m);
+}
+
+static void
 teardown (Fixture *f,
     gconstpointer context G_GNUC_UNUSED)
 {
@@ -514,6 +599,8 @@ main (int argc,
   g_test_add ("/echo/limited", Fixture, &limited_config,
       setup, test_echo, teardown);
   g_test_add ("/creds", Fixture, NULL, setup, test_creds, teardown);
+  g_test_add ("/canonical-path/uae", Fixture, NULL,
+      setup, test_canonical_path_uae, teardown);
 
   return g_test_run ();
 }