From 952852081db408259d5a546927f08e40d94701eb Mon Sep 17 00:00:00 2001
From: Philip Withnall <pwithnall@gnome.org>
Date: Tue, 28 Nov 2023 12:58:20 +0000
Subject: [PATCH 01/17] gdbusmessage: Cache the arg0 value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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)
---
 gio/gdbusmessage.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c
index adddb31544..6c4a5bfbee 100644
--- a/gio/gdbusmessage.c
+++ b/gio/gdbusmessage.c
@@ -508,6 +508,7 @@ struct _GDBusMessage
   guint32 serial;
   GHashTable *headers;
   GVariant *body;
+  GVariant *arg0_cache;  /* (nullable) (owned) */
 #ifdef G_OS_UNIX
   GUnixFDList *fd_list;
 #endif
@@ -530,6 +531,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);
@@ -1168,6 +1170,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
@@ -1178,6 +1181,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);
@@ -2362,6 +2371,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;
         }
@@ -3401,22 +3418,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;
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
@@ -3859,6 +3867,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));
-- 
GitLab