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 46b3ede524)
If we add new features analogous to file forwarding later, we might
find that we need a different magic token. Let's reserve the whole
@@* namespace so we can call it @@something-else.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 1e7e8fdb24)
When the portal's Spawn method is used with the environment cleared,
it's very likely that the "flatpak run" that ends up being run will be
in an environment without UTF-8 support.
If one of the files or directories we try to expose to the sub-sandbox
contains UTF-8/non-ASCII characters, then "flatpak run" would fail with:
error: Invalid byte sequence in conversion input
This is caused by GOption trying to parse the --filesystem option for
flatpak, as, when using the G_OPTION_ARG_CALLBACK argument type, GOption
will split the option name from its value, and try to convert the value
to UTF-8. Which will fail because there's no UTF-8.
It won't however do that if we tell the option parser that the value is
a filename using G_OPTION_FLAG_FILENAME, so set it.
(cherry picked from commit e67847e253)
OpenSUSE inherits the hostname value from DHCP without updating its X11
authentication cookie, and it keeps the initial value in
`XAUTHLOCALHOSTNAME`.
To avoid breaking the X11 applications, OpenSUSE patches libxcb so that
it also considers the value in `XAUTHLOCALHOSTNAME` as another possible
hostname.
https://bugzilla.opensuse.org/show_bug.cgi?id=262309
To cope with that behavior we need to check `XAUTHLOCALHOSTNAME` too
and, if we have a cookie with that address, propagate it inside the
container adjusting its address to the canonical hostname
`unames.nodename`.
Fixes: #4043
Signed-off-by: Ludovico de Nittis <ludovico.denittis@collabora.com>
(cherry picked from commit fd6a3b79f4)
FLATPAK_ERROR_ALREADY_INSTALLED can be returned from, for example,
flatpak_installation_add_remote(), so document that it applies to
remotes not just apps and runtimes.
There are various reasons why distro autobuilder environments might not
be able to use FUSE in build-time tests: they might not have
fusermount(1), they might be locked-down to be unable to load the FUSE
kernel module as a result of security hardening, they might not have the
/etc/mtab symlink, they might not have CAP_SYS_ADMIN in their capability
bounding set, or they might not have write access to /dev/fuse. Try to
check for all of these conditions.
Heavily based on code that I previously contributed to
xdg-desktop-portal. Disabling revokefs, instead of skipping the test
that needs it, is based on the patches applied in @alexlarsson's PPA.
Signed-off-by: Simon McVittie <smcv@collabora.com>
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.
This did not regress in 6d1773d "run: Convert all environment variables
into bwrap arguments", because the LD_LIBRARY_PATH special case in
flatpak_run_add_environment_args() was already not used here; but it's
a bug fix along the same lines as fixing the regression.
Signed-off-by: Simon McVittie <smcv@collabora.com>
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"
Signed-off-by: Simon McVittie <smcv@collabora.com>
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"
Resolves: 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>
This adds `NULL` protection to the `subpaths` argument to
`flatpak_dir_system_helper_call_deploy()`, just like already exists for
other arguments.
It’s a second layer of defence against bugs like fixed in the previous
commit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If an app had no deploy data, and `flatpak_dir_update()` was called with
`opt_subpaths == NULL`, then `arg_subpaths` passed to
`flatpak_dir_system_helper_call_deploy()` would be `NULL`, which causes
an assertion failure from an internal `g_variant_new()` call:
```
g_variant_new_strv: assertion 'length == 0 || strv != NULL' failed
```
Fix that by setting the `subpaths` to an empty array if nothing else is
specified.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
This passes -fsanitize=address in the right place.
Passing it this way instead of CFLAGS allows us to strategically
not add sanitize in specific places as needed.
This is really just syntactic sugar for running `env -u VAR ... COMMAND`,
but env(1) is inconvenient when the form of the COMMAND is not known:
if the COMMAND might contain an equals sign, you end up having to run
`env -u VAR sh -c 'exec "$@"' sh COMMAND`. Let's make this simpler.
This follows up from GHSA-4ppf-fxf6-vxg2 to fix an issue that I noticed
while resolving that vulnerability, but is not required for fixing the
vulnerability.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This follows up from GHSA-4ppf-fxf6-vxg2 to fix missing functionality
that I noticed while resolving that vulnerability, but is not required
for fixing the vulnerability.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Some consumers of environment variables distinguish between present
with an empty value and absent. For example, if an environment variable
represents a search path like VK_ICD_FILENAMES, unsetting it often
results in use of a default, but setting it to the empty string results
in not searching any locations, which is sometimes what is desired.
The shell syntax "${BAR-unset}" expands to the value of ${BAR} if it
is set to anything (even an empty string), or to "unset" if not.
We can use that in the unit test to check that BAR is set to the
empty string in this case.
This follows up from GHSA-4ppf-fxf6-vxg2 to fix an issue that I noticed
while resolving that vulnerability, but is not required for fixing the
vulnerability.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Previously, if you launched a subsandbox while specifying environment
variable overrides, any environment variable overrides that existed
in the parent Flatpak app would take precedence:
host$ flatpak run --env=FOO=1 --command=bash example.app
[📦 example.app ~]$ env | grep FOO
FOO=1
[📦 example.app ~]$ flatpak-spawn --env=FOO=x sh -c 'env | grep FOO'
FOO=1
This does not seem like least-astonishment, and in particular will
cause problems if the app wants to override LD_LIBRARY_PATH in the
subsandbox. Change the precedence so that the environment variables
set by flatpak-spawn will "win":
host$ flatpak run --env=FOO1=1 --env=FOO2=2 --command=bash example.app
[📦 example.app ~]$ env | grep FOO
FOO1=1
FOO2=2
[📦 example.app ~]$ flatpak-spawn --env=FOO1=x sh -c 'env | grep FOO'
FOO1=x
FOO2=2
This follows up from GHSA-4ppf-fxf6-vxg2 to fix an issue that I noticed
while resolving that vulnerability, but is not required for fixing the
vulnerability.
Signed-off-by: Simon McVittie <smcv@collabora.com>
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>
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
