From: Simon McVittie <smcv@collabora.com>
Date: Tue, 12 Jan 2021 12:21:31 +0000
Subject: run: Convert all environment variables into bwrap arguments

This avoids some of them being filtered out by a setuid bwrap. It also
means that if they came from an untrusted source, they cannot be used
to inject arbitrary code into a non-setuid bwrap via mechanisms like
LD_PRELOAD.

Because they get bundled into a memfd or temporary file, they do not
actually appear in argv, ensuring that they remain inaccessible to
processes running under a different uid (which is important if their
values are tokens or other secrets).

[Backported to 1.2.x for Debian 10 security update.]

Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
---
 common/flatpak-bwrap-private.h |  3 +++
 common/flatpak-bwrap.c         | 43 ++++++++++++++++++++++++++++++++++++++++++
 common/flatpak-run.c           | 24 ++++++++++++++---------
 3 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/common/flatpak-bwrap-private.h b/common/flatpak-bwrap-private.h
index 2a633d3..b6f3df9 100644
--- a/common/flatpak-bwrap-private.h
+++ b/common/flatpak-bwrap-private.h
@@ -43,6 +43,8 @@ void          flatpak_bwrap_unset_env (FlatpakBwrap *bwrap,
                                        const char   *variable);
 void          flatpak_bwrap_add_arg (FlatpakBwrap *bwrap,
                                      const char   *arg);
+void          flatpak_bwrap_take_arg (FlatpakBwrap *bwrap,
+                                      char         *arg);
 void          flatpak_bwrap_add_noinherit_fd (FlatpakBwrap *bwrap,
                                               int           fd);
 void          flatpak_bwrap_add_fd (FlatpakBwrap *bwrap,
@@ -73,6 +75,7 @@ void          flatpak_bwrap_add_bind_arg (FlatpakBwrap *bwrap,
                                           const char   *type,
                                           const char   *src,
                                           const char   *dest);
+void          flatpak_bwrap_envp_to_args (FlatpakBwrap *bwrap);
 gboolean      flatpak_bwrap_bundle_args (FlatpakBwrap *bwrap,
                                          int           start,
                                          int           end,
diff --git a/common/flatpak-bwrap.c b/common/flatpak-bwrap.c
index f419d23..57cd750 100644
--- a/common/flatpak-bwrap.c
+++ b/common/flatpak-bwrap.c
@@ -108,6 +108,18 @@ flatpak_bwrap_add_arg (FlatpakBwrap *bwrap, const char *arg)
   g_ptr_array_add (bwrap->argv, g_strdup (arg));
 }
 
+/*
+ * flatpak_bwrap_take_arg:
+ * @arg: (transfer full): Take ownership of this argument
+ *
+ * Add @arg to @bwrap's argv, taking ownership of the pointer.
+ */
+void
+flatpak_bwrap_take_arg (FlatpakBwrap *bwrap, char *arg)
+{
+  g_ptr_array_add (bwrap->argv, arg);
+}
+
 void
 flatpak_bwrap_finish (FlatpakBwrap *bwrap)
 {
@@ -273,6 +285,37 @@ flatpak_bwrap_add_bind_arg (FlatpakBwrap *bwrap,
     }
 }
 
+/*
+ * Convert bwrap->envp into a series of --setenv arguments for bwrap(1),
+ * assumed to be applied to an empty environment. Reset envp to be an
+ * empty environment.
+ */
+void
+flatpak_bwrap_envp_to_args (FlatpakBwrap *bwrap)
+{
+  gsize i;
+
+  for (i = 0; bwrap->envp[i] != NULL; i++)
+    {
+      char *key_val = bwrap->envp[i];
+      char *eq = strchr (key_val, '=');
+
+      if (eq)
+        {
+          flatpak_bwrap_add_arg (bwrap, "--setenv");
+          flatpak_bwrap_take_arg (bwrap, g_strndup (key_val, eq - key_val));
+          flatpak_bwrap_add_arg (bwrap, eq + 1);
+        }
+      else
+        {
+          g_warn_if_reached ();
+        }
+    }
+
+  g_strfreev (g_steal_pointer (&bwrap->envp));
+  bwrap->envp = g_strdupv (flatpak_bwrap_empty_env);
+}
+
 gboolean
 flatpak_bwrap_bundle_args (FlatpakBwrap *bwrap,
                            int           start,
diff --git a/common/flatpak-run.c b/common/flatpak-run.c
index 68719ec..1fa2d43 100644
--- a/common/flatpak-run.c
+++ b/common/flatpak-run.c
@@ -1140,15 +1140,6 @@ flatpak_run_add_environment_args (FlatpakBwrap    *bwrap,
   flatpak_run_add_system_dbus_args (bwrap, proxy_arg_bwrap, context, flags);
   flatpak_run_add_a11y_dbus_args (bwrap, proxy_arg_bwrap, context, flags);
 
-  if (g_environ_getenv (bwrap->envp, "LD_LIBRARY_PATH") != NULL)
-    {
-      /* LD_LIBRARY_PATH is overridden for setuid helper, so pass it as cmdline arg */
-      flatpak_bwrap_add_args (bwrap,
-                              "--setenv", "LD_LIBRARY_PATH", g_environ_getenv (bwrap->envp, "LD_LIBRARY_PATH"),
-                              NULL);
-      flatpak_bwrap_unset_env (bwrap, "LD_LIBRARY_PATH");
-    }
-
   /* Must run this before spawning the dbus proxy, to ensure it
      ends up in the app cgroup */
   if (!flatpak_run_in_transient_unit (app_id, &my_error))
@@ -3376,6 +3367,8 @@ flatpak_run_app (const char     *app_ref,
       command = default_command;
     }
 
+  flatpak_bwrap_envp_to_args (bwrap);
+
   if (!flatpak_bwrap_bundle_args (bwrap, 1, -1, FALSE, error))
     return FALSE;
 
@@ -3403,6 +3396,12 @@ flatpak_run_app (const char     *app_ref,
       if (flags & FLATPAK_RUN_FLAG_DO_NOT_REAP)
         spawn_flags |= G_SPAWN_DO_NOT_REAP_CHILD;
 
+      /* flatpak_bwrap_envp_to_args() moved the environment variables to
+       * be set into --setenv instructions in argv, so the environment
+       * in which the bwrap command runs must be empty. */
+      g_assert (bwrap->envp != NULL);
+      g_assert (bwrap->envp[0] == NULL);
+
       if (!g_spawn_async (NULL,
                           (char **) bwrap->argv->pdata,
                           bwrap->envp,
@@ -3427,6 +3426,13 @@ flatpak_run_app (const char     *app_ref,
 
       /* Ensure we unset O_CLOEXEC */
       flatpak_bwrap_child_setup_cb (bwrap->fds);
+
+      /* flatpak_bwrap_envp_to_args() moved the environment variables to
+       * be set into --setenv instructions in argv, so the environment
+       * in which the bwrap command runs must be empty. */
+      g_assert (bwrap->envp != NULL);
+      g_assert (bwrap->envp[0] == NULL);
+
       if (execvpe (flatpak_get_bwrap (), (char **) bwrap->argv->pdata, bwrap->envp) == -1)
         {
           g_set_error_literal (error, G_IO_ERROR, g_io_error_from_errno (errno),
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 12 Jan 2021 12:32:58 +0000
Subject: tests: Add minimal version of "ok" helper

This makes it possible to cherry-pick improved test coverage from newer
branches without having to edit it.

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 tests/libtest.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/libtest.sh b/tests/libtest.sh
index 4a5e699..87e8944 100644
--- a/tests/libtest.sh
+++ b/tests/libtest.sh
@@ -34,6 +34,10 @@ assert_not_reached () {
     echo $@ 1>&2; exit 1
 }
 
+ok () {
+    echo "ok $@"
+}
+
 test_tmpdir=$(pwd)
 
 # Sanity check that we're in a tmpdir that has
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 11 Jan 2021 12:14:48 +0000
Subject: tests: Expand coverage for environment variable overrides

This checks that `flatpak run --env=` takes precedence over
`flatpak override --env=`, and that environment variables don't get
onto the bwrap command-line (which would be information disclosure
if their values are secret).

Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
---
 tests/test-override.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/tests/test-override.sh b/tests/test-override.sh
index 2a0aab2..2192289 100755
--- a/tests/test-override.sh
+++ b/tests/test-override.sh
@@ -10,7 +10,7 @@ reset_overrides () {
     assert_file_empty info
 }
 
-echo "1..13"
+echo "1..15"
 
 setup_repo
 install_repo
@@ -63,14 +63,80 @@ reset_overrides
 
 ${FLATPAK} override --user --env=FOO=BAR org.test.Hello
 ${FLATPAK} override --user --env=BAR= org.test.Hello
+# TODO: A future commit will add a way to avoid this ever being present in argv
+${FLATPAK} override --user --env=SECRET_TOKEN=3047225e-5e38-4357-b21c-eac83b7e8ea6 org.test.Hello
+# TMPDIR and TZDIR are filtered out by ld.so for setuid processes,
+# so setting these gives us a way to verify that we can pass them through
+# a setuid bwrap (without special-casing them, as we previously did for
+# TMPDIR).
+${FLATPAK} override --user --env=TMPDIR=/nonexistent/tmp org.test.Hello
+${FLATPAK} override --user --env=TZDIR=/nonexistent/tz org.test.Hello
 ${FLATPAK} override --user --show org.test.Hello > override
 
 assert_file_has_content override "^\[Environment\]$"
 assert_file_has_content override "^FOO=BAR$"
 assert_file_has_content override "^BAR=$"
+assert_file_has_content override "^SECRET_TOKEN=3047225e-5e38-4357-b21c-eac83b7e8ea6$"
+assert_file_has_content override "^TMPDIR=/nonexistent/tmp$"
+assert_file_has_content override "^TZDIR=/nonexistent/tz$"
 
 echo "ok override --env"
 
+if skip_one_without_bwrap "sandbox environment variables"; then
+  :
+else
+  ${FLATPAK} run --command=bash org.test.Hello \
+      -c 'echo "FOO=$FOO"; echo "BAR=$BAR"; echo "SECRET_TOKEN=$SECRET_TOKEN"; echo "TMPDIR=$TMPDIR"; echo "TZDIR=$TZDIR"' > out
+  assert_file_has_content out '^FOO=BAR$'
+  assert_file_has_content out '^BAR=$'
+  assert_file_has_content out '^SECRET_TOKEN=3047225e-5e38-4357-b21c-eac83b7e8ea6$'
+  # The variables that would be filtered out by a setuid bwrap get set
+  assert_file_has_content out '^TZDIR=/nonexistent/tz$'
+  assert_file_has_content out '^TMPDIR=/nonexistent/tmp$'
+  ${FLATPAK} run --command=cat org.test.Hello -- /proc/1/cmdline > out
+  # The secret doesn't end up in bubblewrap's cmdline where other users
+  # could see it
+  assert_not_file_has_content out 3047225e-5e38-4357-b21c-eac83b7e8ea6
+
+  ok "sandbox environment variables"
+fi
+
+reset_overrides
+
+if skip_one_without_bwrap "temporary environment variables"; then
+  :
+else
+  ${FLATPAK} override --user --env=FOO=wrong org.test.Hello
+  ${FLATPAK} override --user --env=BAR=wrong org.test.Hello
+  ${FLATPAK} override --user --env=SECRET_TOKEN=wrong org.test.Hello
+  ${FLATPAK} override --user --env=TMPDIR=/nonexistent/wrong org.test.Hello
+  ${FLATPAK} override --user --env=TZDIR=/nonexistent/wrong org.test.Hello
+  ${FLATPAK} override --user --show org.test.Hello > override
+
+  ${FLATPAK} run --command=bash \
+      --env=FOO=BAR \
+      --env=BAR= \
+      --env=SECRET_TOKEN=3047225e-5e38-4357-b21c-eac83b7e8ea6 \
+      --env=TMPDIR=/nonexistent/tmp \
+      --env=TZDIR=/nonexistent/tz \
+      org.test.Hello \
+      -c 'echo "FOO=$FOO"; echo "BAR=$BAR"; echo "SECRET_TOKEN=$SECRET_TOKEN"; echo "TMPDIR=$TMPDIR"; echo "TZDIR=$TZDIR"' > out
+  # The versions from `flatpak run` overrule `flatpak override`
+  assert_file_has_content out '^FOO=BAR$'
+  assert_file_has_content out '^BAR=$'
+  assert_file_has_content out '^SECRET_TOKEN=3047225e-5e38-4357-b21c-eac83b7e8ea6$'
+  assert_file_has_content out '^TZDIR=/nonexistent/tz$'
+  assert_file_has_content out '^TMPDIR=/nonexistent/tmp$'
+  ${FLATPAK} run --command=cat org.test.Hello -- /proc/1/cmdline > out
+  # The secret doesn't end up in bubblewrap's cmdline where other users
+  # could see it
+  assert_not_file_has_content out 3047225e-5e38-4357-b21c-eac83b7e8ea6
+
+  ok "temporary environment variables"
+fi
+
+reset_overrides
+
 ${FLATPAK} override --user --filesystem=home org.test.Hello
 ${FLATPAK} override --user --filesystem=xdg-desktop/foo:create org.test.Hello
 ${FLATPAK} override --user --filesystem=xdg-config:ro org.test.Hello
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 11 Jan 2021 14:51:36 +0000
Subject: common: Move flatpak_buffer_to_sealed_memfd_or_tmpfile to its own
 file

We'll need this to use it in flatpak-portal without pulling the rest of
the common/ directory.

[Part of a 1.2.x backport of GHSA-4ppf-fxf6-vxg2 for Debian 10.]

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 common/Makefile.am.inc               |  2 +
 common/flatpak-utils-memfd-private.h | 32 +++++++++++++
 common/flatpak-utils-memfd.c         | 90 ++++++++++++++++++++++++++++++++++++
 common/flatpak-utils-private.h       |  1 +
 common/flatpak-utils.c               | 50 --------------------
 5 files changed, 125 insertions(+), 50 deletions(-)
 create mode 100644 common/flatpak-utils-memfd-private.h
 create mode 100644 common/flatpak-utils-memfd.c

diff --git a/common/Makefile.am.inc b/common/Makefile.am.inc
index 98d51b5..b5a5c5e 100644
--- a/common/Makefile.am.inc
+++ b/common/Makefile.am.inc
@@ -99,6 +99,8 @@ libflatpak_common_la_SOURCES = \
 	common/flatpak-utils.c \
 	common/flatpak-utils-http.c \
 	common/flatpak-utils-http-private.h \
+	common/flatpak-utils-memfd.c \
+	common/flatpak-utils-memfd-private.h \
 	common/flatpak-utils-private.h \
 	common/flatpak-chain-input-stream.c \
 	common/flatpak-chain-input-stream-private.h \
diff --git a/common/flatpak-utils-memfd-private.h b/common/flatpak-utils-memfd-private.h
new file mode 100644
index 0000000..c0e985f
--- /dev/null
+++ b/common/flatpak-utils-memfd-private.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright © 2014 Red Hat, Inc
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *       Alexander Larsson <alexl@redhat.com>
+ */
+
+#ifndef __FLATPAK_UTILS_MEMFD_H__
+#define __FLATPAK_UTILS_MEMFD_H__
+
+#include "libglnx/libglnx.h"
+
+gboolean flatpak_buffer_to_sealed_memfd_or_tmpfile (GLnxTmpfile *tmpf,
+                                                    const char  *name,
+                                                    const char  *str,
+                                                    size_t       len,
+                                                    GError     **error);
+
+#endif /* __FLATPAK_UTILS_MEMFD_H__ */
diff --git a/common/flatpak-utils-memfd.c b/common/flatpak-utils-memfd.c
new file mode 100644
index 0000000..9a0730f
--- /dev/null
+++ b/common/flatpak-utils-memfd.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright © 2014 Red Hat, Inc
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *       Alexander Larsson <alexl@redhat.com>
+ */
+
+#include "config.h"
+
+#include "flatpak-utils-memfd-private.h"
+
+#include "valgrind-private.h"
+
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/file.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/utsname.h>
+#include <sys/ioctl.h>
+#include <termios.h>
+
+/* If memfd_create() is available, generate a sealed memfd with contents of
+ * @str. Otherwise use an O_TMPFILE @tmpf in anonymous mode, write @str to
+ * @tmpf, and lseek() back to the start. See also similar uses in e.g.
+ * rpm-ostree for running dracut.
+ */
+gboolean
+flatpak_buffer_to_sealed_memfd_or_tmpfile (GLnxTmpfile *tmpf,
+                                           const char  *name,
+                                           const char  *str,
+                                           size_t       len,
+                                           GError     **error)
+{
+  if (len == -1)
+    len = strlen (str);
+  glnx_autofd int memfd = memfd_create (name, MFD_CLOEXEC | MFD_ALLOW_SEALING);
+  int fd; /* Unowned */
+  if (memfd != -1)
+    {
+      fd = memfd;
+    }
+  else
+    {
+      /* We use an anonymous fd (i.e. O_EXCL) since we don't want
+       * the target container to potentially be able to re-link it.
+       */
+      if (!G_IN_SET (errno, ENOSYS, EOPNOTSUPP))
+        return glnx_throw_errno_prefix (error, "memfd_create");
+      if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, tmpf, error))
+        return FALSE;
+      fd = tmpf->fd;
+    }
+  if (ftruncate (fd, len) < 0)
+    return glnx_throw_errno_prefix (error, "ftruncate");
+  if (glnx_loop_write (fd, str, len) < 0)
+    return glnx_throw_errno_prefix (error, "write");
+  if (lseek (fd, 0, SEEK_SET) < 0)
+    return glnx_throw_errno_prefix (error, "lseek");
+  if (memfd != -1)
+    {
+      /* Valgrind doesn't currently handle G_ADD_SEALS, so lets not seal when debugging... */
+      if ((!RUNNING_ON_VALGRIND) &&
+          fcntl (memfd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE | F_SEAL_SEAL) < 0)
+        return glnx_throw_errno_prefix (error, "fcntl(F_ADD_SEALS)");
+      /* The other values can stay default */
+      tmpf->fd = glnx_steal_fd (&memfd);
+      tmpf->initialized = TRUE;
+    }
+  return TRUE;
+}
diff --git a/common/flatpak-utils-private.h b/common/flatpak-utils-private.h
index 6d6d467..fb970c8 100644
--- a/common/flatpak-utils-private.h
+++ b/common/flatpak-utils-private.h
@@ -32,6 +32,7 @@
 #include "flatpak-context-private.h"
 #include "flatpak-error.h"
 #include "flatpak-utils-http-private.h"
+#include "flatpak-utils-memfd-private.h"
 #include <ostree.h>
 #include <json-glib/json-glib.h>
 
diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c
index 4af7586..c6604b0 100644
--- a/common/flatpak-utils.c
+++ b/common/flatpak-utils.c
@@ -2281,56 +2281,6 @@ flatpak_file_rename (GFile        *from,
   return TRUE;
 }
 
-/* If memfd_create() is available, generate a sealed memfd with contents of
- * @str. Otherwise use an O_TMPFILE @tmpf in anonymous mode, write @str to
- * @tmpf, and lseek() back to the start. See also similar uses in e.g.
- * rpm-ostree for running dracut.
- */
-gboolean
-flatpak_buffer_to_sealed_memfd_or_tmpfile (GLnxTmpfile *tmpf,
-                                           const char  *name,
-                                           const char  *str,
-                                           size_t       len,
-                                           GError     **error)
-{
-  if (len == -1)
-    len = strlen (str);
-  glnx_autofd int memfd = memfd_create (name, MFD_CLOEXEC | MFD_ALLOW_SEALING);
-  int fd; /* Unowned */
-  if (memfd != -1)
-    {
-      fd = memfd;
-    }
-  else
-    {
-      /* We use an anonymous fd (i.e. O_EXCL) since we don't want
-       * the target container to potentially be able to re-link it.
-       */
-      if (!G_IN_SET (errno, ENOSYS, EOPNOTSUPP))
-        return glnx_throw_errno_prefix (error, "memfd_create");
-      if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, tmpf, error))
-        return FALSE;
-      fd = tmpf->fd;
-    }
-  if (ftruncate (fd, len) < 0)
-    return glnx_throw_errno_prefix (error, "ftruncate");
-  if (glnx_loop_write (fd, str, len) < 0)
-    return glnx_throw_errno_prefix (error, "write");
-  if (lseek (fd, 0, SEEK_SET) < 0)
-    return glnx_throw_errno_prefix (error, "lseek");
-  if (memfd != -1)
-    {
-      /* Valgrind doesn't currently handle G_ADD_SEALS, so lets not seal when debugging... */
-      if ((!RUNNING_ON_VALGRIND) &&
-          fcntl (memfd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE | F_SEAL_SEAL) < 0)
-        return glnx_throw_errno_prefix (error, "fcntl(F_ADD_SEALS)");
-      /* The other values can stay default */
-      tmpf->fd = glnx_steal_fd (&memfd);
-      tmpf->initialized = TRUE;
-    }
-  return TRUE;
-}
-
 gboolean
 flatpak_open_in_tmpdir_at (int             tmpdir_fd,
                            int             mode,
From: Simon McVittie <smcv@collabora.com>
Date: Sun, 10 Jan 2021 16:18:58 +0000
Subject: context: Add --env-fd option

This allows environment variables to be added to the context without
making their values visible to processes running under a different uid,
which might be significant if the variable's value is a token or some
other secret value.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
---
 common/flatpak-context.c     | 60 ++++++++++++++++++++++++++++++++++++++++++++
 doc/flatpak-build-finish.xml | 18 +++++++++++++
 doc/flatpak-build.xml        | 18 +++++++++++++
 doc/flatpak-override.xml     | 18 +++++++++++++
 doc/flatpak-run.xml          | 18 +++++++++++++
 5 files changed, 132 insertions(+)

diff --git a/common/flatpak-context.c b/common/flatpak-context.c
index 6e4d564..36fb177 100644
--- a/common/flatpak-context.c
+++ b/common/flatpak-context.c
@@ -1039,6 +1039,65 @@ option_env_cb (const gchar *option_name,
   return TRUE;
 }
 
+static gboolean
+option_env_fd_cb (const gchar *option_name,
+                  const gchar *value,
+                  gpointer     data,
+                  GError     **error)
+{
+  FlatpakContext *context = data;
+  g_autoptr(GBytes) env_block = NULL;
+  gsize remaining;
+  const char *p;
+  guint64 fd;
+  gchar *endptr;
+
+  fd = g_ascii_strtoull (value, &endptr, 10);
+
+  if (endptr == NULL || *endptr != '\0' || fd > G_MAXINT)
+    return glnx_throw (error, "Not a valid file descriptor: %s", value);
+
+  env_block = glnx_fd_readall_bytes ((int) fd, NULL, error);
+
+  if (env_block == NULL)
+    return FALSE;
+
+  p = g_bytes_get_data (env_block, &remaining);
+
+  /* env_block might not be \0-terminated */
+  while (remaining > 0)
+    {
+      size_t len = strnlen (p, remaining);
+      const char *equals;
+
+      g_assert (len <= remaining);
+
+      equals = memchr (p, '=', len);
+
+      if (equals == NULL || equals == p)
+        return glnx_throw (error,
+                           "Environment variable must be given in the form VARIABLE=VALUE, not %.*s", (int) len, p);
+
+      flatpak_context_set_env_var (context,
+                                   g_strndup (p, equals - p),
+                                   g_strndup (equals + 1, len - (equals - p) - 1));
+      p += len;
+      remaining -= len;
+
+      if (remaining > 0)
+        {
+          g_assert (*p == '\0');
+          p += 1;
+          remaining -= 1;
+        }
+    }
+
+  if (fd >= 3)
+    close (fd);
+
+  return TRUE;
+}
+
 static gboolean
 option_own_name_cb (const gchar *option_name,
                     const gchar *value,
@@ -1206,6 +1265,7 @@ static GOptionEntry context_options[] = {
   { "filesystem", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_filesystem_cb, N_("Expose filesystem to app (:ro for read-only)"), N_("FILESYSTEM[:ro]") },
   { "nofilesystem", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_nofilesystem_cb, N_("Don't expose filesystem to app"), N_("FILESYSTEM") },
   { "env", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_env_cb, N_("Set environment variable"), N_("VAR=VALUE") },
+  { "env-fd", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_env_fd_cb, N_("Read environment variables in env -0 format from FD"), N_("FD") },
   { "own-name", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_own_name_cb, N_("Allow app to own name on the session bus"), N_("DBUS_NAME") },
   { "talk-name", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_talk_name_cb, N_("Allow app to talk to name on the session bus"), N_("DBUS_NAME") },
   { "system-own-name", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_system_own_name_cb, N_("Allow app to own name on the system bus"), N_("DBUS_NAME") },
diff --git a/doc/flatpak-build-finish.xml b/doc/flatpak-build-finish.xml
index 0754335..4d78a84 100644
--- a/doc/flatpak-build-finish.xml
+++ b/doc/flatpak-build-finish.xml
@@ -281,6 +281,24 @@ key=v1;v2;
                 </para></listitem>
             </varlistentry>
 
+            <varlistentry>
+                <term><option>--env-fd=<replaceable>FD</replaceable></option></term>
+
+                <listitem><para>
+                    Read environment variables from the file descriptor
+                    <replaceable>FD</replaceable>, and set them as if
+                    via <option>--env</option>. This can be used to avoid
+                    environment variables and their values becoming visible
+                    to other users.
+                </para><para>
+                    Each environment variable is in the form
+                    <replaceable>VAR</replaceable>=<replaceable>VALUE</replaceable>
+                    followed by a zero byte. This is the same format used by
+                    <literal>env -0</literal> and
+                    <filename>/proc/*/environ</filename>.
+                </para></listitem>
+            </varlistentry>
+
             <varlistentry>
                 <term><option>--own-name=NAME</option></term>
 
diff --git a/doc/flatpak-build.xml b/doc/flatpak-build.xml
index d8ec784..6c3fe16 100644
--- a/doc/flatpak-build.xml
+++ b/doc/flatpak-build.xml
@@ -283,6 +283,24 @@ key=v1;v2;
                 </para></listitem>
             </varlistentry>
 
+            <varlistentry>
+                <term><option>--env-fd=<replaceable>FD</replaceable></option></term>
+
+                <listitem><para>
+                    Read environment variables from the file descriptor
+                    <replaceable>FD</replaceable>, and set them as if
+                    via <option>--env</option>. This can be used to avoid
+                    environment variables and their values becoming visible
+                    to other users.
+                </para><para>
+                    Each environment variable is in the form
+                    <replaceable>VAR</replaceable>=<replaceable>VALUE</replaceable>
+                    followed by a zero byte. This is the same format used by
+                    <literal>env -0</literal> and
+                    <filename>/proc/*/environ</filename>.
+                </para></listitem>
+            </varlistentry>
+
             <varlistentry>
                 <term><option>--own-name=NAME</option></term>
 
diff --git a/doc/flatpak-override.xml b/doc/flatpak-override.xml
index 6e28c76..6e20960 100644
--- a/doc/flatpak-override.xml
+++ b/doc/flatpak-override.xml
@@ -257,6 +257,24 @@ key=v1;v2;
                 </para></listitem>
             </varlistentry>
 
+            <varlistentry>
+                <term><option>--env-fd=<replaceable>FD</replaceable></option></term>
+
+                <listitem><para>
+                    Read environment variables from the file descriptor
+                    <replaceable>FD</replaceable>, and set them as if
+                    via <option>--env</option>. This can be used to avoid
+                    environment variables and their values becoming visible
+                    to other users.
+                </para><para>
+                    Each environment variable is in the form
+                    <replaceable>VAR</replaceable>=<replaceable>VALUE</replaceable>
+                    followed by a zero byte. This is the same format used by
+                    <literal>env -0</literal> and
+                    <filename>/proc/*/environ</filename>.
+                </para></listitem>
+            </varlistentry>
+
             <varlistentry>
                 <term><option>--own-name=NAME</option></term>
 
diff --git a/doc/flatpak-run.xml b/doc/flatpak-run.xml
index e50895d..3f52c99 100644
--- a/doc/flatpak-run.xml
+++ b/doc/flatpak-run.xml
@@ -391,6 +391,24 @@ key=v1;v2;
                 </para></listitem>
             </varlistentry>
 
+            <varlistentry>
+                <term><option>--env-fd=<replaceable>FD</replaceable></option></term>
+
+                <listitem><para>
+                    Read environment variables from the file descriptor
+                    <replaceable>FD</replaceable>, and set them as if
+                    via <option>--env</option>. This can be used to avoid
+                    environment variables and their values becoming visible
+                    to other users.
+                </para><para>
+                    Each environment variable is in the form
+                    <replaceable>VAR</replaceable>=<replaceable>VALUE</replaceable>
+                    followed by a zero byte. This is the same format used by
+                    <literal>env -0</literal> and
+                    <filename>/proc/*/environ</filename>.
+                </para></listitem>
+            </varlistentry>
+
             <varlistentry>
                 <term><option>--own-name=NAME</option></term>
 
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 12 Jan 2021 12:25:59 +0000
Subject: 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.

[Backported to 1.2.x for Debian 10 security update]

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(-)

diff --git a/portal/Makefile.am.inc b/portal/Makefile.am.inc
index 119c1cf..ef1f5da 100644
--- a/portal/Makefile.am.inc
+++ b/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
diff --git a/portal/flatpak-portal.c b/portal/flatpak-portal.c
index 5c4f2fe..b758ad4 100644
--- a/portal/flatpak-portal.c
+++ b/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
 
@@ -167,8 +175,15 @@ 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)
 {
@@ -177,6 +192,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)
@@ -323,6 +341,9 @@ handle_spawn (PortalFlatpak         *object,
   g_auto(GStrv) sandbox_expose_ro = NULL;
   gboolean sandboxed;
   gboolean devel;
+  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);
@@ -525,7 +546,49 @@ handle_spawn (PortalFlatpak         *object,
   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));
     }
 
   if (devel)
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 11 Jan 2021 12:25:50 +0000
Subject: tests: Exercise --env-fd

Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
---
 tests/test-override.sh | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/tests/test-override.sh b/tests/test-override.sh
index 2192289..a86f518 100755
--- a/tests/test-override.sh
+++ b/tests/test-override.sh
@@ -63,14 +63,16 @@ reset_overrides
 
 ${FLATPAK} override --user --env=FOO=BAR org.test.Hello
 ${FLATPAK} override --user --env=BAR= org.test.Hello
-# TODO: A future commit will add a way to avoid this ever being present in argv
-${FLATPAK} override --user --env=SECRET_TOKEN=3047225e-5e38-4357-b21c-eac83b7e8ea6 org.test.Hello
+# --env-fd with terminating \0 (strictly as documented).
+printf '%s\0' "SECRET_TOKEN=3047225e-5e38-4357-b21c-eac83b7e8ea6" > env.3
+# --env-fd without terminating \0 (which we also accept).
 # TMPDIR and TZDIR are filtered out by ld.so for setuid processes,
 # so setting these gives us a way to verify that we can pass them through
 # a setuid bwrap (without special-casing them, as we previously did for
 # TMPDIR).
-${FLATPAK} override --user --env=TMPDIR=/nonexistent/tmp org.test.Hello
-${FLATPAK} override --user --env=TZDIR=/nonexistent/tz org.test.Hello
+printf '%s\0%s' "TMPDIR=/nonexistent/tmp" "TZDIR=/nonexistent/tz" > env.4
+${FLATPAK} override --user --env-fd=3 --env-fd=4 org.test.Hello \
+    3<env.3 4<env.4
 ${FLATPAK} override --user --show org.test.Hello > override
 
 assert_file_has_content override "^\[Environment\]$"
@@ -116,11 +118,11 @@ else
   ${FLATPAK} run --command=bash \
       --env=FOO=BAR \
       --env=BAR= \
-      --env=SECRET_TOKEN=3047225e-5e38-4357-b21c-eac83b7e8ea6 \
-      --env=TMPDIR=/nonexistent/tmp \
-      --env=TZDIR=/nonexistent/tz \
+      --env-fd=3 \
+      --env-fd=4 \
       org.test.Hello \
-      -c 'echo "FOO=$FOO"; echo "BAR=$BAR"; echo "SECRET_TOKEN=$SECRET_TOKEN"; echo "TMPDIR=$TMPDIR"; echo "TZDIR=$TZDIR"' > out
+      -c 'echo "FOO=$FOO"; echo "BAR=$BAR"; echo "SECRET_TOKEN=$SECRET_TOKEN"; echo "TMPDIR=$TMPDIR"; echo "TZDIR=$TZDIR"' \
+      3<env.3 4<env.4 > out
   # The versions from `flatpak run` overrule `flatpak override`
   assert_file_has_content out '^FOO=BAR$'
   assert_file_has_content out '^BAR=$'
From: Simon McVittie <smcv@collabora.com>
Date: Sun, 10 Jan 2021 16:25:29 +0000
Subject: 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
---
 portal/flatpak-portal.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/portal/flatpak-portal.c b/portal/flatpak-portal.c
index b758ad4..dee4ba3 100644
--- a/portal/flatpak-portal.c
+++ b/portal/flatpak-portal.c
@@ -526,6 +526,13 @@ handle_spawn (PortalFlatpak         *object,
   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++)
     {
@@ -533,7 +540,26 @@ handle_spawn (PortalFlatpak         *object,
       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"));
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 11 Jan 2021 12:48:01 +0000
Subject: tests: Assert that --env= does not go in `flatpak run` or bwrap
 environ

For the portal's use of --env-fd= to be safe, we want the environment
variables that it sets to end up in the environment for the program
that is run by `bwrap` as process 2, but they must not go into the
environment that gets used to run `flatpak run` or `bwrap`. Assert
that this is the case.

For completeness, we're testing both --env= and --env-fd= here,
even though the earlier commit
"portal: Do not use caller-supplied variables in environment"
always uses --env-fd=.

Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 tests/Makefile.am.inc  | 10 ++++++++++
 tests/libpreload.c     | 31 +++++++++++++++++++++++++++++++
 tests/test-override.sh | 18 ++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 tests/libpreload.c

diff --git a/tests/Makefile.am.inc b/tests/Makefile.am.inc
index f473051..f799bbf 100644
--- a/tests/Makefile.am.inc
+++ b/tests/Makefile.am.inc
@@ -101,6 +101,16 @@ dist_installed_test_data = \
 	tests/session.conf.in \
 	$(NULL)
 
+test_ltlibraries = tests/libpreload.la
+
+tests_libpreload_la_SOURCES = tests/libpreload.c
+tests_libpreload_la_LDFLAGS = \
+	-avoid-version \
+	-module \
+	-no-undefined \
+	-rpath $(installed_testdir) \
+	$(NULL)
+
 installed_test_keyringdir = $(installed_testdir)/test-keyring
 installed_test_keyring2dir = $(installed_testdir)/test-keyring2
 
diff --git a/tests/libpreload.c b/tests/libpreload.c
new file mode 100644
index 0000000..a640a94
--- /dev/null
+++ b/tests/libpreload.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2021 Collabora Ltd.
+ * SPDX-License-Identifier: LGPL-2-or-later
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+__attribute__((constructor)) static void
+ctor (void)
+{
+  pid_t me = getpid ();
+  struct stat buf;
+
+  fprintf (stderr, "LD_PRELOAD module got loaded by process %d\n", me);
+
+  if (stat ("/.flatpak-info", &buf) == 0)
+    {
+      fprintf (stderr, "OK: pid %d is in a Flatpak sandbox\n", me);
+    }
+  else
+    {
+      /* If the --env=LD_PRELOAD had come from a call to flatpak-portal,
+       * then this would be a sandbox escape (GHSA-4ppf-fxf6-vxg2). */
+      fprintf (stderr, "Error: pid %d is not in a Flatpak sandbox\n", me);
+      abort ();
+    }
+}
diff --git a/tests/test-override.sh b/tests/test-override.sh
index a86f518..e87e458 100755
--- a/tests/test-override.sh
+++ b/tests/test-override.sh
@@ -3,6 +3,11 @@
 set -euo pipefail
 
 . $(dirname $0)/libtest.sh
+if [ -e "${test_builddir}/.libs/libpreload.so" ]; then
+    install "${test_builddir}/.libs/libpreload.so" "${test_tmpdir}"
+else
+    install "${test_builddir}/libpreload.so" "${test_tmpdir}"
+fi
 
 reset_overrides () {
     ${FLATPAK} override --user --reset org.test.Hello
@@ -116,6 +121,7 @@ else
   ${FLATPAK} override --user --show org.test.Hello > override
 
   ${FLATPAK} run --command=bash \
+      --filesystem="${test_tmpdir}" \
       --env=FOO=BAR \
       --env=BAR= \
       --env-fd=3 \
@@ -134,6 +140,18 @@ else
   # could see it
   assert_not_file_has_content out 3047225e-5e38-4357-b21c-eac83b7e8ea6
 
+  # libpreload.so will abort() if it gets loaded into the `flatpak run`
+  # or `bwrap` processes, so if this succeeds, everything's OK
+  ${FLATPAK} run --command=bash \
+      --filesystem="${test_tmpdir}" \
+      --env=LD_PRELOAD="${test_tmpdir}/libpreload.so" \
+      org.test.Hello -c ''
+  printf '%s\0' "LD_PRELOAD=${test_tmpdir}/libpreload.so" > env.ldpreload
+  ${FLATPAK} run --command=bash \
+      --filesystem="${test_tmpdir}" \
+      --env-fd=3 \
+      org.test.Hello -c '' 3<env.ldpreload
+
   ok "temporary environment variables"
 fi
 
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 18 Jan 2021 17:52:13 +0000
Subject: build: Convert environment into a sequence of bwrap arguments

This means we can systematically pass the environment variables
through bwrap(1), even if it is setuid and thus is filtering out
security-sensitive environment variables. bwrap itself ends up being
run with an empty environment instead.

This fixes a regression when CVE-2021-21261 was fixed: before the
CVE fixes, LD_LIBRARY_PATH would have been passed through like this
and appeared in the `flatpak build` shell, but during the CVE fixes,
the special case that protected LD_LIBRARY_PATH was removed in favour
of the more general flatpak_bwrap_envp_to_args(). That reasoning only
works if we use flatpak_bwrap_envp_to_args(), consistently, everywhere
that we run the potentially-setuid bwrap.

Fixes: 6d1773d2 "run: Convert all environment variables into bwrap arguments"
Bug: https://github.com/flatpak/flatpak/issues/4080
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=980323
Signed-off-by: Simon McVittie <smcv@collabora.com>
Applied-upstream: 1.10.1, commit:9a61d2c44f0a58cebcb9b2787ae88db07ca68bb0
---
 app/flatpak-builtins-build.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/flatpak-builtins-build.c b/app/flatpak-builtins-build.c
index 9410791..5ba3ba8 100644
--- a/app/flatpak-builtins-build.c
+++ b/app/flatpak-builtins-build.c
@@ -566,6 +566,8 @@ flatpak_builtin_build (int argc, char **argv, GCancellable *cancellable, GError
                               NULL);
     }
 
+  flatpak_bwrap_envp_to_args (bwrap);
+
   if (!flatpak_bwrap_bundle_args (bwrap, 1, -1, FALSE, error))
     return FALSE;
 
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 18 Jan 2021 18:07:38 +0000
Subject: dir: Pass environment via bwrap --setenv when running apply_extra

This means we can systematically pass the environment variables
through bwrap(1), even if it is setuid and thus is filtering out
security-sensitive environment variables. bwrap ends up being
run with an empty environment instead.

As with the previous commit, this regressed while fixing CVE-2021-21261.

Fixes: 6d1773d2 "run: Convert all environment variables into bwrap arguments"
Bug: https://github.com/flatpak/flatpak/issues/4080
Signed-off-by: Simon McVittie <smcv@collabora.com>
Applied-upstream: 1.10.1, commit:fb473cad801c6b61706353256cab32330557374a
---
 common/flatpak-dir.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index 40a2e58..f79ebcd 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -6799,6 +6799,8 @@ apply_extra_data (FlatpakDir   *self,
                                          app_context, NULL, NULL, cancellable, error))
     return FALSE;
 
+  flatpak_bwrap_envp_to_args (bwrap);
+
   flatpak_bwrap_add_arg (bwrap, "/app/bin/apply_extra");
 
   flatpak_bwrap_finish (bwrap);
