bus: Assign a serial number for messages from the driver

Normally, it's enough to rely on a message being given a serial number
by the DBusConnection just before it is actually sent. However, in the
rare case where the policy blocks the driver from sending a message
(due to a deny rule or the outgoing message quota being full), we need
to get a valid serial number sooner, so that we can copy it into the
DBUS_HEADER_FIELD_REPLY_SERIAL field (which is mandatory) in the error
message sent to monitors. Otherwise, the dbus-daemon will crash with
an assertion failure if at least one Monitoring client is attached,
because zero is not a valid serial number to copy.

This fixes a denial-of-service vulnerability: if a privileged user is
monitoring the well-known system bus using a Monitoring client like
dbus-monitor or `busctl monitor`, then an unprivileged user can cause
denial-of-service by triggering this crash. A mitigation for this
vulnerability is to avoid attaching Monitoring clients to the system
bus when they are not needed. If there are no Monitoring clients, then
the vulnerable code is not reached.

Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: dbus/dbus#457
(cherry picked from commit b159849e031000d1dbc1ab876b5fc78a3ce9b534)

diff --git a/bus/connection.c b/bus/connection.c
index add1c8b82871877806320647e533ff5b8647677c..95bc6c0a5742c3bfc71ac4459b4810fa29558069 100644 (file)
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -2374,6 +2374,21 @@ bus_transaction_send_from_driver (BusTransaction *transaction,
   if (!dbus_message_set_sender (message, DBUS_SERVICE_DBUS))
     return FALSE;
 
+  /* Make sure the message has a non-zero serial number, otherwise
+   * bus_transaction_capture_error_reply() will not be able to mock up
+   * a corresponding reply for it. Normally this would be delayed until
+   * the first time we actually send the message out from a
+   * connection, when the transaction is committed, but that's too late
+   * in this case.
+   */
+  if (dbus_message_get_serial (message) == 0)
+    {
+      dbus_uint32_t next_serial;
+
+      next_serial = _dbus_connection_get_next_client_serial (connection);
+      dbus_message_set_serial (message, next_serial);
+    }
+
   if (bus_connection_is_active (connection))
     {
       if (!dbus_message_set_destination (message,

diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h
index d6947d181dbedeb72bd49c97ea087a8b93a714ca..df3c7010d9f8292251a8ea68be85ae3ebf3e0dba 100644 (file)
--- a/dbus/dbus-connection-internal.h
+++ b/dbus/dbus-connection-internal.h
@@ -55,6 +55,8 @@ DBUS_PRIVATE_EXPORT
 DBusConnection *  _dbus_connection_ref_unlocked                (DBusConnection     *connection);
 DBUS_PRIVATE_EXPORT
 void              _dbus_connection_unref_unlocked              (DBusConnection     *connection);
+DBUS_PRIVATE_EXPORT
+dbus_uint32_t     _dbus_connection_get_next_client_serial      (DBusConnection *connection);
 void              _dbus_connection_queue_received_message_link (DBusConnection     *connection,
                                                                 DBusList           *link);
 dbus_bool_t       _dbus_connection_has_messages_to_send_unlocked (DBusConnection     *connection);

diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index a61e56a6d291826049f12f430ee9da682d3d5bb8..e6fedbbe5b1450dc89f1e5c1ed141a9b9228abb1 100644 (file)
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -1459,7 +1459,16 @@ _dbus_connection_unref_unlocked (DBusConnection *connection)
     _dbus_connection_last_unref (connection);
 }
 
-static dbus_uint32_t
+/**
+ * Allocate and return the next non-zero serial number for outgoing messages.
+ *
+ * This method is only valid to call from single-threaded code, such as
+ * the dbus-daemon, or with the connection lock held.
+ *
+ * @param connection the connection
+ * @returns A suitable serial number for the next message to be sent on the connection.
+ */
+dbus_uint32_t
 _dbus_connection_get_next_client_serial (DBusConnection *connection)
 {
   dbus_uint32_t serial;