From f527eaeb132dcd5bb06115b987d6a6f8bfafac9b Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 10 Jan 2021 16:25:29 +0000 Subject: [PATCH] portal: Do not use caller-supplied variables in environment If the caller specifies a variable that can be used to inject arbitrary code into processes, we must not allow it to enter the environment block used to run `flatpak run`, which runs unsandboxed. This change requires the previous commit "context: Add --env-fd option", which adds infrastructure used here. To be secure, this change also requires the previous commit "run: Convert all environment variables into bwrap arguments", which protects a non-setuid bwrap(1) from the same attack. Signed-off-by: Simon McVittie Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2 --- portal/flatpak-portal.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) --- flatpak.orig/portal/flatpak-portal.c +++ flatpak/portal/flatpak-portal.c @@ -506,6 +506,13 @@ handle_spawn (PortalFlatpak *obj else env = g_get_environ (); + /* Let the environment variables given by the caller override the ones + * from extra_args. Don't add them to @env, because they are controlled + * by our caller, which might be trying to use them to inject code into + * flatpak(1); add them to the environment block instead. + * + * We don't use --env= here, so that if the values are something that + * should not be exposed to other uids, they can remain confidential. */ n_envs = g_variant_n_children (arg_envs); for (i = 0; i < n_envs; i++) { @@ -513,7 +520,26 @@ handle_spawn (PortalFlatpak *obj const char *val = NULL; g_variant_get_child (arg_envs, i, "{&s&s}", &var, &val); - env = g_environ_setenv (env, var, val, TRUE); + if (var[0] == '\0') + { + g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR, + G_DBUS_ERROR_INVALID_ARGS, + "Environment variable cannot have empty name"); + return G_DBUS_METHOD_INVOCATION_HANDLED; + } + + if (strchr (var, '=') != NULL) + { + g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR, + G_DBUS_ERROR_INVALID_ARGS, + "Environment variable name cannot contain '='"); + return G_DBUS_METHOD_INVOCATION_HANDLED; + } + + g_string_append (env_string, var); + g_string_append_c (env_string, '='); + g_string_append (env_string, val); + g_string_append_c (env_string, '\0'); } g_ptr_array_add (flatpak_argv, g_strdup ("flatpak"));