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: Convert --env in extra-args into --env-fd

This hides overridden variables from the command-line, which means
processes running under other uids can't see them in /proc/*/cmdline,
which might be important if they contain secrets.

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 7ae581ec..88eb02b3 100644
--- a/portal/flatpak-portal.c
+++ b/portal/flatpak-portal.c
@@ -255,10 +255,11 @@ typedef struct
   FdMapEntry *fd_map;
   int         fd_map_len;
   int         instance_id_fd;
   gboolean    set_tty;
   int         tty;
+  int         env_fd;
 } ChildSetupData;
 
 typedef struct
 {
   guint pid;
@@ -483,10 +484,13 @@ child_setup_func (gpointer user_data)
   flatpak_close_fds_workaround (3);
 
   if (data->instance_id_fd != -1)
     drop_cloexec (data->instance_id_fd);
 
+  if (data->env_fd != -1)
+    drop_cloexec (data->env_fd);
+
   /* Unblock all signals */
   sigemptyset (&set);
   if (pthread_sigmask (SIG_SETMASK, &set, NULL) == -1)
     {
       g_warning ("Failed to unblock signals when starting child");
@@ -780,12 +784,14 @@ handle_spawn (PortalFlatpak         *object,
   gboolean sandboxed;
   gboolean expose_pids;
   gboolean share_pids;
   gboolean notify_start;
   gboolean devel;
+  g_autoptr(GString) env_string = g_string_new ("");
 
   child_setup_data.instance_id_fd = -1;
+  child_setup_data.env_fd = -1;
 
   if (fd_list != NULL)
     fds = g_unix_fd_list_peek_fds (fd_list, &fds_len);
 
   app_info = g_object_get_data (G_OBJECT (invocation), "app-info");
@@ -1042,11 +1048,53 @@ handle_spawn (PortalFlatpak         *object,
         g_ptr_array_add (flatpak_argv, g_strdup ("--a11y-bus"));
     }
   else
     {
       for (i = 0; extra_args != NULL && extra_args[i] != NULL; i++)
-        g_ptr_array_add (flatpak_argv, g_strdup (extra_args[i]));
+        {
+          if (g_str_has_prefix (extra_args[i], "--env="))
+            {
+              const char *var_val = extra_args[i] + strlen ("--env=");
+
+              if (var_val[0] == '\0' || var_val[0] == '=')
+                {
+                  g_warning ("Environment variable in extra-args has empty name");
+                  continue;
+                }
+
+              if (strchr (var_val, '=') == NULL)
+                {
+                  g_warning ("Environment variable in extra-args has no value");
+                  continue;
+                }
+
+              g_string_append (env_string, var_val);
+              g_string_append_c (env_string, '\0');
+            }
+          else
+            {
+              g_ptr_array_add (flatpak_argv, g_strdup (extra_args[i]));
+            }
+        }
+    }
+
+  if (env_string->len > 0)
+    {
+      g_auto(GLnxTmpfile) env_tmpf  = { 0, };
+
+      if (!flatpak_buffer_to_sealed_memfd_or_tmpfile (&env_tmpf, "environ",
+                                                      env_string->str,
+                                                      env_string->len, &error))
+        {
+          g_dbus_method_invocation_return_gerror (invocation, error);
+          return G_DBUS_METHOD_INVOCATION_HANDLED;
+        }
+
+      child_setup_data.env_fd = glnx_steal_fd (&env_tmpf.fd);
+      g_ptr_array_add (flatpak_argv,
+                       g_strdup_printf ("--env-fd=%d",
+                                        child_setup_data.env_fd));
     }
 
   expose_pids = (arg_flags & FLATPAK_SPAWN_FLAGS_EXPOSE_PIDS) != 0;
   share_pids = (arg_flags & FLATPAK_SPAWN_FLAGS_SHARE_PIDS) != 0;
 

Vulnerable code:         g_ptr_array_add (flatpak_argv, g_strdup (extra_args[i]));
Fixed code:   int         env_fd;
  if (data->env_fd != -1)
    drop_cloexec (data->env_fd);

  g_autoptr(GString) env_string = g_string_new ("");
  child_setup_data.env_fd = -1;
        {
          if (g_str_has_prefix (extra_args[i], "--env="))
            {
              const char *var_val = extra_args[i] + strlen ("--env=");

              if (var_val[0] == '\0' || var_val[0] == '=')
                {
                  g_warning ("Environment variable in extra-args has empty name");
                  continue;
                }

              if (strchr (var_val, '=') == NULL)
                {
                  g_warning ("Environment variable in extra-args has no value");
                  continue;
                }

              g_string_append (env_string, var_val);
              g_string_append_c (env_string, '\0');
            }
          else
            {
              g_ptr_array_add (flatpak_argv, g_strdup (extra_args[i]));
            }
        }
    }

  if (env_string->len > 0)
    {
      g_auto(GLnxTmpfile) env_tmpf  = { 0, };

      if (!flatpak_buffer_to_sealed_memfd_or_tmpfile (&env_tmpf, "environ",
                                                      env_string->str,
                                                      env_string->len, &error))
        {
          g_dbus_method_invocation_return_gerror (invocation, error);
          return G_DBUS_METHOD_INVOCATION_HANDLED;
        }

      child_setup_data.env_fd = glnx_steal_fd (&env_tmpf.fd);
      g_ptr_array_add (flatpak_argv,
                       g_strdup_printf ("--env-fd=%d",
                                        child_setup_data.env_fd));