gdbusmessage: Cache the arg0 value

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.

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

Helps: #3183
This commit is contained in:
Philip Withnall 2023-11-28 12:58:20 +00:00
parent ded4976337
commit 10e9a917be

View File

@ -498,6 +498,7 @@ struct _GDBusMessage
guint32 serial;
GHashTable *headers;
GVariant *body;
GVariant *arg0_cache; /* (nullable) (owned) */
#ifdef G_OS_UNIX
GUnixFDList *fd_list;
#endif
@ -520,6 +521,7 @@ g_dbus_message_finalize (GObject *object)
g_hash_table_unref (message->headers);
if (message->body != NULL)
g_variant_unref (message->body);
g_clear_pointer (&message->arg0_cache, g_variant_unref);
#ifdef G_OS_UNIX
if (message->fd_list != NULL)
g_object_unref (message->fd_list);
@ -1158,6 +1160,7 @@ g_dbus_message_set_body (GDBusMessage *message,
if (body == NULL)
{
message->body = NULL;
message->arg0_cache = NULL;
g_dbus_message_set_signature (message, NULL);
}
else
@ -1168,6 +1171,12 @@ g_dbus_message_set_body (GDBusMessage *message,
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_len = strlen (type_string);
g_assert (type_string_len >= 2);
@ -2352,6 +2361,14 @@ g_dbus_message_new_from_blob (guchar *blob,
2,
&local_error);
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)
goto fail;
}
@ -3391,22 +3408,13 @@ g_dbus_message_set_signature (GDBusMessage *message,
const gchar *
g_dbus_message_get_arg0 (GDBusMessage *message)
{
const gchar *ret;
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))
{
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;
return NULL;
}
/* ---------------------------------------------------------------------------------------------------- */
@ -3849,6 +3857,7 @@ g_dbus_message_copy (GDBusMessage *message,
* to just ref (as opposed to deep-copying) the GVariant instances
*/
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);
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));