From 8b72ad09c874ddff122b3e67b3470c5e2eab7690 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 28 Apr 2026 15:47:30 +0100 Subject: [PATCH 1/4] gdbusauthmechanismsha1: Validate cookie context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without validation, the server could send a malicious context which contains path traversal characters, allowing it to exfiltrate a SHA-1 hashed copy of arbitrary data from the client’s file system. To exploit this successfully would require the client to choose to connect peer-to-peer to a malicious D-Bus server and to choose the SHA-1 authentication mechanism in preference to all the other mechanisms. This is vanishingly unlikely. Signed-off-by: Philip Withnall Fixes: #3931 --- gio/gdbusauthmechanismsha1.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/gio/gdbusauthmechanismsha1.c b/gio/gdbusauthmechanismsha1.c index c8aa08977c..7f348d862d 100644 --- a/gio/gdbusauthmechanismsha1.c +++ b/gio/gdbusauthmechanismsha1.c @@ -1198,6 +1198,34 @@ mechanism_client_initiate (GDBusAuthMechanism *mechanism, return initial_response; } +/* Context names must be valid ASCII, nonzero length, and may not contain the + * characters slash ("/"), backslash ("\"), space (" "), newline ("\n"), + * carriage return ("\r"), tab ("\t"), or period ("."). + * + * See https://dbus.freedesktop.org/doc/dbus-specification.html#auth-mechanisms-sha */ +static gboolean +validate_cookie_context (const char *cookie_context) +{ + size_t i = 0; + + g_return_val_if_fail (cookie_context != NULL, FALSE); + + for (i = 0; cookie_context[i] != '\0'; i++) + { + if ((uint8_t) cookie_context[i] >= 128 || + cookie_context[i] == '/' || + cookie_context[i] == '\\' || + cookie_context[i] == ' ' || + cookie_context[i] == '\n' || + cookie_context[i] == '\r' || + cookie_context[i] == '\t' || + cookie_context[i] == '.') + return FALSE; + } + + return (i > 0); +} + static void mechanism_client_data_receive (GDBusAuthMechanism *mechanism, const gchar *data, @@ -1232,6 +1260,14 @@ mechanism_client_data_receive (GDBusAuthMechanism *mechanism, } cookie_context = tokens[0]; + if (!validate_cookie_context (tokens[0])) + { + g_free (m->priv->reject_reason); + m->priv->reject_reason = g_strdup_printf ("Malformed cookie_context '%s'", tokens[0]); + m->priv->state = G_DBUS_AUTH_MECHANISM_STATE_REJECTED; + goto out; + } + cookie_id = g_ascii_strtoll (tokens[1], &endp, 10); if (*endp != '\0') { -- GitLab From 948c5984b8f9422997b02c1cbed998f5b52b3109 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 28 Apr 2026 15:49:54 +0100 Subject: [PATCH 2/4] gdbusauthmechanismsha1: Improve validation of cookie ID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The D-Bus specification says the cookie ID has to be non-negative, but we weren’t checking that (or checking that it was non-empty). Signed-off-by: Philip Withnall --- gio/gdbusauthmechanismsha1.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gio/gdbusauthmechanismsha1.c b/gio/gdbusauthmechanismsha1.c index 7f348d862d..3c17f81a19 100644 --- a/gio/gdbusauthmechanismsha1.c +++ b/gio/gdbusauthmechanismsha1.c @@ -1234,7 +1234,7 @@ mechanism_client_data_receive (GDBusAuthMechanism *mechanism, GDBusAuthMechanismSha1 *m = G_DBUS_AUTH_MECHANISM_SHA1 (mechanism); gchar **tokens; const gchar *cookie_context; - guint cookie_id; + int64_t cookie_id; const gchar *server_challenge; gchar *client_challenge; gchar *endp; @@ -1269,7 +1269,7 @@ mechanism_client_data_receive (GDBusAuthMechanism *mechanism, } cookie_id = g_ascii_strtoll (tokens[1], &endp, 10); - if (*endp != '\0') + if (*endp != '\0' || endp == tokens[1] || cookie_id < 0 || cookie_id > UINT32_MAX) { g_free (m->priv->reject_reason); m->priv->reject_reason = g_strdup_printf ("Malformed cookie_id '%s'", tokens[1]); @@ -1279,7 +1279,7 @@ mechanism_client_data_receive (GDBusAuthMechanism *mechanism, server_challenge = tokens[2]; error = NULL; - cookie = keyring_lookup_entry (cookie_context, cookie_id, &error); + cookie = keyring_lookup_entry (cookie_context, (unsigned int) cookie_id, &error); if (cookie == NULL) { g_free (m->priv->reject_reason); -- GitLab From d9d2010689774e6f3e00bf8b8c0b4741f27d1bef Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 28 Apr 2026 15:51:00 +0100 Subject: [PATCH 3/4] gdbusauthmechanism: Expose client reject reason as a new vfunc We can do this because `gdbusauthmechanism.h` is a private header. Hook it up to the existing `reject_reason` code in each `GDBusAuthMechanism` implementation, as all three implementations currently intermingle reject reasons from the server and client code, so there would currently be no benefit to having a separate server and client implementation of `*_get_reject_reason()`. This new private API will be used in a new unit test in the following commit. Signed-off-by: Philip Withnall --- gio/gdbusauthmechanism.c | 7 +++++++ gio/gdbusauthmechanism.h | 2 ++ gio/gdbusauthmechanismanon.c | 8 ++++---- gio/gdbusauthmechanismexternal.c | 8 ++++---- gio/gdbusauthmechanismsha1.c | 8 ++++---- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/gio/gdbusauthmechanism.c b/gio/gdbusauthmechanism.c index be1b1dca06..0b52f17e41 100644 --- a/gio/gdbusauthmechanism.c +++ b/gio/gdbusauthmechanism.c @@ -324,6 +324,13 @@ _g_dbus_auth_mechanism_client_data_send (GDBusAuthMechanism *mechanism, return G_DBUS_AUTH_MECHANISM_GET_CLASS (mechanism)->client_data_send (mechanism, out_data_len); } +gchar * +_g_dbus_auth_mechanism_client_get_reject_reason (GDBusAuthMechanism *mechanism) +{ + g_return_val_if_fail (G_IS_DBUS_AUTH_MECHANISM (mechanism), NULL); + return G_DBUS_AUTH_MECHANISM_GET_CLASS (mechanism)->client_get_reject_reason (mechanism); +} + void _g_dbus_auth_mechanism_client_shutdown (GDBusAuthMechanism *mechanism) { diff --git a/gio/gdbusauthmechanism.h b/gio/gdbusauthmechanism.h index f0edd19a3e..e906a47ac5 100644 --- a/gio/gdbusauthmechanism.h +++ b/gio/gdbusauthmechanism.h @@ -100,6 +100,7 @@ struct _GDBusAuthMechanismClass gsize data_len); gchar *(*client_data_send) (GDBusAuthMechanism *mechanism, gsize *out_data_len); + gchar *(*client_get_reject_reason) (GDBusAuthMechanism *mechanism); void (*client_shutdown) (GDBusAuthMechanism *mechanism); }; @@ -148,6 +149,7 @@ void _g_dbus_auth_mechanism_client_data_receive (GDBus gsize data_len); gchar *_g_dbus_auth_mechanism_client_data_send (GDBusAuthMechanism *mechanism, gsize *out_data_len); +gchar *_g_dbus_auth_mechanism_client_get_reject_reason (GDBusAuthMechanism *mechanism); void _g_dbus_auth_mechanism_client_shutdown (GDBusAuthMechanism *mechanism); diff --git a/gio/gdbusauthmechanismanon.c b/gio/gdbusauthmechanismanon.c index 5f59d4a61d..3d80ec15fe 100644 --- a/gio/gdbusauthmechanismanon.c +++ b/gio/gdbusauthmechanismanon.c @@ -56,7 +56,7 @@ static void mechanism_server_data_receive (GDBusAuthMe gsize data_len); static gchar *mechanism_server_data_send (GDBusAuthMechanism *mechanism, gsize *out_data_len); -static gchar *mechanism_server_get_reject_reason (GDBusAuthMechanism *mechanism); +static gchar *mechanism_server_or_client_get_reject_reason (GDBusAuthMechanism *mechanism); static void mechanism_server_shutdown (GDBusAuthMechanism *mechanism); static GDBusAuthMechanismState mechanism_client_get_state (GDBusAuthMechanism *mechanism); static gchar *mechanism_client_initiate (GDBusAuthMechanism *mechanism, @@ -103,12 +103,13 @@ _g_dbus_auth_mechanism_anon_class_init (GDBusAuthMechanismAnonClass *klass) mechanism_class->server_initiate = mechanism_server_initiate; mechanism_class->server_data_receive = mechanism_server_data_receive; mechanism_class->server_data_send = mechanism_server_data_send; - mechanism_class->server_get_reject_reason = mechanism_server_get_reject_reason; + mechanism_class->server_get_reject_reason = mechanism_server_or_client_get_reject_reason; mechanism_class->server_shutdown = mechanism_server_shutdown; mechanism_class->client_get_state = mechanism_client_get_state; mechanism_class->client_initiate = mechanism_client_initiate; mechanism_class->client_data_receive = mechanism_client_data_receive; mechanism_class->client_data_send = mechanism_client_data_send; + mechanism_class->client_get_reject_reason = mechanism_server_or_client_get_reject_reason; mechanism_class->client_shutdown = mechanism_client_shutdown; } @@ -222,12 +223,11 @@ mechanism_server_data_send (GDBusAuthMechanism *mechanism, } static gchar * -mechanism_server_get_reject_reason (GDBusAuthMechanism *mechanism) +mechanism_server_or_client_get_reject_reason (GDBusAuthMechanism *mechanism) { GDBusAuthMechanismAnon *m = G_DBUS_AUTH_MECHANISM_ANON (mechanism); g_return_val_if_fail (G_IS_DBUS_AUTH_MECHANISM_ANON (mechanism), NULL); - g_return_val_if_fail (m->priv->is_server && !m->priv->is_client, NULL); g_return_val_if_fail (m->priv->state == G_DBUS_AUTH_MECHANISM_STATE_REJECTED, NULL); /* can never end up here because we are never in the REJECTED state */ diff --git a/gio/gdbusauthmechanismexternal.c b/gio/gdbusauthmechanismexternal.c index f7cb1b1b99..fce32fa83f 100644 --- a/gio/gdbusauthmechanismexternal.c +++ b/gio/gdbusauthmechanismexternal.c @@ -64,7 +64,7 @@ static void mechanism_server_data_receive (GDBusAuthMe gsize data_len); static gchar *mechanism_server_data_send (GDBusAuthMechanism *mechanism, gsize *out_data_len); -static gchar *mechanism_server_get_reject_reason (GDBusAuthMechanism *mechanism); +static gchar *mechanism_server_or_client_get_reject_reason (GDBusAuthMechanism *mechanism); static void mechanism_server_shutdown (GDBusAuthMechanism *mechanism); static GDBusAuthMechanismState mechanism_client_get_state (GDBusAuthMechanism *mechanism); static gchar *mechanism_client_initiate (GDBusAuthMechanism *mechanism, @@ -111,12 +111,13 @@ _g_dbus_auth_mechanism_external_class_init (GDBusAuthMechanismExternalClass *kla mechanism_class->server_initiate = mechanism_server_initiate; mechanism_class->server_data_receive = mechanism_server_data_receive; mechanism_class->server_data_send = mechanism_server_data_send; - mechanism_class->server_get_reject_reason = mechanism_server_get_reject_reason; + mechanism_class->server_get_reject_reason = mechanism_server_or_client_get_reject_reason; mechanism_class->server_shutdown = mechanism_server_shutdown; mechanism_class->client_get_state = mechanism_client_get_state; mechanism_class->client_initiate = mechanism_client_initiate; mechanism_class->client_data_receive = mechanism_client_data_receive; mechanism_class->client_data_send = mechanism_client_data_send; + mechanism_class->client_get_reject_reason = mechanism_server_or_client_get_reject_reason; mechanism_class->client_shutdown = mechanism_client_shutdown; } @@ -321,12 +322,11 @@ mechanism_server_data_send (GDBusAuthMechanism *mechanism, } static gchar * -mechanism_server_get_reject_reason (GDBusAuthMechanism *mechanism) +mechanism_server_or_client_get_reject_reason (GDBusAuthMechanism *mechanism) { GDBusAuthMechanismExternal *m = G_DBUS_AUTH_MECHANISM_EXTERNAL (mechanism); g_return_val_if_fail (G_IS_DBUS_AUTH_MECHANISM_EXTERNAL (mechanism), NULL); - g_return_val_if_fail (m->priv->is_server && !m->priv->is_client, NULL); g_return_val_if_fail (m->priv->state == G_DBUS_AUTH_MECHANISM_STATE_REJECTED, NULL); /* can never end up here because we are never in the REJECTED state */ diff --git a/gio/gdbusauthmechanismsha1.c b/gio/gdbusauthmechanismsha1.c index 3c17f81a19..9993bf3245 100644 --- a/gio/gdbusauthmechanismsha1.c +++ b/gio/gdbusauthmechanismsha1.c @@ -119,7 +119,7 @@ static void mechanism_server_data_receive (GDBusAuthMe gsize data_len); static gchar *mechanism_server_data_send (GDBusAuthMechanism *mechanism, gsize *out_data_len); -static gchar *mechanism_server_get_reject_reason (GDBusAuthMechanism *mechanism); +static gchar *mechanism_server_or_client_get_reject_reason (GDBusAuthMechanism *mechanism); static void mechanism_server_shutdown (GDBusAuthMechanism *mechanism); static GDBusAuthMechanismState mechanism_client_get_state (GDBusAuthMechanism *mechanism); static gchar *mechanism_client_initiate (GDBusAuthMechanism *mechanism, @@ -172,12 +172,13 @@ _g_dbus_auth_mechanism_sha1_class_init (GDBusAuthMechanismSha1Class *klass) mechanism_class->server_initiate = mechanism_server_initiate; mechanism_class->server_data_receive = mechanism_server_data_receive; mechanism_class->server_data_send = mechanism_server_data_send; - mechanism_class->server_get_reject_reason = mechanism_server_get_reject_reason; + mechanism_class->server_get_reject_reason = mechanism_server_or_client_get_reject_reason; mechanism_class->server_shutdown = mechanism_server_shutdown; mechanism_class->client_get_state = mechanism_client_get_state; mechanism_class->client_initiate = mechanism_client_initiate; mechanism_class->client_data_receive = mechanism_client_data_receive; mechanism_class->client_data_send = mechanism_client_data_send; + mechanism_class->client_get_reject_reason = mechanism_server_or_client_get_reject_reason; mechanism_class->client_shutdown = mechanism_client_shutdown; } @@ -1128,12 +1129,11 @@ mechanism_server_data_send (GDBusAuthMechanism *mechanism, } static gchar * -mechanism_server_get_reject_reason (GDBusAuthMechanism *mechanism) +mechanism_server_or_client_get_reject_reason (GDBusAuthMechanism *mechanism) { GDBusAuthMechanismSha1 *m = G_DBUS_AUTH_MECHANISM_SHA1 (mechanism); g_return_val_if_fail (G_IS_DBUS_AUTH_MECHANISM_SHA1 (mechanism), NULL); - g_return_val_if_fail (m->priv->is_server && !m->priv->is_client, NULL); g_return_val_if_fail (m->priv->state == G_DBUS_AUTH_MECHANISM_STATE_REJECTED, NULL); return g_strdup (m->priv->reject_reason); -- GitLab From ff97c6f5b6b51919b1cb2a8071ca2b03bc6cf663 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 28 Apr 2026 15:52:53 +0100 Subject: [PATCH 4/4] tests: Add a unit test for GDBusAuthMechanismSha1 cookie context parsing This checks for regressions in the fixes from the previous few commits. Signed-off-by: Philip Withnall Helps: #3931 --- gio/tests/gdbus-auth-mechanism-sha1.c | 177 ++++++++++++++++++++++++++ gio/tests/meson.build | 1 + 2 files changed, 178 insertions(+) create mode 100644 gio/tests/gdbus-auth-mechanism-sha1.c diff --git a/gio/tests/gdbus-auth-mechanism-sha1.c b/gio/tests/gdbus-auth-mechanism-sha1.c new file mode 100644 index 0000000000..abcdb4e3e7 --- /dev/null +++ b/gio/tests/gdbus-auth-mechanism-sha1.c @@ -0,0 +1,177 @@ +/* GLib testing framework examples and tests + * + * Copyright (C) 2026 Philip Withnall + * + * SPDX-License-Identifier: LGPL-2.1-or-later + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General + * Public License along with this library; if not, see . + * + * Author: Philip Withnall + */ + +#include +#include + +#include +#include + +#include "gdbus-tests.h" + +#ifdef G_OS_UNIX +#include +#include +#include +#include +#endif + +#define GIO_COMPILATION 1 +#include "gdbusauthmechanism.h" +#include "gdbusauthmechanismsha1.h" + +/* Vfunc wrappers copied from gdbusauthmechanism.c as they are not public. */ +static gboolean +dbus_auth_mechanism_is_supported (GDBusAuthMechanism *mechanism) +{ + return G_DBUS_AUTH_MECHANISM_GET_CLASS (mechanism)->is_supported (mechanism); +} + +static GDBusAuthMechanismState +dbus_auth_mechanism_client_get_state (GDBusAuthMechanism *mechanism) +{ + return G_DBUS_AUTH_MECHANISM_GET_CLASS (mechanism)->client_get_state (mechanism); +} + +static gchar * +dbus_auth_mechanism_client_initiate (GDBusAuthMechanism *mechanism, + GDBusConnectionFlags conn_flags, + size_t *out_initial_response_len) +{ + return G_DBUS_AUTH_MECHANISM_GET_CLASS (mechanism)->client_initiate (mechanism, + conn_flags, + out_initial_response_len); +} + +static void +dbus_auth_mechanism_client_data_receive (GDBusAuthMechanism *mechanism, + const char *data, + size_t data_len) +{ + G_DBUS_AUTH_MECHANISM_GET_CLASS (mechanism)->client_data_receive (mechanism, data, data_len); +} + +static char * +dbus_auth_mechanism_client_get_reject_reason (GDBusAuthMechanism *mechanism) +{ + return G_DBUS_AUTH_MECHANISM_GET_CLASS (mechanism)->client_get_reject_reason (mechanism); +} + +static void +dbus_auth_mechanism_client_shutdown (GDBusAuthMechanism *mechanism) +{ + G_DBUS_AUTH_MECHANISM_GET_CLASS (mechanism)->client_shutdown (mechanism); +} + +static void +test_server_challenge_validation (void) +{ + const struct + { + const char *server_challenge; + const char *expected_reject_reason_prefix; + } + vectors[] = { + { "valid_context 123 456", "Problems looking up entry in keyring" }, + { "invalid/context 123 456", "Malformed cookie_context" }, + { "invalid.context 123 456", "Malformed cookie_context" }, + { " 123 456", "Malformed cookie_context" }, + { "😀 123 456", "Malformed cookie_context" }, + { "invalid\ncontext 123 456", "Malformed cookie_context" }, + { "invalid\rcontext 123 456", "Malformed cookie_context" }, + { "invalid\tcontext 123 456", "Malformed cookie_context" }, + { "invalid\\context 123 456", "Malformed cookie_context" }, + { "valid_context 456", "Malformed cookie_id" }, + { "valid_context 123notanumber 456", "Malformed cookie_id" }, + { "valid_context -1 456", "Malformed cookie_id" }, + { "valid_context 4294967296 456", "Malformed cookie_id" }, + { "valid_context 123 ", "Malformed data" }, + { "valid_context ", "Malformed data" }, + }; + GType mechanism_type; + GDBusConnection *connection = NULL; + + g_test_summary ("Test that GDBusAuthMechanismSha1 rejects various malformed server data lines"); + + /* Briefly connect to the actual bus to ensure the GDBusAuth mechanisms are + * all registered. */ + session_bus_up (); + + connection = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL); + g_assert_nonnull (connection); + g_clear_object (&connection); + + session_bus_down (); + + /* Check that we now have the type ID for GDBusAuthMechanismSha1 */ + mechanism_type = g_type_from_name ("GDBusAuthMechanismSha1"); + g_assert_cmpint (mechanism_type, !=, 0); + + for (size_t i = 0; i < G_N_ELEMENTS (vectors); i++) + { + GDBusAuthMechanism *mechanism = NULL; + char *data = NULL; + size_t data_len = 0; + char *reject_reason = NULL; + + mechanism = g_object_new (mechanism_type, NULL); + + if (!dbus_auth_mechanism_is_supported (mechanism)) + { + g_test_skip ("Mechanism not supported"); + g_clear_object (&mechanism); + return; + } + + data = dbus_auth_mechanism_client_initiate (mechanism, + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT, + &data_len); + g_free (data); + + dbus_auth_mechanism_client_data_receive (mechanism, vectors[i].server_challenge, strlen (vectors[i].server_challenge)); + + g_assert_cmpint (dbus_auth_mechanism_client_get_state (mechanism), ==, G_DBUS_AUTH_MECHANISM_STATE_REJECTED); + + reject_reason = dbus_auth_mechanism_client_get_reject_reason (mechanism); + g_assert_true (g_str_has_prefix (reject_reason, vectors[i].expected_reject_reason_prefix)); + g_free (reject_reason); + + dbus_auth_mechanism_client_shutdown (mechanism); + + g_clear_object (&mechanism); + } +} + +int +main (int argc, + char *argv[]) +{ + setlocale (LC_ALL, "C"); + + g_test_init (&argc, &argv, G_TEST_OPTION_ISOLATE_DIRS, NULL); + + g_test_dbus_unset (); + + g_test_add_func ("/gdbus/auth-mechanism-sha1/server-challenge-validation", test_server_challenge_validation); + + return g_test_run (); +} diff --git a/gio/tests/meson.build b/gio/tests/meson.build index 9bd0d30d07..b1e9df265c 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -474,6 +474,7 @@ if host_machine.system() != 'windows' }, 'fdo-notification-backend': {}, 'gdbus-auth' : {'extra_sources' : extra_sources}, + 'gdbus-auth-mechanism-sha1': {'extra_sources' : extra_sources}, 'gdbus-bz627724' : {'extra_sources' : extra_sources}, 'gdbus-close-pending' : {'extra_sources' : extra_sources}, 'gdbus-connection' : { -- GitLab