lib/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2026-10-24 00:49:00 +03:00
Code Activity

gdbusauth: Limit length of lines read from client

Browse Source 
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 <pwithnall@gnome.org>
Fixes: #3985
This commit is contained in:
Philip Withnall
2026-07-04 18:13:08 +01:00
parent ca46dff809
commit 4235f7b42b
2 changed files with 163 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
gio/gdbusauth.c
View File

@@ -260,6 +260,21 @@ find_mech_by_name (GDBusAuth *auth,
return ret;
}
static size_t
get_longest_mechanism_name_length (GDBusAuth *auth)
{
size_t len = 0;
for (GList *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)
{
@@ -268,6 +283,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 *
@@ -305,6 +334,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)
@@ -314,11 +344,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,
@@ -1071,6 +1112,7 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
case SERVER_STATE_WAITING_FOR_AUTH:
debug_print ("SERVER: WaitingForAuth");
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);
@@ -1292,6 +1334,7 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
case SERVER_STATE_WAITING_FOR_DATA:
debug_print ("SERVER: WaitingForData");
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);
@@ -1334,6 +1377,7 @@ _g_dbus_auth_run_server (GDBusAuth *auth,
case SERVER_STATE_WAITING_FOR_BEGIN:
debug_print ("SERVER: WaitingForBegin");
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);

119
gio/tests/gdbus-auth.c
View File

@@ -263,6 +263,124 @@ 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_summary ("Test that GDBusServer limits the lengths of reads it does during auth from a client");
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);
g_clear_signal_handler (&new_connection_id, server);
g_clear_object (&server);
}
/* ---------------------------------------------------------------------------------------------------- */
int
@@ -282,6 +400,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
*