From ce6a6e593310d42f7aecddb88d98e759f22ab6db Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 12 Jan 2021 12:25:59 +0000
Subject: [PATCH] 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
---
 portal/Makefile.am.inc  |  4 ++-
 portal/flatpak-portal.c | 65 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 67 insertions(+), 2 deletions(-)

--- flatpak.orig/portal/Makefile.am.inc
+++ flatpak/portal/Makefile.am.inc
@@ -29,11 +29,13 @@ flatpak_portal_SOURCES = \
 	portal/flatpak-portal-app-info.h		\
 	common/flatpak-portal-error.c	\
 	common/flatpak-portal-error.h	\
+	common/flatpak-utils-memfd.c \
+	common/flatpak-utils-memfd-private.h \
 	$(NULL)
 
 BUILT_SOURCES += $(nodist_flatpak_portal_SOURCES)
 CLEANFILES += $(nodist_flatpak_portal_SOURCES)
 
-flatpak_portal_LDADD = $(AM_LDADD) $(BASE_LIBS)
+flatpak_portal_LDADD = $(AM_LDADD) $(BASE_LIBS) libglnx.la
 flatpak_portal_CFLAGS = $(AM_CFLAGS) $(BASE_CFLAGS) -DFLATPAK_COMPILATION
 flatpak_portal_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/portal
--- flatpak.orig/portal/flatpak-portal.c
+++ flatpak/portal/flatpak-portal.c
@@ -32,6 +32,14 @@
 #include "flatpak-portal.h"
 #include "flatpak-portal-app-info.h"
 #include "flatpak-portal-error.h"
+#include "flatpak-utils-memfd-private.h"
+
+/* Syntactic sugar added in newer GLib, which makes the error paths more
+ * clearly correct */
+#ifndef G_DBUS_METHOD_INVOCATION_HANDLED
+# define G_DBUS_METHOD_INVOCATION_HANDLED TRUE
+# define G_DBUS_METHOD_INVOCATION_UNHANDLED FALSE
+#endif
 
 #define IDLE_TIMEOUT_SECS 10 * 60
 
@@ -166,9 +174,16 @@ typedef struct
   int         fd_map_len;
   gboolean    set_tty;
   int         tty;
+  int         env_fd;
 } ChildSetupData;
 
 static void
+drop_cloexec (int fd)
+{
+  fcntl (fd, F_SETFD, 0);
+}
+
+static void
 child_setup_func (gpointer user_data)
 {
   ChildSetupData *data = (ChildSetupData *) user_data;
@@ -176,6 +191,9 @@ child_setup_func (gpointer user_data)
   sigset_t set;
   int i;
 
+  if (data->env_fd != -1)
+    drop_cloexec (data->env_fd);
+
   /* Unblock all signals */
   sigemptyset (&set);
   if (pthread_sigmask (SIG_SETMASK, &set, NULL) == -1)
@@ -321,6 +339,9 @@ handle_spawn (PortalFlatpak         *obj
   g_auto(GStrv) sandbox_expose = NULL;
   g_auto(GStrv) sandbox_expose_ro = NULL;
   gboolean sandboxed;
+  g_autoptr(GString) env_string = g_string_new ("");
+
+  child_setup_data.env_fd = -1;
 
   app_info = g_object_get_data (G_OBJECT (invocation), "app-info");
   g_assert (app_info != NULL);
@@ -505,7 +526,49 @@ handle_spawn (PortalFlatpak         *obj
   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));
     }
 
   /* Inherit launcher network access from launcher, unless