CVE-2021-21261: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: Flatpak is a system for building, distributing, and running sandboxed desktop applications on Linux. A bug was discovered in the `flatpak-portal` service that can allow sandboxed applications to execute arbitrary code on the host system (a sandbox escape). This sandbox-escape bug is present in versions from 0.11.4 and before fixed versions 1.8.5 and 1.10.0. The Flatpak portal D-Bus service (`flatpak-portal`, also known by its D-Bus service name `org.freedesktop.portal.Flatpak`) allows apps in a Flatpak sandbox to launch their own subprocesses in a new sandbox instance, either with the same security settings as the caller or with more restrictive security settings. For example, this is used in Flatpak-packaged web browsers such as Chromium to launch subprocesses that will process untrusted web content, and give those subprocesses a more restrictive sandbox than the browser itself. In vulnerable versions, the Flatpak portal service passes caller-specified environment variables to non-sandboxed processes on the host system, and in particular to the `flatpak run` command that is used to launch the new sandbox instance. A malicious or compromised Flatpak app could set environment variables that are trusted by the `flatpak run` command, and use them to execute arbitrary code that is not in a sandbox. As a workaround, this vulnerability can be mitigated by preventing the `flatpak-portal` service from starting, but that mitigation will prevent many Flatpak apps from working correctly. This is fixed in versions 1.8.5 and 1.10.0.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 8.8
Published: 2021-01-14T20:15Z
Repository: https://github.com/flatpak/flatpak
Language: C
Files changed: portal/flatpak-portal.c
Commit date: 2021-01-14 09:33:24 +0100
Commit message: 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 <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2

Diff: diff --git a/portal/flatpak-portal.c b/portal/flatpak-portal.c
index 88eb02b3..28ce22d5 100644
--- a/portal/flatpak-portal.c
+++ b/portal/flatpak-portal.c
@@ -996,18 +996,44 @@ handle_spawn (PortalFlatpak         *object,
       env = g_strdupv (empty);
     }
   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++)
     {
       const char *var = NULL;
       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"));
   g_ptr_array_add (flatpak_argv, g_strdup ("run"));
 

Vulnerable code:       env = g_environ_setenv (env, var, val, TRUE);
Fixed code:   /* 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. */
      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');