From caf532c3d0e22b9fa240780119df88a0494f22a7 Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Mon, 19 Sep 2022 14:30:35 +0200 Subject: dbus-connection: Add builtin_filters_enabled flag --- dbus/dbus-connection-internal.h | 3 +++ dbus/dbus-connection.c | 47 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h index de4e55b1..d6947d18 100644 --- a/dbus/dbus-connection-internal.h +++ b/dbus/dbus-connection-internal.h @@ -120,6 +120,9 @@ dbus_bool_t _dbus_connection_get_linux_security_label (DBusConnectio char **label_p); DBUS_PRIVATE_EXPORT DBusCredentials *_dbus_connection_get_credentials (DBusConnection *connection); +DBUS_PRIVATE_EXPORT +void _dbus_connection_set_builtin_filters_enabled (DBusConnection *connection, + dbus_bool_t value); /* if DBUS_ENABLE_STATS */ DBUS_PRIVATE_EXPORT diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 159dbe1b..a61e56a6 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -316,6 +316,8 @@ struct DBusConnection unsigned int exit_on_disconnect : 1; /**< If #TRUE, exit after handling disconnect signal */ + unsigned int builtin_filters_enabled : 1; /**< If #TRUE, handle org.freedesktop.DBus.Peer messages automatically, whether they have a bus name or not */ + unsigned int route_peer_messages : 1; /**< If #TRUE, if org.freedesktop.DBus.Peer messages have a bus name, don't handle them automatically */ unsigned int disconnected_message_arrived : 1; /**< We popped or are dispatching the disconnected message. @@ -1343,6 +1345,7 @@ _dbus_connection_new_for_transport (DBusTransport *transport) connection->objects = objects; connection->exit_on_disconnect = FALSE; connection->shareable = FALSE; + connection->builtin_filters_enabled = TRUE; connection->route_peer_messages = FALSE; connection->disconnected_message_arrived = FALSE; connection->disconnected_message_processed = FALSE; @@ -4657,10 +4660,14 @@ dbus_connection_dispatch (DBusConnection *connection) goto out; } - result = _dbus_connection_run_builtin_filters_unlocked_no_update (connection, message); - if (result != DBUS_HANDLER_RESULT_NOT_YET_HANDLED) - goto out; - + /* If skipping builtin filters, we are probably a monitor. */ + if (connection->builtin_filters_enabled) + { + result = _dbus_connection_run_builtin_filters_unlocked_no_update (connection, message); + if (result != DBUS_HANDLER_RESULT_NOT_YET_HANDLED) + goto out; + } + if (!_dbus_list_copy (&connection->filter_list, &filter_list_copy)) { _dbus_connection_release_dispatch (connection); @@ -5532,6 +5539,38 @@ dbus_connection_set_allow_anonymous (DBusConnection *connection, CONNECTION_UNLOCK (connection); } +/** + * Enables the builtin filtering of messages. + * + * Currently the only filtering implemented by libdbus and mandated by the spec + * is that of peer messages. + * + * If #TRUE, #DBusConnection automatically handles all messages to the + * org.freedesktop.DBus.Peer interface. For monitors this can break the + * specification if the response is sending a message. + * + * If #FALSE, the result is similar to calling + * dbus_connection_set_route_peer_messages() with argument TRUE, but + * messages with a NULL destination are also dispatched to the + * application instead of being passed to the built-in filters. + * + * If a normal application disables this flag, it can break things badly. So + * only unset this if you are a monitor. + * + * @param connection the connection + * @param value #TRUE to pass through org.freedesktop.DBus.Peer messages + */ +void +_dbus_connection_set_builtin_filters_enabled (DBusConnection *connection, + dbus_bool_t value) +{ + _dbus_assert (connection != NULL); + + CONNECTION_LOCK (connection); + connection->builtin_filters_enabled = value; + CONNECTION_UNLOCK (connection); +} + /** * * Normally #DBusConnection automatically handles all messages to the -- cgit v1.2.1 From c4573311dd7e9d39d2b5c177eac8d9ff6316e4fe Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Mon, 19 Sep 2022 14:31:51 +0200 Subject: dbus-monitor: Disable automatic message filtering --- test/monitor.c | 4 +++- tools/dbus-monitor.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/test/monitor.c b/test/monitor.c index eb11eb81..d8005c87 100644 --- a/test/monitor.c +++ b/test/monitor.c @@ -28,6 +28,8 @@ #include +#include "dbus/dbus-connection-internal.h" + #include "test-utils-glib.h" typedef struct { @@ -505,7 +507,7 @@ become_monitor (Fixture *f, int i; dbus_uint32_t zero = 0; - dbus_connection_set_route_peer_messages (f->monitor, TRUE); + _dbus_connection_set_builtin_filters_enabled (f->monitor, FALSE); if (config == NULL) config = f->config; diff --git a/tools/dbus-monitor.c b/tools/dbus-monitor.c index fcc923ee..bd111488 100644 --- a/tools/dbus-monitor.c +++ b/tools/dbus-monitor.c @@ -21,6 +21,7 @@ #include +#include "dbus/dbus-connection-internal.h" #include "dbus/dbus-internals.h" #include @@ -494,7 +495,7 @@ main (int argc, char *argv[]) /* Receive o.fd.Peer messages as normal messages, rather than having * libdbus handle them internally, which is the wrong thing for * a monitor */ - dbus_connection_set_route_peer_messages (connection, TRUE); + _dbus_connection_set_builtin_filters_enabled (connection, FALSE); if (!dbus_connection_add_filter (connection, filter_func, _DBUS_INT_TO_POINTER (binary_mode), NULL)) -- cgit v1.2.1 From 37a65e0a36355d9a079ed086424a6bc11abb5d7d Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Mon, 10 Oct 2022 19:44:29 +0200 Subject: dbus-connection: Test built-in filters --- test/loopback.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/test/loopback.c b/test/loopback.c index f89f5a95..bca0d3c2 100644 --- a/test/loopback.c +++ b/test/loopback.c @@ -61,6 +61,37 @@ assert_no_error (const DBusError *e) g_error ("expected success but got error: %s: %s", e->name, e->message); } +/* these are macros so they get the right line number */ + +#define assert_method_reply(m, sender, destination, signature) \ +do { \ + g_assert_cmpstr (dbus_message_type_to_string (dbus_message_get_type (m)), \ + ==, dbus_message_type_to_string (DBUS_MESSAGE_TYPE_METHOD_RETURN)); \ + g_assert_cmpstr (dbus_message_get_sender (m), ==, sender); \ + g_assert_cmpstr (dbus_message_get_destination (m), ==, destination); \ + g_assert_cmpstr (dbus_message_get_path (m), ==, NULL); \ + g_assert_cmpstr (dbus_message_get_interface (m), ==, NULL); \ + g_assert_cmpstr (dbus_message_get_member (m), ==, NULL); \ + g_assert_cmpstr (dbus_message_get_signature (m), ==, signature); \ + g_assert_cmpint (dbus_message_get_serial (m), !=, 0); \ + g_assert_cmpint (dbus_message_get_reply_serial (m), !=, 0); \ +} while (0) + +#define assert_error_reply(m, sender, destination, error_name) \ +do { \ + g_assert_cmpstr (dbus_message_type_to_string (dbus_message_get_type (m)), \ + ==, dbus_message_type_to_string (DBUS_MESSAGE_TYPE_ERROR)); \ + g_assert_cmpstr (dbus_message_get_sender (m), ==, sender); \ + g_assert_cmpstr (dbus_message_get_destination (m), ==, destination); \ + g_assert_cmpstr (dbus_message_get_error_name (m), ==, error_name); \ + g_assert_cmpstr (dbus_message_get_path (m), ==, NULL); \ + g_assert_cmpstr (dbus_message_get_interface (m), ==, NULL); \ + g_assert_cmpstr (dbus_message_get_member (m), ==, NULL); \ + g_assert_cmpstr (dbus_message_get_signature (m), ==, "s"); \ + g_assert_cmpint (dbus_message_get_serial (m), !=, 0); \ + g_assert_cmpint (dbus_message_get_reply_serial (m), !=, 0); \ +} while (0) + static DBusHandlerResult server_message_cb (DBusConnection *server_conn, DBusMessage *message, @@ -415,6 +446,59 @@ test_message (Fixture *f, dbus_clear_message (&outgoing); } +static void +test_builtin_filters (Fixture *f, + gconstpointer addr) +{ + dbus_bool_t have_mem; + dbus_uint32_t serial; + DBusMessage *ping; + DBusMessage *m; + + if (f->skip) + return; + + test_connect (f, addr); + + ping = dbus_message_new_method_call (NULL, "/foo", DBUS_INTERFACE_PEER, + "Ping"); + + _dbus_connection_set_builtin_filters_enabled (f->client_conn, TRUE); + + have_mem = dbus_connection_send (f->server_conn, ping, &serial); + g_assert (have_mem); + g_assert (serial != 0); + + while (g_queue_get_length (&f->server_messages) < 1) + test_main_context_iterate (f->ctx, TRUE); + + m = g_queue_pop_head (&f->server_messages); + assert_method_reply (m, NULL, NULL, ""); + dbus_message_unref (m); + + m = g_queue_pop_head (&f->server_messages); + g_assert (m == NULL); + + _dbus_connection_set_builtin_filters_enabled (f->client_conn, FALSE); + + have_mem = dbus_connection_send (f->server_conn, ping, &serial); + g_assert (have_mem); + g_assert (serial != 0); + + while (g_queue_get_length (&f->server_messages) < 1) + test_main_context_iterate (f->ctx, TRUE); + + m = g_queue_pop_head (&f->server_messages); + assert_error_reply (m, NULL, NULL, + "org.freedesktop.DBus.Error.UnknownMethod"); + dbus_message_unref (m); + + m = g_queue_pop_head (&f->server_messages); + g_assert (m == NULL); + + dbus_message_unref (ping); +} + static void teardown (Fixture *f, gconstpointer addr G_GNUC_UNUSED) @@ -523,6 +607,9 @@ main (int argc, test_bad_guid, teardown); #endif + g_test_add ("/builtin-filters", Fixture, "tcp:host=127.0.0.1", setup, + test_builtin_filters, teardown); + ret = g_test_run (); dbus_shutdown (); return ret; -- cgit v1.2.1 From 7a0f050a54d6017302e5aa246e3eda909c028004 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 30 Sep 2022 15:08:20 +0100 Subject: dbus-message: Report OOM as OOM, not InvalidArgs Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/420 Signed-off-by: Simon McVittie (cherry picked from commit 3c0e63c10a63dc856e4c698c9e363f0ad7a223a9) --- dbus/dbus-message.c | 3 +++ test/message.c | 9 --------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 45b1adfd..9bc13cf9 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -5185,6 +5185,9 @@ dbus_message_demarshal (const char *str, return msg; fail_corrupt: + if (loader->corruption_reason == DBUS_VALIDITY_UNKNOWN_OOM_ERROR) + goto fail_oom; + dbus_set_error (error, DBUS_ERROR_INVALID_ARGS, "Message is corrupted (%s)", _dbus_validity_to_error_message (loader->corruption_reason)); _dbus_message_loader_unref (loader); diff --git a/test/message.c b/test/message.c index 60ef113d..d2c9db2b 100644 --- a/test/message.c +++ b/test/message.c @@ -291,15 +291,6 @@ test_valid_message_blobs (void *message_name, goto out; } - /* TODO: Validity checking sometimes returns InvalidArgs for OOM */ - if (dbus_error_has_name (&e, DBUS_ERROR_INVALID_ARGS) && - !have_memory && - strstr (e.message, "Out of memory") != NULL) - { - g_test_message ("Out of memory (not a problem)"); - goto out; - } - g_test_message ("Parsing %s reported unexpected error %s: %s", path, e.name, e.message); g_test_fail (); -- cgit v1.2.1 From de9e130207ea7582bd38dc70dfdcdb1fe706bd34 Mon Sep 17 00:00:00 2001 From: Evgeny Vereshchagin Date: Sun, 9 Oct 2022 07:53:02 +0000 Subject: config-parser: no longer get past the last NULL passed to locate_attributes Fixes: bc86794f23fa53 Fixes: ``` ==302818==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7ffd6ac253c0 at pc 0x000000534d0b bp 0x7ffd6ac24e10 sp 0x7ffd6ac24e08 READ of size 8 at 0x7ffd6ac253c0 thread T0 #0 0x534d0a in locate_attributes /home/vagrant/dbus/build/../bus/config-parser.c:658:16 #1 0x52ea3f in start_busconfig_child /home/vagrant/dbus/build/../bus/config-parser.c:1080:12 #2 0x52cca4 in bus_config_parser_start_element /home/vagrant/dbus/build/../bus/config-parser.c:2039:14 #3 0x52b82b in expat_StartElementHandler /home/vagrant/dbus/build/../bus/config-loader-expat.c:107:8 #4 0x7f2179f2d2bd (/lib64/libexpat.so.1+0xd2bd) (BuildId: 0165eed77c910f6ef2227d21afa9c5c5ed5849c2) #5 0x7f2179f2aed3 (/lib64/libexpat.so.1+0xaed3) (BuildId: 0165eed77c910f6ef2227d21afa9c5c5ed5849c2) #6 0x7f2179f2c9ec (/lib64/libexpat.so.1+0xc9ec) (BuildId: 0165eed77c910f6ef2227d21afa9c5c5ed5849c2) #7 0x7f2179f30a8e in XML_ParseBuffer (/lib64/libexpat.so.1+0x10a8e) (BuildId: 0165eed77c910f6ef2227d21afa9c5c5ed5849c2) #8 0x52b040 in bus_config_load /home/vagrant/dbus/build/../bus/config-loader-expat.c:259:9 #9 0x523c8a in bus_context_new /home/vagrant/dbus/build/../bus/bus.c:828:12 #10 0x521056 in main /home/vagrant/dbus/build/../bus/main.c:716:13 #11 0x7f2179a2954f in __libc_start_call_main (/lib64/libc.so.6+0x2954f) (BuildId: 9c5863396a11aab52ae8918ae01a362cefa855fe) #12 0x7f2179a29608 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x29608) (BuildId: 9c5863396a11aab52ae8918ae01a362cefa855fe) #13 0x42a914 in _start (/home/vagrant/dbus/build/bus/dbus-daemon+0x42a914) (BuildId: df5369f85137975aff9bd398ae859706cc3c52ff) Address 0x7ffd6ac253c0 is located in stack of thread T0 at offset 0 in frame #0 0x52cfaf in start_busconfig_child /home/vagrant/dbus/build/../bus/config-parser.c:733 ``` Signed-off-by: Evgeny Vereshchagin (cherry picked from commit ae03bcdb1116a953d4d33661cf878e68cdfbb9fd) --- bus/config-parser.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bus/config-parser.c b/bus/config-parser.c index f9b70477..9f2e3c79 100644 --- a/bus/config-parser.c +++ b/bus/config-parser.c @@ -642,10 +642,11 @@ locate_attributes (BusConfigParser *parser, va_start (args, first_attribute_retloc); name = va_arg (args, const char*); - retloc = va_arg (args, const char**); + retloc = NULL; while (name != NULL) { + retloc = va_arg (args, const char**); _dbus_assert (retloc != NULL); _dbus_assert (n_attrs < MAX_ATTRS); @@ -655,7 +656,6 @@ locate_attributes (BusConfigParser *parser, *retloc = NULL; name = va_arg (args, const char*); - retloc = va_arg (args, const char**); } va_end (args); -- cgit v1.2.1 From dbb1b8f5e66ec1bb77b5fba5d7aed7fee059d235 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 11 Oct 2022 14:36:22 +0100 Subject: Update NEWS for 1.14.x Signed-off-by: Simon McVittie --- NEWS | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index ff2c617f..a6505c24 100644 --- a/NEWS +++ b/NEWS @@ -1,7 +1,19 @@ dbus 1.14.6 (UNRELEASED) ======================== -... +Fixes: + +• When connected to a dbus-broker, stop dbus-monitor from incorrectly + replying to Peer method calls that were sent to the dbus-broker with + a NULL destination (dbus#301, Kai A. Hiller) + +• Fix out-of-bounds varargs read in the dbus-daemon's config-parser. + This is not attacker-triggerable and appears to be harmless in practice, + but is technically undefined behaviour and is detected as such by + AddressSanitizer. (dbus!357, Evgeny Vereshchagin) + +• If dbus_message_demarshal() runs out of memory while validating a message, + report it as NoMemory rather than InvalidArgs (dbus#420, Simon McVittie) dbus 1.14.4 (2022-10-05) ======================== -- cgit v1.2.1