gactiongroupexporter: Validate actions activated or changed over D-Bus

The action name, parameter and new state are all controlled by an
external process, so can’t be trusted. Ensure they are validated before
being passed to functions which assert that they are correctly typed and
extant.

Add unit tests.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Backport: cherry-picked to glib-2-74, and additional braces added to
  avoid a `-Wdeclaration-after-statement` warning not present on `main`
  because we’ve dropped that warning on `main`

Helps: #1904

2 files changed, 241 insertions, 1 deletions

diff --git a/gio/gactiongroupexporter.c b/gio/gactiongroupexporter.c
index 575a03ca2..3bc2f0418 100644
--- a/gio/gactiongroupexporter.c
+++ b/gio/gactiongroupexporter.c
@@ -433,11 +433,37 @@ org_gtk_Actions_method_call (GDBusConnection       *connection,
       GVariant *platform_data;
       GVariantIter *iter;
       const gchar *name;
+      const GVariantType *parameter_type = NULL;
 
       g_variant_get (parameters, "(&sav@a{sv})", &name, &iter, &platform_data);
       g_variant_iter_next (iter, "v", &parameter);
       g_variant_iter_free (iter);
 
+      /* Check the action exists and the parameter type matches. */
+      if (!g_action_group_query_action (exporter->action_group,
+                                        name, NULL, &parameter_type,
+                                        NULL, NULL, NULL))
+        {
+          g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR, G_DBUS_ERROR_INVALID_ARGS,
+                                                 "Unknown action ‘%s’", name);
+          g_clear_pointer (&parameter, g_variant_unref);
+          g_variant_unref (platform_data);
+          return;
+        }
+
+      if (!((parameter_type == NULL && parameter == NULL) ||
+            (parameter_type != NULL && parameter != NULL && g_variant_is_of_type (parameter, parameter_type))))
+        {
+          g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR, G_DBUS_ERROR_INVALID_ARGS,
+                                                 "Invalid parameter for action ‘%s’: expected type %s but got type %s",
+                                                 name,
+                                                 (parameter_type != NULL) ? (const gchar *) parameter_type : "()",
+                                                 (parameter != NULL) ? g_variant_get_type_string (parameter) : "()");
+          g_clear_pointer (&parameter, g_variant_unref);
+          g_variant_unref (platform_data);
+          return;
+        }
+
       if (G_IS_REMOTE_ACTION_GROUP (exporter->action_group))
         g_remote_action_group_activate_action_full (G_REMOTE_ACTION_GROUP (exporter->action_group),
                                                     name, parameter, platform_data);
@@ -455,9 +481,43 @@ org_gtk_Actions_method_call (GDBusConnection       *connection,
       GVariant *platform_data;
       const gchar *name;
       GVariant *state;
+      const GVariantType *state_type = NULL;
 
       g_variant_get (parameters, "(&sv@a{sv})", &name, &state, &platform_data);
 
+      /* Check the action exists and the state type matches. */
+      if (!g_action_group_query_action (exporter->action_group,
+                                        name, NULL, NULL,
+                                        &state_type, NULL, NULL))
+        {
+          g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR, G_DBUS_ERROR_INVALID_ARGS,
+                                                 "Unknown action ‘%s’", name);
+          g_variant_unref (state);
+          g_variant_unref (platform_data);
+          return;
+        }
+
+      if (state_type == NULL)
+        {
+          g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR, G_DBUS_ERROR_INVALID_ARGS,
+                                                 "Cannot change state of action ‘%s’ as it is stateless", name);
+          g_variant_unref (state);
+          g_variant_unref (platform_data);
+          return;
+        }
+
+      if (!g_variant_is_of_type (state, state_type))
+        {
+          g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR, G_DBUS_ERROR_INVALID_ARGS,
+                                                 "Invalid state for action ‘%s’: expected type %s but got type %s",
+                                                 name,
+                                                 (const gchar *) state_type,
+                                                 g_variant_get_type_string (state));
+          g_variant_unref (state);
+          g_variant_unref (platform_data);
+          return;
+        }
+
       if (G_IS_REMOTE_ACTION_GROUP (exporter->action_group))
         g_remote_action_group_change_action_state_full (G_REMOTE_ACTION_GROUP (exporter->action_group),
                                                         name, state, platform_data);
diff --git a/gio/tests/actions.c b/gio/tests/actions.c
index 3f8e6d2a1..0bd63634b 100644
--- a/gio/tests/actions.c
+++ b/gio/tests/actions.c
@@ -837,9 +837,189 @@ test_dbus_export (void)
   g_variant_unref (v);
   g_clear_object (&async_result);
 
+  /* check that activating a parameterless action over D-Bus works */
+  g_assert_cmpint (activation_count ("undo"), ==, 0);
+
+  g_dbus_connection_call (bus,
+                          g_dbus_connection_get_unique_name (bus),
+                          "/",
+                          "org.gtk.Actions",
+                          "Activate",
+                          g_variant_new ("(sava{sv})", "undo", NULL, NULL),
+                          NULL,
+                          0,
+                          G_MAXINT,
+                          NULL,
+                          async_result_cb,
+                          &async_result);
+
+  while (async_result == NULL)
+    g_main_context_iteration (NULL, TRUE);
+
+  v = g_dbus_connection_call_finish (bus, async_result, &error);
+  g_assert_no_error (error);
+  g_assert_nonnull (v);
+  g_assert_true (g_variant_is_of_type (v, G_VARIANT_TYPE_UNIT));
+  g_variant_unref (v);
+  g_clear_object (&async_result);
+
+  g_assert_cmpint (activation_count ("undo"), ==, 1);
+
+  /* check that activating a parameterful action over D-Bus works */
+  g_assert_cmpint (activation_count ("lang"), ==, 0);
+  ensure_state (G_ACTION_GROUP (group), "lang", "'latin'");
+
+  g_dbus_connection_call (bus,
+                          g_dbus_connection_get_unique_name (bus),
+                          "/",
+                          "org.gtk.Actions",
+                          "Activate",
+                          g_variant_new ("(s@ava{sv})", "lang", g_variant_new_parsed ("[<'spanish'>]"), NULL),
+                          NULL,
+                          0,
+                          G_MAXINT,
+                          NULL,
+                          async_result_cb,
+                          &async_result);
+
+  while (async_result == NULL)
+    g_main_context_iteration (NULL, TRUE);
+
+  v = g_dbus_connection_call_finish (bus, async_result, &error);
+  g_assert_no_error (error);
+  g_assert_nonnull (v);
+  g_assert_true (g_variant_is_of_type (v, G_VARIANT_TYPE_UNIT));
+  g_variant_unref (v);
+  g_clear_object (&async_result);
+
+  g_assert_cmpint (activation_count ("lang"), ==, 1);
+  ensure_state (G_ACTION_GROUP (group), "lang", "'spanish'");
+
+  /* check that various error conditions are rejected */
+  {
+  struct
+    {
+      const gchar *action_name;
+      GVariant *parameter;  /* (owned floating) (nullable) */
+    }
+  activate_error_conditions[] =
+    {
+      { "nope", NULL },  /* non-existent action */
+      { "lang", g_variant_new_parsed ("[<@u 4>]") },  /* wrong parameter type */
+      { "lang", NULL },  /* parameter missing */
+      { "undo", g_variant_new_parsed ("[<'silly'>]") },  /* extraneous parameter */
+    };
+
+  for (gsize i = 0; i < G_N_ELEMENTS (activate_error_conditions); i++)
+    {
+      GVariant *parameter = g_steal_pointer (&activate_error_conditions[i].parameter);
+      const gchar *type_string = (parameter != NULL) ? "(s@ava{sv})" : "(sava{sv})";
+
+      g_dbus_connection_call (bus,
+                              g_dbus_connection_get_unique_name (bus),
+                              "/",
+                              "org.gtk.Actions",
+                              "Activate",
+                              g_variant_new (type_string,
+                                             activate_error_conditions[i].action_name,
+                                             g_steal_pointer (&parameter),
+                                             NULL),
+                              NULL,
+                              0,
+                              G_MAXINT,
+                              NULL,
+                              async_result_cb,
+                              &async_result);
+
+      while (async_result == NULL)
+        g_main_context_iteration (NULL, TRUE);
+
+      v = g_dbus_connection_call_finish (bus, async_result, &error);
+      g_assert_error (error, G_DBUS_ERROR, G_DBUS_ERROR_INVALID_ARGS);
+      g_assert_null (v);
+      g_clear_error (&error);
+      g_clear_object (&async_result);
+    }
+  }
+
+  /* check that setting an action’s state over D-Bus works */
+  g_assert_cmpint (activation_count ("lang"), ==, 1);
+  ensure_state (G_ACTION_GROUP (group), "lang", "'spanish'");
+
+  g_dbus_connection_call (bus,
+                          g_dbus_connection_get_unique_name (bus),
+                          "/",
+                          "org.gtk.Actions",
+                          "SetState",
+                          g_variant_new ("(sva{sv})", "lang", g_variant_new_string ("portuguese"), NULL),
+                          NULL,
+                          0,
+                          G_MAXINT,
+                          NULL,
+                          async_result_cb,
+                          &async_result);
+
+  while (async_result == NULL)
+    g_main_context_iteration (NULL, TRUE);
+
+  v = g_dbus_connection_call_finish (bus, async_result, &error);
+  g_assert_no_error (error);
+  g_assert_nonnull (v);
+  g_assert_true (g_variant_is_of_type (v, G_VARIANT_TYPE_UNIT));
+  g_variant_unref (v);
+  g_clear_object (&async_result);
+
+  g_assert_cmpint (activation_count ("lang"), ==, 1);
+  ensure_state (G_ACTION_GROUP (group), "lang", "'portuguese'");
+
+  /* check that various error conditions are rejected */
+  {
+  struct
+    {
+      const gchar *action_name;
+      GVariant *state;  /* (owned floating) (not nullable) */
+    }
+  set_state_error_conditions[] =
+    {
+      { "nope", g_variant_new_string ("hello") },  /* non-existent action */
+      { "undo", g_variant_new_string ("not stateful") },  /* not a stateful action */
+      { "lang", g_variant_new_uint32 (3) },  /* wrong state type */
+    };
+
+  for (gsize i = 0; i < G_N_ELEMENTS (set_state_error_conditions); i++)
+    {
+      g_dbus_connection_call (bus,
+                              g_dbus_connection_get_unique_name (bus),
+                              "/",
+                              "org.gtk.Actions",
+                              "SetState",
+                              g_variant_new ("(s@va{sv})",
+                                             set_state_error_conditions[i].action_name,
+                                             g_variant_new_variant (g_steal_pointer (&set_state_error_conditions[i].state)),
+                                             NULL),
+                              NULL,
+                              0,
+                              G_MAXINT,
+                              NULL,
+                              async_result_cb,
+                              &async_result);
+
+      while (async_result == NULL)
+        g_main_context_iteration (NULL, TRUE);
+
+      v = g_dbus_connection_call_finish (bus, async_result, &error);
+      g_assert_error (error, G_DBUS_ERROR, G_DBUS_ERROR_INVALID_ARGS);
+      g_assert_null (v);
+      g_clear_error (&error);
+      g_clear_object (&async_result);
+    }
+  }
+
   /* test that the initial transfer works */
   g_assert_true (G_IS_DBUS_ACTION_GROUP (proxy));
-  g_assert_true (compare_action_groups (G_ACTION_GROUP (group), G_ACTION_GROUP (proxy)));
+  while (!compare_action_groups (G_ACTION_GROUP (group), G_ACTION_GROUP (proxy)))
+    g_main_context_iteration (NULL, TRUE);
+  n_actions_state_changed = 0;
 
   /* test that various changes get propagated from group to proxy */
   n_actions_added = 0;