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