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
This commit is contained in:
Philip Withnall 2022-11-10 13:28:10 +00:00
parent 583ed7a954
commit 58cf769033
2 changed files with 241 additions and 1 deletions

View File

@ -433,11 +433,37 @@ org_gtk_Actions_method_call (GDBusConnection *connection,
GVariant *platform_data; GVariant *platform_data;
GVariantIter *iter; GVariantIter *iter;
const gchar *name; const gchar *name;
const GVariantType *parameter_type = NULL;
g_variant_get (parameters, "(&sav@a{sv})", &name, &iter, &platform_data); g_variant_get (parameters, "(&sav@a{sv})", &name, &iter, &platform_data);
g_variant_iter_next (iter, "v", &parameter); g_variant_iter_next (iter, "v", &parameter);
g_variant_iter_free (iter); 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)) if (G_IS_REMOTE_ACTION_GROUP (exporter->action_group))
g_remote_action_group_activate_action_full (G_REMOTE_ACTION_GROUP (exporter->action_group), g_remote_action_group_activate_action_full (G_REMOTE_ACTION_GROUP (exporter->action_group),
name, parameter, platform_data); name, parameter, platform_data);
@ -455,9 +481,43 @@ org_gtk_Actions_method_call (GDBusConnection *connection,
GVariant *platform_data; GVariant *platform_data;
const gchar *name; const gchar *name;
GVariant *state; GVariant *state;
const GVariantType *state_type = NULL;
g_variant_get (parameters, "(&sv@a{sv})", &name, &state, &platform_data); 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)) if (G_IS_REMOTE_ACTION_GROUP (exporter->action_group))
g_remote_action_group_change_action_state_full (G_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); name, state, platform_data);

View File

@ -837,9 +837,189 @@ test_dbus_export (void)
g_variant_unref (v); g_variant_unref (v);
g_clear_object (&async_result); 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 actions 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 */ /* test that the initial transfer works */
g_assert_true (G_IS_DBUS_ACTION_GROUP (proxy)); 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 */ /* test that various changes get propagated from group to proxy */
n_actions_added = 0; n_actions_added = 0;