From 5fd101eccc65b2c83d9cb497e397cd8e92add856 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 10 Nov 2022 13:24:34 +0000 Subject: [PATCH 1/9] tests: Move a helper function around in the actions test This will be used in an upcoming commit. This introduces no functional changes. Signed-off-by: Philip Withnall Helps: #1904 --- gio/tests/actions.c | 76 ++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/gio/tests/actions.c b/gio/tests/actions.c index 749a782f1..3f8e6d2a1 100644 --- a/gio/tests/actions.c +++ b/gio/tests/actions.c @@ -190,6 +190,22 @@ strv_set_equal (const gchar * const *strv, ...) return res; } +static void +ensure_state (GActionGroup *group, + const gchar *action_name, + const gchar *expected) +{ + GVariant *value; + gchar *printed; + + value = g_action_group_get_action_state (group, action_name); + printed = g_variant_print (value, TRUE); + g_variant_unref (value); + + g_assert_cmpstr (printed, ==, expected); + g_free (printed); +} + G_GNUC_BEGIN_IGNORE_DEPRECATIONS static void @@ -1030,22 +1046,6 @@ verify_changed (const gchar *log_entry) g_clear_pointer (&state_change_log, g_free); } -static void -ensure_state (GSimpleActionGroup *group, - const gchar *action_name, - const gchar *expected) -{ - GVariant *value; - gchar *printed; - - value = g_action_group_get_action_state (G_ACTION_GROUP (group), action_name); - printed = g_variant_print (value, TRUE); - g_variant_unref (value); - - g_assert_cmpstr (printed, ==, expected); - g_free (printed); -} - static void test_property_actions (void) { @@ -1106,11 +1106,11 @@ test_property_actions (void) g_object_unref (client); g_object_unref (app); - ensure_state (group, "app-id", "'org.gtk.test'"); - ensure_state (group, "keepalive", "uint32 0"); - ensure_state (group, "tls", "false"); - ensure_state (group, "disable-proxy", "false"); - ensure_state (group, "type", "'stream'"); + ensure_state (G_ACTION_GROUP (group), "app-id", "'org.gtk.test'"); + ensure_state (G_ACTION_GROUP (group), "keepalive", "uint32 0"); + ensure_state (G_ACTION_GROUP (group), "tls", "false"); + ensure_state (G_ACTION_GROUP (group), "disable-proxy", "false"); + ensure_state (G_ACTION_GROUP (group), "type", "'stream'"); verify_changed (NULL); @@ -1118,88 +1118,88 @@ test_property_actions (void) g_action_group_change_action_state (G_ACTION_GROUP (group), "app-id", g_variant_new ("s", "org.gtk.test2")); verify_changed ("app-id:'org.gtk.test2'"); g_assert_cmpstr (g_application_get_application_id (app), ==, "org.gtk.test2"); - ensure_state (group, "app-id", "'org.gtk.test2'"); + ensure_state (G_ACTION_GROUP (group), "app-id", "'org.gtk.test2'"); g_action_group_activate_action (G_ACTION_GROUP (group), "app-id", g_variant_new ("s", "org.gtk.test3")); verify_changed ("app-id:'org.gtk.test3'"); g_assert_cmpstr (g_application_get_application_id (app), ==, "org.gtk.test3"); - ensure_state (group, "app-id", "'org.gtk.test3'"); + ensure_state (G_ACTION_GROUP (group), "app-id", "'org.gtk.test3'"); g_application_set_application_id (app, "org.gtk.test"); verify_changed ("app-id:'org.gtk.test'"); - ensure_state (group, "app-id", "'org.gtk.test'"); + ensure_state (G_ACTION_GROUP (group), "app-id", "'org.gtk.test'"); /* uint tests */ g_action_group_change_action_state (G_ACTION_GROUP (group), "keepalive", g_variant_new ("u", 1234)); verify_changed ("keepalive:uint32 1234"); g_assert_cmpuint (g_application_get_inactivity_timeout (app), ==, 1234); - ensure_state (group, "keepalive", "uint32 1234"); + ensure_state (G_ACTION_GROUP (group), "keepalive", "uint32 1234"); g_action_group_activate_action (G_ACTION_GROUP (group), "keepalive", g_variant_new ("u", 5678)); verify_changed ("keepalive:uint32 5678"); g_assert_cmpuint (g_application_get_inactivity_timeout (app), ==, 5678); - ensure_state (group, "keepalive", "uint32 5678"); + ensure_state (G_ACTION_GROUP (group), "keepalive", "uint32 5678"); g_application_set_inactivity_timeout (app, 0); verify_changed ("keepalive:uint32 0"); - ensure_state (group, "keepalive", "uint32 0"); + ensure_state (G_ACTION_GROUP (group), "keepalive", "uint32 0"); /* bool tests */ g_action_group_change_action_state (G_ACTION_GROUP (group), "tls", g_variant_new ("b", TRUE)); verify_changed ("tls:true"); g_assert_true (g_socket_client_get_tls (client)); - ensure_state (group, "tls", "true"); + ensure_state (G_ACTION_GROUP (group), "tls", "true"); g_action_group_change_action_state (G_ACTION_GROUP (group), "disable-proxy", g_variant_new ("b", TRUE)); verify_changed ("disable-proxy:true"); - ensure_state (group, "disable-proxy", "true"); + ensure_state (G_ACTION_GROUP (group), "disable-proxy", "true"); g_assert_false (g_socket_client_get_enable_proxy (client)); /* test toggle true->false */ g_action_group_activate_action (G_ACTION_GROUP (group), "tls", NULL); verify_changed ("tls:false"); g_assert_false (g_socket_client_get_tls (client)); - ensure_state (group, "tls", "false"); + ensure_state (G_ACTION_GROUP (group), "tls", "false"); /* and now back false->true */ g_action_group_activate_action (G_ACTION_GROUP (group), "tls", NULL); verify_changed ("tls:true"); g_assert_true (g_socket_client_get_tls (client)); - ensure_state (group, "tls", "true"); + ensure_state (G_ACTION_GROUP (group), "tls", "true"); g_socket_client_set_tls (client, FALSE); verify_changed ("tls:false"); - ensure_state (group, "tls", "false"); + ensure_state (G_ACTION_GROUP (group), "tls", "false"); /* now do the same for the inverted action */ g_action_group_activate_action (G_ACTION_GROUP (group), "disable-proxy", NULL); verify_changed ("disable-proxy:false"); g_assert_true (g_socket_client_get_enable_proxy (client)); - ensure_state (group, "disable-proxy", "false"); + ensure_state (G_ACTION_GROUP (group), "disable-proxy", "false"); g_action_group_activate_action (G_ACTION_GROUP (group), "disable-proxy", NULL); verify_changed ("disable-proxy:true"); g_assert_false (g_socket_client_get_enable_proxy (client)); - ensure_state (group, "disable-proxy", "true"); + ensure_state (G_ACTION_GROUP (group), "disable-proxy", "true"); g_socket_client_set_enable_proxy (client, TRUE); verify_changed ("disable-proxy:false"); - ensure_state (group, "disable-proxy", "false"); + ensure_state (G_ACTION_GROUP (group), "disable-proxy", "false"); /* enum tests */ g_action_group_change_action_state (G_ACTION_GROUP (group), "type", g_variant_new ("s", "datagram")); verify_changed ("type:'datagram'"); g_assert_cmpint (g_socket_client_get_socket_type (client), ==, G_SOCKET_TYPE_DATAGRAM); - ensure_state (group, "type", "'datagram'"); + ensure_state (G_ACTION_GROUP (group), "type", "'datagram'"); g_action_group_activate_action (G_ACTION_GROUP (group), "type", g_variant_new ("s", "stream")); verify_changed ("type:'stream'"); g_assert_cmpint (g_socket_client_get_socket_type (client), ==, G_SOCKET_TYPE_STREAM); - ensure_state (group, "type", "'stream'"); + ensure_state (G_ACTION_GROUP (group), "type", "'stream'"); g_socket_client_set_socket_type (client, G_SOCKET_TYPE_SEQPACKET); verify_changed ("type:'seqpacket'"); - ensure_state (group, "type", "'seqpacket'"); + ensure_state (G_ACTION_GROUP (group), "type", "'seqpacket'"); /* Check some error cases... */ g_test_expect_message ("GLib-GIO", G_LOG_LEVEL_CRITICAL, "*non-existent*"); From dbe4531e8686831381446387cd2f1d8b2387ce02 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 10 Nov 2022 13:28:10 +0000 Subject: [PATCH 2/9] 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; From da634e7a2565646df54231edfe299b2e04e6c71d Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 10 Nov 2022 22:58:40 +0000 Subject: [PATCH 3/9] gapplication: Validate actions activated over D-Bus As with the previous commit, the arguments to `ActivateAction` have to be validated before being passed to `g_action_group_activate_action()`. As they come over D-Bus, they are coming from an untrusted source. Includes unit tests for all D-Bus methods on `GApplication`. Signed-off-by: Philip Withnall Helps: #1904 --- gio/gapplicationimpl-dbus.c | 26 +++ gio/tests/gapplication.c | 439 ++++++++++++++++++++++++++++++++++++ 2 files changed, 465 insertions(+) diff --git a/gio/gapplicationimpl-dbus.c b/gio/gapplicationimpl-dbus.c index bcad19d2e..ac6644d88 100644 --- a/gio/gapplicationimpl-dbus.c +++ b/gio/gapplicationimpl-dbus.c @@ -286,6 +286,7 @@ g_application_impl_method_call (GDBusConnection *connection, GVariant *platform_data; GVariantIter *iter; const gchar *name; + const GVariantType *parameter_type = NULL; /* Only on the freedesktop interface */ @@ -293,6 +294,31 @@ g_application_impl_method_call (GDBusConnection *connection, 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 (impl->exported_actions, + 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; + } + class->before_emit (impl->app, platform_data); g_action_group_activate_action (impl->exported_actions, name, parameter); class->after_emit (impl->app, platform_data); diff --git a/gio/tests/gapplication.c b/gio/tests/gapplication.c index bf0395cdb..46d4949b4 100644 --- a/gio/tests/gapplication.c +++ b/gio/tests/gapplication.c @@ -1194,6 +1194,441 @@ test_replace (gconstpointer data) } } +static void +dbus_activate_cb (GApplication *app, + gpointer user_data) +{ + guint *n_activations = user_data; + + *n_activations = *n_activations + 1; + g_main_context_wakeup (NULL); +} + +static void dbus_startup_reply_cb (GObject *source_object, + GAsyncResult *result, + gpointer user_data); + +static void +dbus_startup_cb (GApplication *app, + gpointer user_data) +{ + GDBusConnection *connection = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL); + GDBusMessage *message = G_DBUS_MESSAGE (user_data); + + g_assert_nonnull (connection); + + g_dbus_connection_send_message_with_reply (connection, message, + G_DBUS_SEND_MESSAGE_FLAGS_NONE, -1, + NULL, NULL, + dbus_startup_reply_cb, g_object_ref (app)); + + g_clear_object (&connection); +} + +static void +dbus_startup_reply_cb (GObject *source_object, + GAsyncResult *result, + gpointer user_data) +{ + GDBusConnection *connection = G_DBUS_CONNECTION (source_object); + GApplication *app = G_APPLICATION (user_data); + GDBusMessage *reply = NULL; + GError *local_error = NULL; + + reply = g_dbus_connection_send_message_with_reply_finish (connection, result, &local_error); + g_assert_no_error (local_error); + + /* Nothing to check on the reply for now. */ + g_clear_object (&reply); + + g_application_release (app); + g_clear_object (&app); +} + +static void +test_dbus_activate (void) +{ + GTestDBus *bus = NULL; + GVariantBuilder builder; + GDBusMessage *message = NULL; + GPtrArray *messages = NULL; /* (element-type GDBusMessage) (owned) */ + gsize i; + + g_test_summary ("Test that calling the Activate D-Bus method works"); + + /* Try various different messages */ + messages = g_ptr_array_new_with_free_func (g_object_unref); + + /* Via org.gtk.Application */ + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.Activate", + "/org/gtk/TestApplication/Activate", + "org.gtk.Application", + "Activate"); + g_dbus_message_set_body (message, g_variant_new ("(a{sv})", NULL)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* Via org.freedesktop.Application */ + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.Activate", + "/org/gtk/TestApplication/Activate", + "org.freedesktop.Application", + "Activate"); + g_dbus_message_set_body (message, g_variant_new ("(a{sv})", NULL)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* With some platform data */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}")); + g_variant_builder_add (&builder, "{sv}", "cwd", g_variant_new_bytestring ("/home/henry")); + + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.Activate", + "/org/gtk/TestApplication/Activate", + "org.gtk.Application", + "Activate"); + g_dbus_message_set_body (message, g_variant_new ("(a{sv})", &builder)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* Try each message */ + bus = g_test_dbus_new (G_TEST_DBUS_NONE); + g_test_dbus_up (bus); + + for (i = 0; i < messages->len; i++) + { + GApplication *app = NULL; + gulong activate_id, startup_id; + guint n_activations = 0; + + g_test_message ("Message %" G_GSIZE_FORMAT, i); + + app = g_application_new ("org.gtk.TestApplication.Activate", G_APPLICATION_DEFAULT_FLAGS); + activate_id = g_signal_connect (app, "activate", G_CALLBACK (dbus_activate_cb), &n_activations); + startup_id = g_signal_connect (app, "startup", G_CALLBACK (dbus_startup_cb), messages->pdata[i]); + + g_application_hold (app); + g_application_run (app, 0, NULL); + + /* It’ll be activated once as normal, and once due to the D-Bus call */ + g_assert_cmpuint (n_activations, ==, 2); + + g_signal_handler_disconnect (app, startup_id); + g_signal_handler_disconnect (app, activate_id); + g_clear_object (&app); + } + + g_ptr_array_unref (messages); + + g_test_dbus_down (bus); + g_clear_object (&bus); +} + +static void +dbus_activate_noop_cb (GApplication *app, + gpointer user_data) +{ + /* noop */ +} + +static void +dbus_open_cb (GApplication *app, + gpointer files, + int n_files, + char *hint, + gpointer user_data) +{ + guint *n_opens = user_data; + + *n_opens = *n_opens + 1; + g_main_context_wakeup (NULL); +} + +static void +test_dbus_open (void) +{ + GTestDBus *bus = NULL; + GVariantBuilder builder, builder2; + GDBusMessage *message = NULL; + GPtrArray *messages = NULL; /* (element-type GDBusMessage) (owned) */ + gsize i; + + g_test_summary ("Test that calling the Open D-Bus method works"); + + /* Try various different messages */ + messages = g_ptr_array_new_with_free_func (g_object_unref); + + /* Via org.gtk.Application */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("as")); + g_variant_builder_add (&builder, "s", "file:///home/henry/test"); + + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.Open", + "/org/gtk/TestApplication/Open", + "org.gtk.Application", + "Open"); + g_dbus_message_set_body (message, g_variant_new ("(assa{sv})", &builder, "hint", NULL)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* Via org.freedesktop.Application (which has no hint parameter) */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("as")); + g_variant_builder_add (&builder, "s", "file:///home/henry/test"); + + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.Open", + "/org/gtk/TestApplication/Open", + "org.freedesktop.Application", + "Open"); + g_dbus_message_set_body (message, g_variant_new ("(asa{sv})", &builder, NULL)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* With some platform data and more than one file */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("as")); + g_variant_builder_add (&builder, "s", "file:///home/henry/test"); + g_variant_builder_add (&builder, "s", "file:///home/henry/test2"); + + g_variant_builder_init (&builder2, G_VARIANT_TYPE ("a{sv}")); + g_variant_builder_add (&builder2, "{sv}", "cwd", g_variant_new_bytestring ("/home/henry")); + + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.Open", + "/org/gtk/TestApplication/Open", + "org.gtk.Application", + "Open"); + g_dbus_message_set_body (message, g_variant_new ("(assa{sv})", &builder, "", &builder2)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* No files */ + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.Open", + "/org/gtk/TestApplication/Open", + "org.gtk.Application", + "Open"); + g_dbus_message_set_body (message, g_variant_new ("(assa{sv})", NULL, "", NULL)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* Try each message */ + bus = g_test_dbus_new (G_TEST_DBUS_NONE); + g_test_dbus_up (bus); + + for (i = 0; i < messages->len; i++) + { + GApplication *app = NULL; + gulong activate_id, open_id, startup_id; + guint n_opens = 0; + + g_test_message ("Message %" G_GSIZE_FORMAT, i); + + app = g_application_new ("org.gtk.TestApplication.Open", G_APPLICATION_HANDLES_OPEN); + activate_id = g_signal_connect (app, "activate", G_CALLBACK (dbus_activate_noop_cb), NULL); + open_id = g_signal_connect (app, "open", G_CALLBACK (dbus_open_cb), &n_opens); + startup_id = g_signal_connect (app, "startup", G_CALLBACK (dbus_startup_cb), messages->pdata[i]); + + g_application_hold (app); + g_application_run (app, 0, NULL); + + g_assert_cmpuint (n_opens, ==, 1); + + g_signal_handler_disconnect (app, startup_id); + g_signal_handler_disconnect (app, open_id); + g_signal_handler_disconnect (app, activate_id); + g_clear_object (&app); + } + + g_ptr_array_unref (messages); + + g_test_dbus_down (bus); + g_clear_object (&bus); +} + +static void +dbus_command_line_cb (GApplication *app, + GApplicationCommandLine *command_line, + gpointer user_data) +{ + guint *n_command_lines = user_data; + + *n_command_lines = *n_command_lines + 1; + g_main_context_wakeup (NULL); +} + +static void +test_dbus_command_line (void) +{ + GTestDBus *bus = NULL; + GVariantBuilder builder; + GDBusMessage *message = NULL; + GPtrArray *messages = NULL; /* (element-type GDBusMessage) (owned) */ + gsize i; + + g_test_summary ("Test that calling the CommandLine D-Bus method works"); + + /* Try various different messages */ + messages = g_ptr_array_new_with_free_func (g_object_unref); + + /* Via org.gtk.Application */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("aay")); + g_variant_builder_add (&builder, "^ay", "test-program"); + g_variant_builder_add (&builder, "^ay", "--open"); + g_variant_builder_add (&builder, "^ay", "/path/to/something"); + + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.CommandLine", + "/org/gtk/TestApplication/CommandLine", + "org.gtk.Application", + "CommandLine"); + g_dbus_message_set_body (message, g_variant_new ("(oaaya{sv})", + "/my/org/gtk/private/CommandLine", + &builder, NULL)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* Try each message */ + bus = g_test_dbus_new (G_TEST_DBUS_NONE); + g_test_dbus_up (bus); + + for (i = 0; i < messages->len; i++) + { + GApplication *app = NULL; + gulong activate_id, command_line_id, startup_id; + guint n_command_lines = 0; + + g_test_message ("Message %" G_GSIZE_FORMAT, i); + + app = g_application_new ("org.gtk.TestApplication.CommandLine", G_APPLICATION_HANDLES_COMMAND_LINE); + activate_id = g_signal_connect (app, "activate", G_CALLBACK (dbus_activate_noop_cb), NULL); + command_line_id = g_signal_connect (app, "command-line", G_CALLBACK (dbus_command_line_cb), &n_command_lines); + startup_id = g_signal_connect (app, "startup", G_CALLBACK (dbus_startup_cb), messages->pdata[i]); + + g_application_hold (app); + g_application_run (app, 0, NULL); + + /* It’s called once for handling the local command line on startup, and again + * for the D-Bus call */ + g_assert_cmpuint (n_command_lines, ==, 2); + + g_signal_handler_disconnect (app, startup_id); + g_signal_handler_disconnect (app, command_line_id); + g_signal_handler_disconnect (app, activate_id); + g_clear_object (&app); + } + + g_ptr_array_unref (messages); + + g_test_dbus_down (bus); + g_clear_object (&bus); +} + +static void +dbus_activate_action_cb (GSimpleAction *action, + GVariant *parameter, + gpointer user_data) +{ + guint *n_activations = user_data; + + *n_activations = *n_activations + 1; + g_main_context_wakeup (NULL); +} + +static void +test_dbus_activate_action (void) +{ + GTestDBus *bus = NULL; + GVariantBuilder builder; + struct + { + GDBusMessage *message; /* (not nullable) (owned) */ + guint n_expected_activations; + } messages[6]; + gsize i; + + g_test_summary ("Test that calling the ActivateAction D-Bus method works"); + + /* Action without parameter */ + messages[0].message = g_dbus_message_new_method_call ("org.gtk.TestApplication.ActivateAction", + "/org/gtk/TestApplication/ActivateAction", + "org.freedesktop.Application", + "ActivateAction"); + g_dbus_message_set_body (messages[0].message, g_variant_new ("(sava{sv})", "undo", NULL, NULL)); + messages[0].n_expected_activations = 1; + + /* Action with parameter */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("av")); + g_variant_builder_add (&builder, "v", g_variant_new_string ("spanish")); + + messages[1].message = g_dbus_message_new_method_call ("org.gtk.TestApplication.ActivateAction", + "/org/gtk/TestApplication/ActivateAction", + "org.freedesktop.Application", + "ActivateAction"); + g_dbus_message_set_body (messages[1].message, g_variant_new ("(sava{sv})", "lang", &builder, NULL)); + messages[1].n_expected_activations = 1; + + /* Action with unexpected parameter */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("av")); + g_variant_builder_add (&builder, "v", g_variant_new_string ("should not be passed")); + + messages[2].message = g_dbus_message_new_method_call ("org.gtk.TestApplication.ActivateAction", + "/org/gtk/TestApplication/ActivateAction", + "org.freedesktop.Application", + "ActivateAction"); + g_dbus_message_set_body (messages[2].message, g_variant_new ("(sava{sv})", "undo", &builder, NULL)); + messages[2].n_expected_activations = 0; + + /* Action without required parameter */ + messages[3].message = g_dbus_message_new_method_call ("org.gtk.TestApplication.ActivateAction", + "/org/gtk/TestApplication/ActivateAction", + "org.freedesktop.Application", + "ActivateAction"); + g_dbus_message_set_body (messages[3].message, g_variant_new ("(sava{sv})", "lang", NULL, NULL)); + messages[3].n_expected_activations = 0; + + /* Action with wrong parameter type */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("av")); + g_variant_builder_add (&builder, "v", g_variant_new_uint32 (42)); + + messages[4].message = g_dbus_message_new_method_call ("org.gtk.TestApplication.ActivateAction", + "/org/gtk/TestApplication/ActivateAction", + "org.freedesktop.Application", + "ActivateAction"); + g_dbus_message_set_body (messages[4].message, g_variant_new ("(sava{sv})", "lang", &builder, NULL)); + messages[4].n_expected_activations = 0; + + /* Nonexistent action */ + messages[5].message = g_dbus_message_new_method_call ("org.gtk.TestApplication.ActivateAction", + "/org/gtk/TestApplication/ActivateAction", + "org.freedesktop.Application", + "ActivateAction"); + g_dbus_message_set_body (messages[5].message, g_variant_new ("(sava{sv})", "nonexistent", NULL, NULL)); + messages[5].n_expected_activations = 0; + + /* Try each message */ + bus = g_test_dbus_new (G_TEST_DBUS_NONE); + g_test_dbus_up (bus); + + for (i = 0; i < G_N_ELEMENTS (messages); i++) + { + GApplication *app = NULL; + gulong activate_id, startup_id; + const GActionEntry entries[] = + { + { "undo", dbus_activate_action_cb, NULL, NULL, NULL, { 0 } }, + { "lang", dbus_activate_action_cb, "s", "'latin'", NULL, { 0 } }, + }; + guint n_activations = 0; + + g_test_message ("Message %" G_GSIZE_FORMAT, i); + + app = g_application_new ("org.gtk.TestApplication.ActivateAction", G_APPLICATION_DEFAULT_FLAGS); + activate_id = g_signal_connect (app, "activate", G_CALLBACK (dbus_activate_noop_cb), NULL); + startup_id = g_signal_connect (app, "startup", G_CALLBACK (dbus_startup_cb), messages[i].message); + + /* Export some actions. */ + g_action_map_add_action_entries (G_ACTION_MAP (app), entries, G_N_ELEMENTS (entries), &n_activations); + + g_application_hold (app); + g_application_run (app, 0, NULL); + + g_assert_cmpuint (n_activations, ==, messages[i].n_expected_activations); + + g_signal_handler_disconnect (app, startup_id); + g_signal_handler_disconnect (app, activate_id); + g_clear_object (&app); + g_clear_object (&messages[i].message); + } + + g_test_dbus_down (bus); + g_clear_object (&bus); +} + int main (int argc, char **argv) { @@ -1225,6 +1660,10 @@ main (int argc, char **argv) g_test_add_func ("/gapplication/api", test_api); g_test_add_data_func ("/gapplication/replace", GINT_TO_POINTER (TRUE), test_replace); g_test_add_data_func ("/gapplication/no-replace", GINT_TO_POINTER (FALSE), test_replace); + g_test_add_func ("/gapplication/dbus/activate", test_dbus_activate); + g_test_add_func ("/gapplication/dbus/open", test_dbus_open); + g_test_add_func ("/gapplication/dbus/command-line", test_dbus_command_line); + g_test_add_func ("/gapplication/dbus/activate-action", test_dbus_activate_action); return g_test_run (); } From 3987f41f8c2d58a6cf079f13c81683468c939470 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 15 Nov 2022 18:05:28 +0000 Subject: [PATCH 4/9] =?UTF-8?q?gfdonotificationbackend:=20Don=E2=80=99t=20?= =?UTF-8?q?remove=20notification=20if=20invoking=20action=20fails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Invoking an action on a notification should remove it (by default, unless the `resident` hint is set, but GLib doesn’t currently support that). If, somehow, an invalid action is invoked on the notification, that shouldn’t cause it to be removed though, because no action has taken place. So change the code to do that. Signed-off-by: Philip Withnall --- gio/gfdonotificationbackend.c | 55 ++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/gio/gfdonotificationbackend.c b/gio/gfdonotificationbackend.c index dd93a5dd8..682190d90 100644 --- a/gio/gfdonotificationbackend.c +++ b/gio/gfdonotificationbackend.c @@ -131,7 +131,7 @@ g_fdo_notification_backend_find_notification_by_notify_id (GFdoNotificationBacke return NULL; } -static void +static gboolean activate_action (GFdoNotificationBackend *backend, const gchar *name, GVariant *parameter) @@ -141,15 +141,19 @@ activate_action (GFdoNotificationBackend *backend, /* Callers should not provide a floating variant here */ g_assert (parameter == NULL || !g_variant_is_floating (parameter)); - if (name) + if (name != NULL && + g_str_has_prefix (name, "app.")) { - if (g_str_has_prefix (name, "app.")) - g_action_group_activate_action (G_ACTION_GROUP (g_backend->application), name + 4, parameter); + g_action_group_activate_action (G_ACTION_GROUP (g_backend->application), name + 4, parameter); + return TRUE; } - else + else if (name == NULL) { g_application_activate (g_backend->application); + return TRUE; } + + return FALSE; } static void @@ -165,6 +169,7 @@ notify_signal (GDBusConnection *connection, guint32 id = 0; const gchar *action = NULL; FreedesktopNotification *n; + gboolean notification_closed = TRUE; if (g_str_equal (signal_name, "NotificationClosed") && g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(uu)"))) @@ -187,29 +192,39 @@ notify_signal (GDBusConnection *connection, { if (g_str_equal (action, "default")) { - activate_action (backend, n->default_action, n->default_action_target); + if (!activate_action (backend, n->default_action, n->default_action_target)) + notification_closed = FALSE; } else { - gchar *name; - GVariant *target; + gchar *name = NULL; + GVariant *target = NULL; - if (g_action_parse_detailed_name (action, &name, &target, NULL)) - { - activate_action (backend, name, target); - g_free (name); - if (target) - g_variant_unref (target); - } + if (!g_action_parse_detailed_name (action, &name, &target, NULL) || + !activate_action (backend, name, target)) + notification_closed = FALSE; + + g_free (name); + g_clear_pointer (&target, g_variant_unref); } } - /* Get the notification again in case the action redrew it */ - n = g_fdo_notification_backend_find_notification_by_notify_id (backend, id); - if (n != NULL) + /* Remove the notification, as it’s either been explicitly closed + * (`NotificationClosed` signal) or has been closed as a result of activating + * an action successfully. GLib doesn’t currently support the `resident` hint + * on notifications which would allow them to stay around after having an + * action invoked on them (see + * https://specifications.freedesktop.org/notification-spec/notification-spec-latest.html#idm45877717456448) + * + * First, get the notification again in case the action redrew it */ + if (notification_closed) { - backend->notifications = g_slist_remove (backend->notifications, n); - freedesktop_notification_free (n); + n = g_fdo_notification_backend_find_notification_by_notify_id (backend, id); + if (n != NULL) + { + backend->notifications = g_slist_remove (backend->notifications, n); + freedesktop_notification_free (n); + } } } From 08012bd3e0568f65e9453eee2a3920c6f1e628c8 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 14 Nov 2022 16:27:35 +0000 Subject: [PATCH 5/9] tests: Add stub tests for GFdoNotificationBackend This test is fairly pointless, but puts the infrastructure in place for adding more tests for `GFdoNotificationBackend` in upcoming commits. Signed-off-by: Philip Withnall Helps: #1904 --- gio/tests/fdo-notification-backend.c | 103 +++++++++++++++++++++++++++ gio/tests/meson.build | 1 + 2 files changed, 104 insertions(+) create mode 100644 gio/tests/fdo-notification-backend.c diff --git a/gio/tests/fdo-notification-backend.c b/gio/tests/fdo-notification-backend.c new file mode 100644 index 000000000..50d2599d8 --- /dev/null +++ b/gio/tests/fdo-notification-backend.c @@ -0,0 +1,103 @@ +/* + * Copyright © 2022 Endless OS Foundation, LLC + * + * SPDX-License-Identifier: LGPL-2.1-or-later + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General + * Public License along with this library; if not, see . + * + * Author: Philip Withnall + */ + +#include +#include + +#include +#include "gio/gnotificationbackend.h" + + +static GNotificationBackend * +construct_backend (GApplication **app_out) +{ + GApplication *app = NULL; + GType fdo_type = G_TYPE_INVALID; + GNotificationBackend *backend = NULL; + GError *local_error = NULL; + + /* Construct the app first and withdraw a notification, to ensure that IO modules are loaded. */ + app = g_application_new ("org.gtk.TestApplication", G_APPLICATION_DEFAULT_FLAGS); + g_application_register (app, NULL, &local_error); + g_assert_no_error (local_error); + g_application_withdraw_notification (app, "org.gtk.TestApplication.NonexistentNotification"); + + fdo_type = g_type_from_name ("GFdoNotificationBackend"); + g_assert_cmpuint (fdo_type, !=, G_TYPE_INVALID); + + /* Replicate the behaviour from g_notification_backend_new_default(), which is + * not exported publicly so can‘t be easily used in the test. */ + backend = g_object_new (fdo_type, NULL); + backend->application = app; + backend->dbus_connection = g_application_get_dbus_connection (app); + if (backend->dbus_connection) + g_object_ref (backend->dbus_connection); + + if (app_out != NULL) + *app_out = g_object_ref (app); + + g_clear_object (&app); + + return g_steal_pointer (&backend); +} + +static void +test_construction (void) +{ + GNotificationBackend *backend = NULL; + GApplication *app = NULL; + GTestDBus *bus = NULL; + + g_test_message ("Test constructing a GFdoNotificationBackend"); + + /* Set up a test session bus and connection. */ + bus = g_test_dbus_new (G_TEST_DBUS_NONE); + g_test_dbus_up (bus); + + backend = construct_backend (&app); + g_assert_nonnull (backend); + + g_application_quit (app); + g_clear_object (&app); + g_clear_object (&backend); + + g_test_dbus_down (bus); + g_clear_object (&bus); +} + +int +main (int argc, + char *argv[]) +{ + setlocale (LC_ALL, ""); + + /* Force use of the FDO backend */ + g_setenv ("GNOTIFICATION_BACKEND", "freedesktop", TRUE); + + g_test_init (&argc, &argv, NULL); + + /* Make sure we don’t send notifications to the actual D-Bus session. */ + g_test_dbus_unset (); + + g_test_add_func ("/fdo-notification-backend/construction", test_construction); + + return g_test_run (); +} diff --git a/gio/tests/meson.build b/gio/tests/meson.build index 11acad460..c7fd4ed8e 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -405,6 +405,7 @@ if host_machine.system() != 'windows' 'extra_sources' : extra_sources, 'suite' : ['slow'], }, + 'fdo-notification-backend': {}, 'gdbus-auth' : {'extra_sources' : extra_sources}, 'gdbus-bz627724' : {'extra_sources' : extra_sources}, 'gdbus-close-pending' : {'extra_sources' : extra_sources}, From 83c11637ba97e6c8b1ac99105aceab85c80b9a51 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 10 Nov 2022 23:05:26 +0000 Subject: [PATCH 6/9] gfdonotificationbackend: Validate actions before activating them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These actions are activated as a result of receiving the `ActionInvoked` signal from `org.freedesktop.Notifications`. As that’s received from another process over D-Bus, it’s feasible that it could be malformed. Without validating the action and its parameter, assertions will be hit within the `GAction` code. While we should be able to trust whatever process owns `org.freedesktop.Notifications`, it’s possible that’s not the case, so best validate what we receive. Includes unit tests. Signed-off-by: Philip Withnall Helps: #1904 --- gio/gfdonotificationbackend.c | 15 +- gio/tests/fdo-notification-backend.c | 223 +++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 2 deletions(-) diff --git a/gio/gfdonotificationbackend.c b/gio/gfdonotificationbackend.c index 682190d90..594ed9966 100644 --- a/gio/gfdonotificationbackend.c +++ b/gio/gfdonotificationbackend.c @@ -144,8 +144,19 @@ activate_action (GFdoNotificationBackend *backend, if (name != NULL && g_str_has_prefix (name, "app.")) { - g_action_group_activate_action (G_ACTION_GROUP (g_backend->application), name + 4, parameter); - return TRUE; + const GVariantType *parameter_type = NULL; + const gchar *action_name = name + strlen ("app."); + + /* @name and @parameter come as untrusted input over D-Bus, so validate them first */ + if (g_action_group_query_action (G_ACTION_GROUP (g_backend->application), + action_name, NULL, ¶meter_type, + NULL, NULL, NULL) && + ((parameter_type == NULL && parameter == NULL) || + (parameter_type != NULL && parameter != NULL && g_variant_is_of_type (parameter, parameter_type)))) + { + g_action_group_activate_action (G_ACTION_GROUP (g_backend->application), action_name, parameter); + return TRUE; + } } else if (name == NULL) { diff --git a/gio/tests/fdo-notification-backend.c b/gio/tests/fdo-notification-backend.c index 50d2599d8..aed9aab67 100644 --- a/gio/tests/fdo-notification-backend.c +++ b/gio/tests/fdo-notification-backend.c @@ -83,6 +83,228 @@ test_construction (void) g_clear_object (&bus); } +static void +daemon_method_call_cb (GDBusConnection *connection, + const gchar *sender, + const gchar *object_path, + const gchar *interface_name, + const gchar *method_name, + GVariant *parameters, + GDBusMethodInvocation *invocation, + gpointer user_data) +{ + GDBusMethodInvocation **current_method_invocation_out = user_data; + + g_assert_null (*current_method_invocation_out); + *current_method_invocation_out = g_steal_pointer (&invocation); + + g_main_context_wakeup (NULL); +} + +static void +name_acquired_or_lost_cb (GDBusConnection *connection, + const gchar *name, + gpointer user_data) +{ + gboolean *name_acquired = user_data; + + *name_acquired = !*name_acquired; + + g_main_context_wakeup (NULL); +} + +static void +dbus_activate_action_cb (GSimpleAction *action, + GVariant *parameter, + gpointer user_data) +{ + guint *n_activations = user_data; + + *n_activations = *n_activations + 1; + g_main_context_wakeup (NULL); +} + +static void +assert_send_notification (GNotificationBackend *backend, + GDBusMethodInvocation **current_method_invocation, + guint32 notify_id) +{ + GNotification *notification = NULL; + + notification = g_notification_new ("Some Notification"); + G_NOTIFICATION_BACKEND_GET_CLASS (backend)->send_notification (backend, "notification1", notification); + g_clear_object (¬ification); + + while (*current_method_invocation == NULL) + g_main_context_iteration (NULL, TRUE); + + g_assert_cmpstr (g_dbus_method_invocation_get_interface_name (*current_method_invocation), ==, "org.freedesktop.Notifications"); + g_assert_cmpstr (g_dbus_method_invocation_get_method_name (*current_method_invocation), ==, "Notify"); + g_dbus_method_invocation_return_value (g_steal_pointer (current_method_invocation), g_variant_new ("(u)", notify_id)); +} + +static void +assert_emit_action_invoked (GDBusConnection *daemon_connection, + GVariant *parameters) +{ + GError *local_error = NULL; + + g_dbus_connection_emit_signal (daemon_connection, + NULL, + "/org/freedesktop/Notifications", + "org.freedesktop.Notifications", + "ActionInvoked", + parameters, + &local_error); + g_assert_no_error (local_error); +} + +static void +test_dbus_activate_action (void) +{ + /* Very trimmed down version of + * https://specifications.freedesktop.org/notification-spec/notification-spec-latest.html */ + const GDBusArgInfo daemon_notify_in_app_name = { -1, "AppName", "s", NULL }; + const GDBusArgInfo daemon_notify_in_replaces_id = { -1, "ReplacesId", "u", NULL }; + const GDBusArgInfo daemon_notify_in_app_icon = { -1, "AppIcon", "s", NULL }; + const GDBusArgInfo daemon_notify_in_summary = { -1, "Summary", "s", NULL }; + const GDBusArgInfo daemon_notify_in_body = { -1, "Body", "s", NULL }; + const GDBusArgInfo daemon_notify_in_actions = { -1, "Actions", "as", NULL }; + const GDBusArgInfo daemon_notify_in_hints = { -1, "Hints", "a{sv}", NULL }; + const GDBusArgInfo daemon_notify_in_expire_timeout = { -1, "ExpireTimeout", "i", NULL }; + const GDBusArgInfo *daemon_notify_in_args[] = + { + &daemon_notify_in_app_name, + &daemon_notify_in_replaces_id, + &daemon_notify_in_app_icon, + &daemon_notify_in_summary, + &daemon_notify_in_body, + &daemon_notify_in_actions, + &daemon_notify_in_hints, + &daemon_notify_in_expire_timeout, + NULL + }; + const GDBusArgInfo daemon_notify_out_id = { -1, "Id", "u", NULL }; + const GDBusArgInfo *daemon_notify_out_args[] = { &daemon_notify_out_id, NULL }; + const GDBusMethodInfo daemon_notify_info = { -1, "Notify", (GDBusArgInfo **) daemon_notify_in_args, (GDBusArgInfo **) daemon_notify_out_args, NULL }; + const GDBusMethodInfo *daemon_methods[] = { &daemon_notify_info, NULL }; + const GDBusInterfaceInfo daemon_interface_info = { -1, "org.freedesktop.Notifications", (GDBusMethodInfo **) daemon_methods, NULL, NULL, NULL }; + + GTestDBus *bus = NULL; + GDBusConnection *daemon_connection = NULL; + guint daemon_object_id = 0, daemon_name_id = 0; + const GDBusInterfaceVTable vtable = { daemon_method_call_cb, NULL, NULL, { NULL, } }; + GDBusMethodInvocation *current_method_invocation = NULL; + GApplication *app = NULL; + GNotificationBackend *backend = NULL; + guint32 notify_id; + GError *local_error = NULL; + const GActionEntry entries[] = + { + { "undo", dbus_activate_action_cb, NULL, NULL, NULL, { 0 } }, + { "lang", dbus_activate_action_cb, "s", "'latin'", NULL, { 0 } }, + }; + guint n_activations = 0; + gboolean name_acquired = FALSE; + + g_test_summary ("Test how the backend handles valid and invalid ActionInvoked signals from the daemon"); + + /* Set up a test session bus and connection. */ + bus = g_test_dbus_new (G_TEST_DBUS_NONE); + g_test_dbus_up (bus); + + /* Create a mock org.freedesktop.Notifications daemon */ + daemon_connection = g_dbus_connection_new_for_address_sync (g_test_dbus_get_bus_address (bus), + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT | + G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION, + NULL, NULL, &local_error); + g_assert_no_error (local_error); + + daemon_object_id = g_dbus_connection_register_object (daemon_connection, + "/org/freedesktop/Notifications", + (GDBusInterfaceInfo *) &daemon_interface_info, + &vtable, + ¤t_method_invocation, NULL, &local_error); + g_assert_no_error (local_error); + + daemon_name_id = g_bus_own_name_on_connection (daemon_connection, + "org.freedesktop.Notifications", + G_BUS_NAME_OWNER_FLAGS_DO_NOT_QUEUE, + name_acquired_or_lost_cb, + name_acquired_or_lost_cb, + &name_acquired, NULL); + + while (!name_acquired) + g_main_context_iteration (NULL, TRUE); + + /* Construct our FDO backend under test */ + backend = construct_backend (&app); + g_action_map_add_action_entries (G_ACTION_MAP (app), entries, G_N_ELEMENTS (entries), &n_activations); + + /* Send a notification to ensure that the backend is listening for D-Bus action signals. */ + notify_id = 1233; + assert_send_notification (backend, ¤t_method_invocation, ++notify_id); + + /* Send a valid fake action signal. */ + n_activations = 0; + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "app.undo")); + + while (n_activations == 0) + g_main_context_iteration (NULL, TRUE); + + g_assert_cmpuint (n_activations, ==, 1); + + /* Send a valid fake action signal with a target. We have to create a new + * notification first, as invoking an action on a notification removes it, and + * the GLib implementation of org.freedesktop.Notifications doesn’t currently + * support the `resident` hint to avoid that. */ + assert_send_notification (backend, ¤t_method_invocation, ++notify_id); + n_activations = 0; + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "app.lang::spanish")); + + while (n_activations == 0) + g_main_context_iteration (NULL, TRUE); + + g_assert_cmpuint (n_activations, ==, 1); + + /* Send a series of invalid action signals, followed by one valid one which + * we should be able to detect. */ + assert_send_notification (backend, ¤t_method_invocation, ++notify_id); + n_activations = 0; + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "app.nonexistent")); + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "app.lang(13)")); + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "app.undo::should-have-no-parameter")); + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "app.lang")); + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "undo")); /* no `app.` prefix */ + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "app.lang(")); /* invalid parse format */ + assert_emit_action_invoked (daemon_connection, g_variant_new ("(us)", notify_id, "app.undo")); + + while (n_activations == 0) + g_main_context_iteration (NULL, TRUE); + + g_assert_cmpuint (n_activations, ==, 1); + + /* Shut down. */ + g_assert_null (current_method_invocation); + + g_application_quit (app); + g_clear_object (&app); + g_clear_object (&backend); + + g_dbus_connection_unregister_object (daemon_connection, daemon_object_id); + g_bus_unown_name (daemon_name_id); + + g_dbus_connection_flush_sync (daemon_connection, NULL, &local_error); + g_assert_no_error (local_error); + g_dbus_connection_close_sync (daemon_connection, NULL, &local_error); + g_assert_no_error (local_error); + + g_clear_object (&daemon_connection); + + g_test_dbus_down (bus); + g_clear_object (&bus); +} + int main (int argc, char *argv[]) @@ -98,6 +320,7 @@ main (int argc, g_test_dbus_unset (); g_test_add_func ("/fdo-notification-backend/construction", test_construction); + g_test_add_func ("/fdo-notification-backend/dbus/activate-action", test_dbus_activate_action); return g_test_run (); } From eb0d9e709a644bed2da215ad01f3ca1b34ab159b Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 10 Nov 2022 23:11:03 +0000 Subject: [PATCH 7/9] gapplication: Document that command line options must be validated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They come from an external process, so they must be validated. In particular, it’s always easy to forget to validate the type of a `GVariant`, and just try to get the stored value using a well-known type; but that’s a programming error if the `GVariant` actually stores a different type. Always check the variant type first if loading from a `v`. Signed-off-by: Philip Withnall Helps: #1904 --- gio/gapplication.c | 2 ++ gio/gapplicationcommandline.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/gio/gapplication.c b/gio/gapplication.c index 1d602c763..3708e812c 100644 --- a/gio/gapplication.c +++ b/gio/gapplication.c @@ -673,6 +673,8 @@ add_packed_option (GApplication *application, * inspected and modified. If %G_APPLICATION_HANDLES_COMMAND_LINE is * set, then the resulting dictionary is sent to the primary instance, * where g_application_command_line_get_options_dict() will return it. + * As it has been passed outside the process at this point, the types of all + * values in the options dict must be checked before being used. * This "packing" is done according to the type of the argument -- * booleans for normal flags, strings for strings, bytestrings for * filenames, etc. The packing only occurs if the flag is given (ie: we diff --git a/gio/gapplicationcommandline.c b/gio/gapplicationcommandline.c index b8dc460c9..740ef5601 100644 --- a/gio/gapplicationcommandline.c +++ b/gio/gapplicationcommandline.c @@ -507,6 +507,9 @@ g_application_command_line_get_arguments (GApplicationCommandLine *cmdline, * If no options were sent then an empty dictionary is returned so that * you don't need to check for %NULL. * + * The data has been passed via an untrusted external process, so the types of + * all values must be checked before being used. + * * Returns: (transfer none): a #GVariantDict with the options * * Since: 2.40 From c0eef5e22689cdf8bc66ce0db64309ee01684a83 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 10 Nov 2022 23:12:49 +0000 Subject: [PATCH 8/9] gapplication: Validate types of well-known platform data keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The platform data comes from the parent process, which should normally be considered trusted (if we don’t trust it, it can do all sorts of other things to mess this process up, such as setting `LD_LIBRARY_PATH`). However, it can also come from any process which calls `CommandLine` over D-Bus, so always has to be able to handle untrusted input. In particular, `v`-typed `GVariant`s must always have their dynamic type validated before having values of a static type retrieved from them. Includes unit tests. Signed-off-by: Philip Withnall Helps: #1904 --- gio/gapplication.h | 3 +++ gio/gapplicationcommandline.c | 9 +++++--- gio/tests/gapplication.c | 42 ++++++++++++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/gio/gapplication.h b/gio/gapplication.h index 083ac95df..cb6b90867 100644 --- a/gio/gapplication.h +++ b/gio/gapplication.h @@ -95,8 +95,11 @@ struct _GApplicationClass gchar ***arguments, int *exit_status); + /* @platform_data comes from an external process and is untrusted. All value types + * must be validated before being used. */ void (* before_emit) (GApplication *application, GVariant *platform_data); + /* Same as for @before_emit. */ void (* after_emit) (GApplication *application, GVariant *platform_data); void (* add_platform_data) (GApplication *application, diff --git a/gio/gapplicationcommandline.c b/gio/gapplicationcommandline.c index 740ef5601..e9a6f46c6 100644 --- a/gio/gapplicationcommandline.c +++ b/gio/gapplicationcommandline.c @@ -260,20 +260,20 @@ grok_platform_data (GApplicationCommandLine *cmdline) g_variant_iter_init (&iter, cmdline->priv->platform_data); while (g_variant_iter_loop (&iter, "{&sv}", &key, &value)) - if (strcmp (key, "cwd") == 0) + if (strcmp (key, "cwd") == 0 && g_variant_is_of_type (value, G_VARIANT_TYPE_BYTESTRING)) { if (!cmdline->priv->cwd) cmdline->priv->cwd = g_variant_dup_bytestring (value, NULL); } - else if (strcmp (key, "environ") == 0) + else if (strcmp (key, "environ") == 0 && g_variant_is_of_type (value, G_VARIANT_TYPE_BYTESTRING_ARRAY)) { if (!cmdline->priv->environ) cmdline->priv->environ = g_variant_dup_bytestring_array (value, NULL); } - else if (strcmp (key, "options") == 0) + else if (strcmp (key, "options") == 0 && g_variant_is_of_type (value, G_VARIANT_TYPE_VARDICT)) { if (!cmdline->priv->options) cmdline->priv->options = g_variant_ref (value); @@ -796,6 +796,9 @@ g_application_command_line_get_exit_status (GApplicationCommandLine *cmdline) * information like the current working directory and the startup * notification ID. * + * It comes from an untrusted external process and hence the types of all + * values must be validated before being used. + * * For local invocation, it will be %NULL. * * Returns: (nullable) (transfer full): the platform data, or %NULL diff --git a/gio/tests/gapplication.c b/gio/tests/gapplication.c index 46d4949b4..a1eb85be7 100644 --- a/gio/tests/gapplication.c +++ b/gio/tests/gapplication.c @@ -1447,7 +1447,7 @@ static void test_dbus_command_line (void) { GTestDBus *bus = NULL; - GVariantBuilder builder; + GVariantBuilder builder, builder2; GDBusMessage *message = NULL; GPtrArray *messages = NULL; /* (element-type GDBusMessage) (owned) */ gsize i; @@ -1472,6 +1472,46 @@ test_dbus_command_line (void) &builder, NULL)); g_ptr_array_add (messages, g_steal_pointer (&message)); + /* With platform data */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("aay")); + g_variant_builder_add (&builder, "^ay", "test-program"); + g_variant_builder_add (&builder, "^ay", "--open"); + g_variant_builder_add (&builder, "^ay", "/path/to/something"); + + g_variant_builder_init (&builder2, G_VARIANT_TYPE ("a{sv}")); + g_variant_builder_add (&builder2, "{sv}", "cwd", g_variant_new_bytestring ("/home")); + g_variant_builder_add_parsed (&builder2, "{'environ', <@aay [ b'HOME=/home/bloop', b'PATH=/blah']>}"); + g_variant_builder_add_parsed (&builder2, "{'options', <{'a': <@u 32>, 'b': <'bloop'>}>}"); + + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.CommandLine", + "/org/gtk/TestApplication/CommandLine", + "org.gtk.Application", + "CommandLine"); + g_dbus_message_set_body (message, g_variant_new ("(oaaya{sv})", + "/my/org/gtk/private/CommandLine", + &builder, &builder2)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + + /* With invalid typed platform data */ + g_variant_builder_init (&builder, G_VARIANT_TYPE ("aay")); + g_variant_builder_add (&builder, "^ay", "test-program"); + g_variant_builder_add (&builder, "^ay", "--open"); + g_variant_builder_add (&builder, "^ay", "/path/to/something"); + + g_variant_builder_init (&builder2, G_VARIANT_TYPE ("a{sv}")); + g_variant_builder_add (&builder2, "{sv}", "cwd", g_variant_new_string ("/home should be a bytestring")); + g_variant_builder_add_parsed (&builder2, "{'environ', <['HOME=should be a bytestring', 'PATH=this also']>}"); + g_variant_builder_add_parsed (&builder2, "{'options', <['should be a', 'dict']>}"); + + message = g_dbus_message_new_method_call ("org.gtk.TestApplication.CommandLine", + "/org/gtk/TestApplication/CommandLine", + "org.gtk.Application", + "CommandLine"); + g_dbus_message_set_body (message, g_variant_new ("(oaaya{sv})", + "/my/org/gtk/private/CommandLine", + &builder, &builder2)); + g_ptr_array_add (messages, g_steal_pointer (&message)); + /* Try each message */ bus = g_test_dbus_new (G_TEST_DBUS_NONE); g_test_dbus_up (bus); From 191e89878d3445356cb10e02a8d47b646b134751 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 11 Nov 2022 00:49:20 +0000 Subject: [PATCH 9/9] tests: Add basic GApplicationCommandLine unit tests Signed-off-by: Philip Withnall --- gio/tests/application-command-line.c | 92 ++++++++++++++++++++++++++++ gio/tests/meson.build | 1 + 2 files changed, 93 insertions(+) create mode 100644 gio/tests/application-command-line.c diff --git a/gio/tests/application-command-line.c b/gio/tests/application-command-line.c new file mode 100644 index 000000000..185d49f58 --- /dev/null +++ b/gio/tests/application-command-line.c @@ -0,0 +1,92 @@ +/* + * Copyright © 2022 Endless OS Foundation, LLC + * + * SPDX-License-Identifier: LGPL-2.1-or-later + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General + * Public License along with this library; if not, see . + * + * Author: Philip Withnall + */ + +#include +#include + + +static void +test_basic_properties (void) +{ + GApplicationCommandLine *cl = NULL; + const gchar * const arguments[] = { "arg1", "arg2", "arg3", NULL }; + GVariantBuilder options_builder = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT); + GVariantBuilder platform_data_builder = G_VARIANT_BUILDER_INIT (G_VARIANT_TYPE_VARDICT); + gchar **argv = NULL; + int argc = 0; + GVariantDict *options_dict; + GVariant *platform_data; + GVariantDict *platform_data_dict = NULL; + gboolean is_remote; + + /* Basic construction. */ + g_variant_builder_add (&options_builder, "{sv}", "option1", g_variant_new_string ("value1")); + g_variant_builder_add (&options_builder, "{sv}", "option2", g_variant_new_string ("value2")); + + g_variant_builder_add (&platform_data_builder, "{sv}", "data1", g_variant_new_string ("data-value1")); + g_variant_builder_add (&platform_data_builder, "{sv}", "data2", g_variant_new_string ("data-value2")); + + cl = g_object_new (G_TYPE_APPLICATION_COMMAND_LINE, + "arguments", g_variant_new_bytestring_array (arguments, -1), + "options", g_variant_builder_end (&options_builder), + "platform-data", g_variant_builder_end (&platform_data_builder), + NULL); + g_assert_nonnull (cl); + + /* Check the getters. */ + argv = g_application_command_line_get_arguments (cl, &argc); + g_assert_cmpint (argc, ==, 3); + g_assert_cmpstrv (argv, arguments); + g_clear_pointer (&argv, g_strfreev); + + options_dict = g_application_command_line_get_options_dict (cl); + g_assert_nonnull (options_dict); + g_assert_true (g_variant_dict_contains (options_dict, "option1")); + g_assert_true (g_variant_dict_contains (options_dict, "option2")); + + g_assert_false (g_application_command_line_get_is_remote (cl)); + + platform_data = g_application_command_line_get_platform_data (cl); + g_assert_nonnull (platform_data); + platform_data_dict = g_variant_dict_new (platform_data); + g_assert_true (g_variant_dict_contains (platform_data_dict, "data1")); + g_assert_true (g_variant_dict_contains (platform_data_dict, "data2")); + g_variant_dict_unref (platform_data_dict); + g_variant_unref (platform_data); + + /* And g_object_get(). */ + g_object_get (cl, "is-remote", &is_remote, NULL); + g_assert_false (is_remote); + + g_clear_object (&cl); +} + +int +main (int argc, + char *argv[]) +{ + setlocale (LC_ALL, ""); + g_test_init (&argc, &argv, NULL); + + g_test_add_func ("/application-command-line/basic-properties", test_basic_properties); + + return g_test_run (); +} diff --git a/gio/tests/meson.build b/gio/tests/meson.build index c7fd4ed8e..fc249f14b 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -48,6 +48,7 @@ endif # Test programs buildable on all platforms gio_tests = { + 'application-command-line': {}, 'appmonitor' : { # FIXME: https://gitlab.gnome.org/GNOME/glib/-/issues/1392 'can_fail' : host_system == 'darwin',