From a47d3b2038c010b317033a3141af5c906dff07d8 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 18 Aug 2021 16:51:29 +0100 Subject: [PATCH] run: Don't let XDG_RUNTIME_DIR from user override the value we set We use `bwrap --setenv XDG_RUNTIME_DIR` to set it to `/run/user/UID`, regardless of what it is on the host system, but the changes made to resolve CVE-2021-21261 unintentionally broke this by overwriting it with the user's XDG_RUNTIME_DIR. In practice this worked for most people, who either have XDG_RUNTIME_DIR set to the same value we use (which is the conventional setup from systemd-logind and elogind), or entirely unset (if they do not have systemd-logind or elogind). However, it broke Wayland and other XDG_RUNTIME_DIR-based protocols for people who intentionally set up an XDG_RUNTIME_DIR that is different. Fixes: 6d1773d2 "run: Convert all environment variables into bwrap arguments" Resolves: https://github.com/flatpak/flatpak/issues/4372 Signed-off-by: Simon McVittie (cherry picked from commit d3e6e71feec23ae6f6c68c2860a1330e12427357) --- common/flatpak-run.c | 4 ++++ tests/test-run.sh | 9 ++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/common/flatpak-run.c b/common/flatpak-run.c index c72b0cd965..c96f344e6e 100644 --- a/common/flatpak-run.c +++ b/common/flatpak-run.c @@ -1377,6 +1377,10 @@ static const ExportData default_exports[] = { {"XDG_DATA_DIRS", "/app/share:/usr/share"}, {"SHELL", "/bin/sh"}, {"TMPDIR", NULL}, /* Unset TMPDIR as it may not exist in the sandbox */ + /* We always use /run/user/UID, even if the user's XDG_RUNTIME_DIR + * outside the sandbox is somewhere else. Don't allow a different + * setting from outside the sandbox to overwrite this. */ + {"XDG_RUNTIME_DIR", NULL}, /* Some env vars are common enough and will affect the sandbox badly if set on the host. We clear these always. */ diff --git a/tests/test-run.sh b/tests/test-run.sh index 9bf2f1b7cb..186cc284f7 100644 --- a/tests/test-run.sh +++ b/tests/test-run.sh @@ -24,7 +24,7 @@ set -euo pipefail skip_without_bwrap skip_revokefs_without_fuse -echo "1..16" +echo "1..17" # Use stable rather than master as the branch so we can test that the run # command automatically finds the branch correctly @@ -74,6 +74,13 @@ assert_file_has_content hello_out '^Hello world, from a sandbox$' ok "hello" +mkdir xrd +XDG_RUNTIME_DIR="$(pwd)/xrd" run_sh org.test.Platform 'echo $XDG_RUNTIME_DIR' > value-in-sandbox +head value-in-sandbox >&2 +assert_file_has_content value-in-sandbox "^/run/user/$(id -u)\$" + +ok "XDG_RUNTIME_DIR not inherited" + run_sh org.test.Platform cat /.flatpak-info >runtime-fpi assert_file_has_content runtime-fpi "[Runtime]" assert_file_has_content runtime-fpi "^runtime=runtime/org\.test\.Platform/$ARCH/stable$"