common/flatpak-bwrap-private.h | 3 3 + 0 - 0 !
common/flatpak-bwrap.c | 43 43 + 0 - 0 !
common/flatpak-run.c | 24 15 + 9 - 0 !
3 files changed, 61 insertions(+), 9 deletions(-)

 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

tests/libtest.sh | 4 4 + 0 - 0 !
1 file changed, 4 insertions(+)

 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/test-override.sh | 68 67 + 1 - 0 !
1 file changed, 67 insertions(+), 1 deletion(-)

 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

common/Makefile.am.inc | 2 2 + 0 - 0 !
common/flatpak-utils-memfd-private.h | 32 32 + 0 - 0 !
common/flatpak-utils-memfd.c | 90 90 + 0 - 0 !
common/flatpak-utils-private.h | 1 1 + 0 - 0 !
common/flatpak-utils.c | 50 0 + 50 - 0 !
5 files changed, 125 insertions(+), 50 deletions(-)

 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/flatpak-context.c | 60 60 + 0 - 0 !
doc/flatpak-build-finish.xml | 18 18 + 0 - 0 !
doc/flatpak-build.xml | 18 18 + 0 - 0 !
doc/flatpak-override.xml | 18 18 + 0 - 0 !
doc/flatpak-run.xml | 18 18 + 0 - 0 !
5 files changed, 132 insertions(+)

 context: add --env-fd option

This allows environment variables to be added to the context without

portal/Makefile.am.inc | 4 3 + 1 - 0 !
portal/flatpak-portal.c | 65 64 + 1 - 0 !
2 files changed, 67 insertions(+), 2 deletions(-)

 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

tests/test-override.sh | 18 10 + 8 - 0 !
1 file changed, 10 insertions(+), 8 deletions(-)

 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

portal/flatpak-portal.c | 28 27 + 1 - 0 !
1 file changed, 27 insertions(+), 1 deletion(-)

 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

tests/Makefile.am.inc | 10 10 + 0 - 0 !
tests/libpreload.c | 31 31 + 0 - 0 !
tests/test-override.sh | 18 18 + 0 - 0 !
3 files changed, 59 insertions(+)

 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>

app/flatpak-builtins-build.c | 2 2 + 0 - 0 !
1 file changed, 2 insertions(+)

 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>

common/flatpak-dir.c | 2 2 + 0 - 0 !
1 file changed, 2 insertions(+)

 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>

common/flatpak-dir.c | 2 2 + 0 - 0 !
1 file changed, 2 insertions(+)

 disallow @@ and @@u usage in desktop files

(cherry picked from commit 652a28ffab67cb6cd7d12dc3a93979bcd3731c7f)

Bug: https://github.com/flatpak/flatpak/issues/4146
Bug-Debian: https://bugs.debian.org/984859

common/flatpak-dir.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 dir: reserve the whole @@ prefix

If we add new features analogous to file forwarding later, we might

common/flatpak-dir.c | 6 5 + 1 - 0 !
1 file changed, 5 insertions(+), 1 deletion(-)

 dir: refuse to export .desktop files with suspicious uses of @@
 tokens

This is either a malicious/compromised app trying to do an attack, or
a mistake that will break handling of %f, %u and so on. Either way,
if we refuse to export the .desktop file, resulting in installation
failing, then it makes the rejection more obvious than quietly
removing the magic tokens.

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 46b3ede5241561c7d588951048c687c5075a3eac)
Bug: https://github.com/flatpak/flatpak/issues/4146
Bug-Debian: https://bugs.debian.org/984859

tests/http-utils-test-server.py | 2 1 + 1 - 0 !
tests/oci-registry-client.py | 2 1 + 1 - 0 !
tests/oci-registry-server.py | 2 1 + 1 - 0 !
tests/test-http-utils.sh | 2 1 + 1 - 0 !
tests/test-oci-registry.sh | 4 2 + 2 - 0 !
tests/test-webserver.sh | 4 2 + 2 - 0 !
6 files changed, 8 insertions(+), 8 deletions(-)

 use python 3 for tests