From d19f6c330aa42e17df6dc36d12b6f4dfa507dbb3 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 11 Jan 2021 12:48:01 +0000
Subject: [PATCH] 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 e1417ca4..00fc1b6e 100644
--- a/tests/Makefile.am.inc
+++ b/tests/Makefile.am.inc
@@ -167,6 +167,16 @@ dist_installed_test_data = \
 	tests/org.flatpak.Authenticator.test.service.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 00000000..a640a945
--- /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 56be1217..8a0dc1c5 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
 
 skip_revokefs_without_fuse
 
@@ -118,6 +123,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 \
@@ -136,6 +142,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