luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-02-23 18:52:09 +01:00
Code Issues Packages Projects Releases Wiki Activity

gdbusmessage: Cache the arg0 value

Browse Source 
Technically we can’t rely on it being kept alive by the `message->body`
pointer, unless we can guarantee that the `GVariant` is always
serialised. That’s not necessarily the case, so keep a separate ref on
the arg0 value at all times.

This avoids a potential use-after-free.

Spotted by Thomas Haller in
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3720#note_1924707.

[This is a prerequisite for having tests pass after fixing the
vulnerability described in glib#3268, because after fixing that
vulnerability, the use-after-free genuinely does happen during
regression testing. -smcv]

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

Helps: #3183, #3268
(cherry picked from commit 10e9a917be7fb92b6b27837ef7a7f1d0be6095d5)
This commit is contained in:
Philip Withnall 2023-11-28 12:58:20 +00:00
parent 336bca622e
commit 952852081d
1 changed files with 22 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
gio/gdbusmessage.c
View File

@ -508,6 +508,7 @@ struct _GDBusMessage
guint32 serial; guint32 serial;
GHashTable *headers; GHashTable *headers;
GVariant *body; GVariant *body;
GVariant *arg0_cache; /* (nullable) (owned) */
#ifdef G_OS_UNIX #ifdef G_OS_UNIX
GUnixFDList *fd_list; GUnixFDList *fd_list;
#endif #endif
@ -530,6 +531,7 @@ g_dbus_message_finalize (GObject *object)
g_hash_table_unref (message->headers); g_hash_table_unref (message->headers);
if (message->body != NULL) if (message->body != NULL)
g_variant_unref (message->body); g_variant_unref (message->body);
g_clear_pointer (&message->arg0_cache, g_variant_unref);
#ifdef G_OS_UNIX #ifdef G_OS_UNIX
if (message->fd_list != NULL) if (message->fd_list != NULL)
g_object_unref (message->fd_list); g_object_unref (message->fd_list);
@ -1168,6 +1170,7 @@ g_dbus_message_set_body (GDBusMessage *message,
if (body == NULL) if (body == NULL)
{ {
message->body = NULL; message->body = NULL;
message->arg0_cache = NULL;
g_dbus_message_set_signature (message, NULL); g_dbus_message_set_signature (message, NULL);
} }
else else
@ -1178,6 +1181,12 @@ g_dbus_message_set_body (GDBusMessage *message,
message->body = g_variant_ref_sink (body); message->body = g_variant_ref_sink (body);
if (g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE) &&
g_variant_n_children (message->body) > 0)
message->arg0_cache = g_variant_get_child_value (message->body, 0);
else
message->arg0_cache = NULL;
type_string = g_variant_get_type_string (body); type_string = g_variant_get_type_string (body);
type_string_len = strlen (type_string); type_string_len = strlen (type_string);
g_assert (type_string_len >= 2); g_assert (type_string_len >= 2);
@ -2362,6 +2371,14 @@ g_dbus_message_new_from_blob (guchar *blob,
2, 2,
&local_error); &local_error);
g_variant_type_free (variant_type); g_variant_type_free (variant_type);
if (message->body != NULL &&
g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE) &&
g_variant_n_children (message->body) > 0)
message->arg0_cache = g_variant_get_child_value (message->body, 0);
else
message->arg0_cache = NULL;
if (message->body == NULL) if (message->body == NULL)
goto fail; goto fail;
} }
@ -3401,22 +3418,13 @@ g_dbus_message_set_signature (GDBusMessage *message,
const gchar * const gchar *
g_dbus_message_get_arg0 (GDBusMessage *message) g_dbus_message_get_arg0 (GDBusMessage *message)
{ {
const gchar *ret;
g_return_val_if_fail (G_IS_DBUS_MESSAGE (message), NULL); g_return_val_if_fail (G_IS_DBUS_MESSAGE (message), NULL);
ret = NULL; if (message->arg0_cache != NULL &&
g_variant_is_of_type (message->arg0_cache, G_VARIANT_TYPE_STRING))
return g_variant_get_string (message->arg0_cache, NULL);
if (message->body != NULL && g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE)) return NULL;
{
GVariant *item;
item = g_variant_get_child_value (message->body, 0);
if (g_variant_is_of_type (item, G_VARIANT_TYPE_STRING))
ret = g_variant_get_string (item, NULL);
g_variant_unref (item);
}
return ret;
} }
/* ---------------------------------------------------------------------------------------------------- */ /* ---------------------------------------------------------------------------------------------------- */
@ -3859,6 +3867,7 @@ g_dbus_message_copy (GDBusMessage *message,
* to just ref (as opposed to deep-copying) the GVariant instances * to just ref (as opposed to deep-copying) the GVariant instances
*/ */
ret->body = message->body != NULL ? g_variant_ref (message->body) : NULL; ret->body = message->body != NULL ? g_variant_ref (message->body) : NULL;
ret->arg0_cache = message->arg0_cache != NULL ? g_variant_ref (message->arg0_cache) : NULL;
g_hash_table_iter_init (&iter, message->headers); g_hash_table_iter_init (&iter, message->headers);
while (g_hash_table_iter_next (&iter, &header_key, (gpointer) &header_value)) while (g_hash_table_iter_next (&iter, &header_key, (gpointer) &header_value))
g_hash_table_insert (ret->headers, header_key, g_variant_ref (header_value)); g_hash_table_insert (ret->headers, header_key, g_variant_ref (header_value));