Hardening: reject UpdateActivationEnvironment on non-canonical path

UpdateActivationEnvironment is the one dbus-daemon API call that is
obviously dangerous (it is intended for the session bus),
so the default system.conf does not allow anyone to call it.

It has recently come to the D-Bus maintainers' attention that some
system services incorrectly install D-Bus policy rules that allow
arbitrary method calls to any destination as long as they have a
"safe" object path. This is not actually safe: some system services
that use low-level D-Bus bindings like libdbus, including dbus-daemon
itself, provide the same API on all object paths.

Unauthorized calls to UpdateActivationEnvironment are probably just
resource consumption rather than privilege escalation, because on
the system bus, the modified environment is only used to execute
a setuid wrapper that avoids LD_PRELOAD etc. via normal setuid
handling, and sanitizes its own environment before executing
the real service. However, it's safest to assume the worst and
treat it as a potential privilege escalation.

Accordingly, as a hardening measure to avoid privilege escalation on
systems with these faulty services, stop allowing calls to
("/com/example/Whatever",
"org.freedesktop.DBus.UpdateActivationEnvironment")
and only allow ("/org/freedesktop/DBus",
"org.freedesktop.DBus.UpdateActivationEnvironment").

We deliberately continue to provide read-only APIs like
GetConnectionUnixUser at all object paths, for backwards compatibility.

Reviewed-by: Thiago Macieira <thiago@kde.org>
[adjusted commit message to note that this is probably only DoS -smcv]

2 files changed, 37 insertions, 2 deletions

diff --git a/bus/driver.c b/bus/driver.c
index e95a79d9..0b9c3ed5 100644
--- a/bus/driver.c
+++ b/bus/driver.c
@@ -878,6 +878,9 @@ 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;
+
   activation = bus_connection_get_activation (connection);
 
   dbus_message_iter_init (message, &iter);
@@ -1965,6 +1968,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 */