From dbe4531e8686831381446387cd2f1d8b2387ce02 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 10 Nov 2022 13:28:10 +0000 Subject: [PATCH] gactiongroupexporter: Validate actions activated or changed over D-Bus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Helps: #1904 --- gio/gactiongroupexporter.c | 60 +++++++++++++ gio/tests/actions.c | 178 ++++++++++++++++++++++++++++++++++++- 2 files changed, 237 insertions(+), 1 deletion(-) 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", ¶meter); 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, ¶meter_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 (¶meter, 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 (¶meter, 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..293e9074a 100644 --- a/gio/tests/actions.c +++ b/gio/tests/actions.c @@ -837,9 +837,185 @@ 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 (¶meter), + 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;