From fd2ce100affce9e822c81f51d66b2b4b93a76889 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Sat, 4 Jul 2026 18:13:08 +0100 Subject: [PATCH 1/2] gdbusauth: Limit length of lines read from client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client isn’t trusted at this point, and there was previously nothing limiting how long a line `GDBusAuth` would read. So an untrusted client could exhaust the server’s memory by sending anything except `\r\n`. Fix that by applying a reasonably length limit when reading a line, and add a unit test. Spotted by Gitee Codepecker Lab. Signed-off-by: Philip Withnall Fixes: #3985 --- gio/gdbusauth.c | 55 ++++++++++++++++++- gio/tests/gdbus-auth.c | 122 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+), 2 deletions(-) diff --git a/gio/gdbusauth.c b/gio/gdbusauth.c index e9a953a2c..a94a3e359 100644 --- a/gio/gdbusauth.c +++ b/gio/gdbusauth.c @@ -262,6 +262,22 @@ find_mech_by_name (GDBusAuth *auth, return ret; } +static size_t +get_longest_mechanism_name_length (GDBusAuth *auth) +{ + size_t len = 0; + GList *l; + + for (l = auth->priv->available_mechanisms; l != NULL; l = l->next) + { + Mechanism *m = l->data; + + len = MAX (len, strlen (m->name)); + } + + return len; +} + GDBusAuth * _g_dbus_auth_new (GIOStream *stream) { @@ -270,6 +286,20 @@ _g_dbus_auth_new (GIOStream *stream) NULL); } +/* Arbitrarily chosen limit on the length of a DATA command payload, to prevent + * unbounded reads from malicious clients. + * + * - The ANONYMOUS mechanism doesn’t use DATA. + * - The EXTERNAL mechanism just uses it to transfer a decimal-encoded UID. + * - The DBUS_COOKIE_SHA1 mechanism transfers a challenge and a SHA1 hash. The + * hash is bounded in length, but the challenge is not, so could potentially + * hit this limit. It doesn’t seem unreasonable to bound the challenge to + * ~4KB though. GDBus itself generates a 16 byte challenge. + * + * See https://dbus.freedesktop.org/doc/dbus-specification.html#auth-command-data + */ +#define MAX_DATA_PAYLOAD_LENGTH_BYTES 4096 + /* ---------------------------------------------------------------------------------------------------- */ /* like g_data_input_stream_read_line() but sets error if there's no content to read */ static gchar * @@ -307,6 +337,7 @@ _my_g_data_input_stream_read_line (GDataInputStream *dis, */ static gchar * _my_g_input_stream_read_line_safe (GInputStream *i, + size_t max_line_length, gsize *out_line_length, GCancellable *cancellable, GError **error) @@ -316,11 +347,22 @@ _my_g_input_stream_read_line_safe (GInputStream *i, gssize num_read; gboolean last_was_cr; + g_assert (max_line_length <= SIZE_MAX - 2); + str = g_string_new (NULL); last_was_cr = FALSE; while (TRUE) { + if (str->len >= max_line_length + 2 /* allow for \r\n */) + { + g_set_error_literal (error, + G_IO_ERROR, + G_IO_ERROR_FAILED, + _("Malformed D-Bus authentication line")); + goto fail; + } + num_read = g_input_stream_read (i, &c, 1, @@ -1073,7 +1115,11 @@ _g_dbus_auth_run_server (GDBusAuth *auth, { case SERVER_STATE_WAITING_FOR_AUTH: debug_print ("SERVER: WaitingForAuth"); - line = _my_g_data_input_stream_read_line (dis, &line_length, cancellable, error); + line = _my_g_input_stream_read_line_safe (g_io_stream_get_input_stream (auth->priv->stream), + strlen ("AUTH ") + get_longest_mechanism_name_length (auth) + strlen (" ") + MAX_DATA_PAYLOAD_LENGTH_BYTES, + &line_length, + cancellable, + error); debug_print ("SERVER: WaitingForAuth, read '%s'", line); if (line == NULL) goto out; @@ -1272,7 +1318,11 @@ _g_dbus_auth_run_server (GDBusAuth *auth, case SERVER_STATE_WAITING_FOR_DATA: debug_print ("SERVER: WaitingForData"); - line = _my_g_data_input_stream_read_line (dis, &line_length, cancellable, error); + line = _my_g_input_stream_read_line_safe (g_io_stream_get_input_stream (auth->priv->stream), + strlen ("DATA ") + MAX_DATA_PAYLOAD_LENGTH_BYTES, + &line_length, + cancellable, + error); debug_print ("SERVER: WaitingForData, read '%s'", line); if (line == NULL) goto out; @@ -1319,6 +1369,7 @@ _g_dbus_auth_run_server (GDBusAuth *auth, * appears after "BEGIN\r\n"....) */ line = _my_g_input_stream_read_line_safe (g_io_stream_get_input_stream (auth->priv->stream), + MAX (strlen ("BEGIN"), strlen ("NEGOTIATE_UNIX_FD")), &line_length, cancellable, error); diff --git a/gio/tests/gdbus-auth.c b/gio/tests/gdbus-auth.c index f44e932e2..0fb780c1e 100644 --- a/gio/tests/gdbus-auth.c +++ b/gio/tests/gdbus-auth.c @@ -278,6 +278,127 @@ temp_dbus_keyrings_teardown (void) g_unsetenv ("G_DBUS_COOKIE_SHA1_KEYRING_DIR_IGNORE_PERMISSION"); } +static void +async_result_cb (GObject *obj, + GAsyncResult *result, + void *user_data) +{ + GAsyncResult **result_out = user_data; + + g_assert (result_out != NULL); + g_assert (*result_out == NULL); + + *result_out = g_object_ref (result); + g_main_context_wakeup (g_main_context_get_thread_default ()); +} + +static gboolean +server_new_connection_unexpected_cb (GDBusServer *server, + GDBusConnection *connection, + void *user_data) +{ + g_assert_not_reached (); + return FALSE; +} + +static void +test_auth_server_read_limit (void) +{ + GDBusServer *server = NULL; + unsigned long new_connection_id = 0; + const char *server_address; + GIOStream *client_stream = NULL; + GOutputStream *client_output_stream; + GInputStream *client_input_stream; + GAsyncResult *result = NULL; + char *write_buffer = NULL; + char read_buffer[100]; + ssize_t read_len; + size_t bytes_written; + GError *local_error = NULL; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/3985"); + + server = server_new_for_mechanism (NULL); + + new_connection_id = g_signal_connect (server, + "new-connection", + G_CALLBACK (server_new_connection_unexpected_cb), + NULL); + server_address = g_dbus_server_get_client_address (server); + g_dbus_server_start (server); + + /* Start connecting as a client */ + g_dbus_address_get_stream (server_address, NULL, async_result_cb, &result); + + while (result == NULL) + g_main_context_iteration (NULL, TRUE); + + client_stream = g_dbus_address_get_stream_finish (result, NULL, &local_error); + g_assert_no_error (local_error); + g_clear_object (&result); + + /* Send an over-long AUTH line, maliciously */ + client_output_stream = g_io_stream_get_output_stream (client_stream); + client_input_stream = g_io_stream_get_input_stream (client_stream); + + write_buffer = g_strdup_printf ("AUTH DBUS_COOKIE_SHA1 context%0*d 123 456\r\n", 5000, 0); + + g_output_stream_write_all_async (client_output_stream, + write_buffer, + strlen (write_buffer), + G_PRIORITY_DEFAULT, + NULL, + async_result_cb, + &result); + + while (result == NULL) + g_main_context_iteration (NULL, TRUE); + + g_output_stream_write_all_finish (client_output_stream, result, &bytes_written, &local_error); + g_assert_no_error (local_error); + g_assert_cmpuint (bytes_written, ==, strlen (write_buffer)); + g_clear_object (&result); + + g_clear_pointer (&write_buffer, g_free); + + /* Authentication should have been rejected, so reading or writing the stream + * should now fail. */ + read_len = g_input_stream_read (client_input_stream, + read_buffer, + sizeof (read_buffer), + NULL, + &local_error); + g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_CONNECTION_CLOSED); + g_assert_cmpint (read_len, <, 0); + g_clear_error (&local_error); + + write_buffer = g_strdup_printf ("AUTH\r\n"); + + g_output_stream_write_all (client_output_stream, + write_buffer, + strlen (write_buffer), + &bytes_written, + NULL, + &local_error); + g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_CONNECTION_CLOSED); + g_assert_cmpuint (bytes_written, ==, 0); + g_clear_error (&local_error); + + g_clear_pointer (&write_buffer, g_free); + + /* Cleanup */ + g_clear_object (&client_stream); + g_dbus_server_stop (server); + + if (new_connection_id != 0) + { + g_signal_handler_disconnect (server, new_connection_id); + new_connection_id = 0; + } + g_clear_object (&server); +} + /* ---------------------------------------------------------------------------------------------------- */ int @@ -297,6 +418,7 @@ main (int argc, g_test_add_func ("/gdbus/auth/server/ANONYMOUS", auth_server_anonymous); g_test_add_func ("/gdbus/auth/server/EXTERNAL", auth_server_external); g_test_add_func ("/gdbus/auth/server/DBUS_COOKIE_SHA1", auth_server_dbus_cookie_sha1); + g_test_add_func ("/gdbus/auth/server/read-limit", test_auth_server_read_limit); /* TODO: we currently don't have tests for * From c50053362f01e6927f9347532a93e32074495bde Mon Sep 17 00:00:00 2001 From: RHEL Packaging Agent Date: Thu, 30 Jul 2026 12:09:40 +0000 Subject: [PATCH 2/2] Fix g_test_bug usage for GLib 2.56.x compatibility In GLib 2.56.x, g_test_bug() requires g_test_bug_base() to be called first to set the base URI. The upstream test used a full URL in g_test_bug() which is only supported in newer GLib versions. Fix by adding g_test_bug_base() in main() and using just the issue number in g_test_bug(). --- gio/tests/gdbus-auth.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gio/tests/gdbus-auth.c b/gio/tests/gdbus-auth.c index 0fb780c1e..53898dbeb 100644 --- a/gio/tests/gdbus-auth.c +++ b/gio/tests/gdbus-auth.c @@ -317,7 +317,7 @@ test_auth_server_read_limit (void) size_t bytes_written; GError *local_error = NULL; - g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/3985"); + g_test_bug ("3985"); server = server_new_for_mechanism (NULL); @@ -412,6 +412,7 @@ main (int argc, temp_dbus_keyrings_setup (); g_test_init (&argc, &argv, NULL); + g_test_bug_base ("https://gitlab.gnome.org/GNOME/glib/-/issues/"); g_test_add_func ("/gdbus/auth/client/EXTERNAL", auth_client_external); g_test_add_func ("/gdbus/auth/client/DBUS_COOKIE_SHA1", auth_client_dbus_cookie_sha1);