summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhilip Withnall <pwithnall@endlessos.org>2022-11-10 23:12:49 +0000
committerPhilip Withnall <pwithnall@endlessos.org>2022-12-08 14:46:47 +0000
commita6a847ababd22e57846f0edbcf957211bd2fb796 (patch)
tree71c608b1113c20dec7057a45b18a07cd199c7fa8
parent04b685ce27153393acc5538e8f038dc581ed92bd (diff)
downloadglib-a6a847ababd22e57846f0edbcf957211bd2fb796.tar.gz
gapplication: Validate types of well-known platform data keys
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 <pwithnall@endlessos.org> Helps: #1904
-rw-r--r--gio/gapplication.h3
-rw-r--r--gio/gapplicationcommandline.c9
-rw-r--r--gio/tests/gapplication.c42
3 files changed, 50 insertions, 4 deletions
diff --git a/gio/gapplication.h b/gio/gapplication.h
index 345405366..70074349a 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 fbe634a47..ef6c2d2a3 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): 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);