From 4d8106ba534c7f3c2037cca4c654fd13ce0b93d9 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 8 Mar 2024 20:10:29 +0000
Subject: [PATCH 06/18] tests: Add test coverage for signals that match the
 message bus's name

This is a special case of unique names, even though it's syntactically
a well-known name.

Signed-off-by: Simon McVittie <smcv@collabora.com>

CVE: CVE-2024-34397
Upstream-Status: Backport [https://gitlab.gnome.org/GNOME/glib/-/commit/4d8106ba534c7f3c2037cca4c654fd13ce0b93d9]
Signed-off-by: Peter Marko <peter.marko@siemens.com>
---
 gio/tests/gdbus-subscribe.c | 161 ++++++++++++++++++++++++++++++++++--
 1 file changed, 154 insertions(+), 7 deletions(-)

diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c
index 350ec9f52..af100de7d 100644
--- a/gio/tests/gdbus-subscribe.c
+++ b/gio/tests/gdbus-subscribe.c
@@ -13,6 +13,7 @@
 #define DBUS_SERVICE_DBUS "org.freedesktop.DBus"
 #define DBUS_PATH_DBUS "/org/freedesktop/DBus"
 #define DBUS_INTERFACE_DBUS DBUS_SERVICE_DBUS
+#define NAME_OWNER_CHANGED "NameOwnerChanged"
 
 /* A signal that each connection emits to indicate that it has finished
  * emitting other signals */
@@ -101,6 +102,7 @@ typedef struct
   const char *iface;
   const char *member;
   const char *arg0;
+  const char *args;
   guint received_by_conn;
   guint received_by_proxy;
 } TestEmitSignal;
@@ -120,6 +122,8 @@ typedef struct
 {
   const char *name;
   TestConn owner;
+  guint received_by_conn;
+  guint received_by_proxy;
 } TestOwnName;
 
 typedef enum
@@ -461,6 +465,63 @@ static const TestPlan plan_limit_by_well_known_name =
   },
 };
 
+static const TestPlan plan_limit_to_message_bus =
+{
+  .description = "A subscription to the message bus only accepts messages "
+                 "from the message bus",
+  .steps = {
+    {
+      /* Subscriber wants to receive signals from the message bus itself */
+      .action = TEST_ACTION_SUBSCRIBE,
+      .u.subscribe = {
+        .string_sender = DBUS_SERVICE_DBUS,
+        .path = DBUS_PATH_DBUS,
+        .iface = DBUS_INTERFACE_DBUS,
+      },
+    },
+    {
+      /* Attacker wants to trick subscriber into thinking that the message
+       * bus sent a signal */
+      .action = TEST_ACTION_EMIT_SIGNAL,
+      .u.signal = {
+        .sender = TEST_CONN_ATTACKER,
+        .path = DBUS_PATH_DBUS,
+        .iface = DBUS_INTERFACE_DBUS,
+        .member = NAME_OWNER_CHANGED,
+        .arg0 = "would I lie to you?",
+        .received_by_conn = 0,
+        .received_by_proxy = 0
+      },
+    },
+    {
+      /* Attacker tries harder, by sending a signal unicast directly to
+       * the subscriber, and using more realistic arguments */
+      .action = TEST_ACTION_EMIT_SIGNAL,
+      .u.signal = {
+        .unicast_to = TEST_CONN_SUBSCRIBER,
+        .sender = TEST_CONN_ATTACKER,
+        .path = DBUS_PATH_DBUS,
+        .iface = DBUS_INTERFACE_DBUS,
+        .member = NAME_OWNER_CHANGED,
+        .args = "('com.example.Name', '', ':1.12')",
+        .received_by_conn = 0,
+        .received_by_proxy = 0
+      },
+    },
+    {
+      /* When the message bus sends a signal (in this case triggered by
+       * owning a name), it should still get through */
+      .action = TEST_ACTION_OWN_NAME,
+      .u.own_name = {
+        .name = OWNED_LATER_NAME,
+        .owner = TEST_CONN_SERVICE,
+        .received_by_conn = 1,
+        .received_by_proxy = 1
+      },
+    },
+  },
+};
+
 typedef struct
 {
   const TestPlan *plan;
@@ -591,7 +652,18 @@ fixture_received_signal (Fixture    *f,
         }
     }
 
-  g_assert_cmpint (received->sender, !=, TEST_CONN_NONE);
+  if (g_str_equal (sender_name, DBUS_SERVICE_DBUS))
+    {
+      g_test_message ("Signal received from message bus %s",
+                      sender_name);
+    }
+  else
+    {
+      g_test_message ("Signal received from %s %s",
+                      test_conn_descriptions[received->sender],
+                      sender_name);
+      g_assert_cmpint (received->sender, !=, TEST_CONN_NONE);
+    }
 
   g_test_message ("Signal received from %s %s via %s",
                   test_conn_descriptions[received->sender],
@@ -607,13 +679,56 @@ fixture_received_signal (Fixture    *f,
       g_test_message ("\tString argument 0: %s", received->arg0);
       g_test_message ("\tSent in step: %u", received->step);
     }
-  else
+  else if (g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(uu)")))
     {
-      g_assert_cmpstr (g_variant_get_type_string (parameters), ==, "(uu)");
       g_variant_get (parameters, "(uu)", NULL, &received->step);
       g_test_message ("\tArgument 0: (not a string)");
       g_test_message ("\tSent in step: %u", received->step);
     }
+  else if (g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(sss)")))
+    {
+      const char *name;
+      const char *old_owner;
+      const char *new_owner;
+
+      /* The only signal of this signature that we legitimately receive
+       * during this test is NameOwnerChanged, so just assert that it
+       * is from the message bus and can be matched to a plausible step.
+       * (This is less thorough than the above, and will not work if we
+       * add a test scenario where a name's ownership is repeatedly
+       * changed while watching NameOwnerChanged - so don't do that.) */
+      g_assert_cmpstr (sender_name, ==, DBUS_SERVICE_DBUS);
+      g_assert_cmpstr (path, ==, DBUS_PATH_DBUS);
+      g_assert_cmpstr (iface, ==, DBUS_INTERFACE_DBUS);
+      g_assert_cmpstr (member, ==, NAME_OWNER_CHANGED);
+
+      g_variant_get (parameters, "(&s&s&s)", &name, &old_owner, &new_owner);
+
+      for (i = 0; i < G_N_ELEMENTS (f->plan->steps); i++)
+        {
+          const TestStep *step = &f->plan->steps[i];
+
+          if (step->action == TEST_ACTION_OWN_NAME)
+            {
+              const TestOwnName *own_name = &step->u.own_name;
+
+              if (g_str_equal (name, own_name->name)
+                  && g_str_equal (new_owner, f->unique_names[own_name->owner])
+                  && own_name->received_by_conn > 0)
+                {
+                  received->step = i;
+                  break;
+                }
+            }
+
+          if (i >= G_N_ELEMENTS (f->plan->steps))
+            g_error ("Could not match message to a test step");
+        }
+    }
+  else
+    {
+      g_error ("Unexpected message received");
+    }
 
   g_ptr_array_add (f->received, g_steal_pointer (&received));
 }
@@ -782,10 +897,15 @@ fixture_emit_signal (Fixture              *f,
    * Otherwise put something that will not match any arg0.
    * Either way, put the sequence number in argument 1 so we can
    * correlate sent messages with received messages later. */
-  if (signal->arg0 != NULL)
+  if (signal->args != NULL)
     {
-      g_test_message ("\tString argument 0: %s", signal->arg0);
       /* floating */
+      body = g_variant_new_parsed (signal->args);
+      g_assert_nonnull (body);
+    }
+  else if (signal->arg0 != NULL)
+    {
+      g_test_message ("\tString argument 0: %s", signal->arg0);
       body = g_variant_new ("(su)", signal->arg0, (guint32) step_number);
     }
   else
@@ -933,8 +1053,6 @@ fixture_run_plan (Fixture          *f,
 
       g_assert_cmpuint (received->step, <, G_N_ELEMENTS (f->received_by_conn));
       g_assert_cmpuint (received->step, <, G_N_ELEMENTS (f->received_by_proxy));
-      g_assert_cmpint (plan->steps[received->step].action,
-                       ==, TEST_ACTION_EMIT_SIGNAL);
 
       if (received->received_by_proxy != NULL)
         f->received_by_proxy[received->step] += 1;
@@ -974,6 +1092,34 @@ fixture_run_plan (Fixture          *f,
               g_assert_cmpuint (f->received_by_proxy[i], ==, 0);
             }
         }
+      else if (step->action == TEST_ACTION_OWN_NAME)
+        {
+          const TestOwnName *own_name = &plan->steps[i].u.own_name;
+
+          if (mode != SUBSCRIPTION_MODE_PROXY)
+            {
+              g_test_message ("NameOwnerChanged from step %u was received %u "
+                              "times by GDBusConnection, expected %u",
+                              i, f->received_by_conn[i], own_name->received_by_conn);
+              g_assert_cmpuint (f->received_by_conn[i], ==, own_name->received_by_conn);
+            }
+          else
+            {
+              g_assert_cmpuint (f->received_by_conn[i], ==, 0);
+            }
+
+          if (mode != SUBSCRIPTION_MODE_CONN)
+            {
+              g_test_message ("NameOwnerChanged from step %u was received %u "
+                              "times by GDBusProxy, expected %u",
+                              i, f->received_by_proxy[i], own_name->received_by_proxy);
+              g_assert_cmpuint (f->received_by_proxy[i], ==, own_name->received_by_proxy);
+            }
+          else
+            {
+              g_assert_cmpuint (f->received_by_proxy[i], ==, 0);
+            }
+        }
     }
 }
 
@@ -1100,6 +1246,7 @@ main (int   argc,
   ADD_SUBSCRIBE_TEST (limit_by_unique_name);
   ADD_SUBSCRIBE_TEST (nonexistent_unique_name);
   ADD_SUBSCRIBE_TEST (limit_by_well_known_name);
+  ADD_SUBSCRIBE_TEST (limit_to_message_bus);
 
   return g_test_run();
 }
-- 
2.30.2

