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>
(cherry picked from commit 9a61d2c44f)
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
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
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>
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>
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
processes running under a different uid (which is important if their
values are tokens or other 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
Support a list of versions that are supported. This will be useful
for e.g. the extra_data for extensions once that is backported to
1.2, because that will require it to say that it is supported for
> 1.2.5 in the 1.2 series and > 1.4.2 otherwise.
Closes: #3112
Approved by: alexlarsson
(cherry picked from commit 5026f01153)
(cherry picked from commit 9bbe6fbb48)
Closes: #3115
Approved by: alexlarsson
This sets required-flatpak in the metadata to some different versions
and ensure we're properly able or not able to install it.
Additionally it uses some options with multiple versions. This is not
yet supported but I want to test the existing code and make sure it
properly falls back to just using the first element of the list.
Closes: #3112
Approved by: alexlarsson
(cherry picked from commit 62117308c1)
(cherry picked from commit 5db4631a79)
Closes: #3115
Approved by: alexlarsson
Currently if you have a remote configured with the URL
"https://dl.flathub.org/repo/" (as you would if you use the flatpakrepo
file) and you use a flatpakref file which specifies the URL
"https://dl.flathub.org/repo", Flatpak tries to add a duplicate remote
because it doesn't see those URLs as equal. So ignore the trailing slash
when comparing remote URLs. OSTree works equally well with both kinds
(it uses g_build_filename()).
Flathub served flatpakref files with URLs missing a trailing slash until
this commit:
https://github.com/flathub/ansible-playbook/commit/b20694f09
Also, add a unit test that fails without this patch.
Fixes https://github.com/flatpak/flatpak/issues/2979Closes: #3065
Approved by: alexlarsson
(cherry picked from commit 866ba643d2)
Closes: #3115
Approved by: alexlarsson
We were only handling the old single-value 'version' key, even though
we handled the 'versions' key when finding remote related refs.
The result of this was that some extensions, such as the 19.08 opengl
default one was installed by default (as it was found as remote related)
yet still removed with --unused (as it was not locally related).
Fixes https://github.com/flatpak/flatpak/issues/3004Closes: #3093
Approved by: mwleed
(manually cherry-picked from commit aa7b62a0bf)
Closes: #3115
Approved by: alexlarsson
This prevents a crash in flatpak_installation_list_remotes_by_type() if
the `FlatpakDir` can’t ensure it has a repo configured.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #3028
Approved by: alexlarsson
(cherry picked from commit 791f31898b)
(cherry picked from commit 12cecc852c)
Closes: #3115
Approved by: alexlarsson
If the download URL for an icon was already cached locally, the HTTP
code returned FLATPAK_OCI_ERROR_NOT_CHANGED - this was treated as a real
error, and after downloading missing icons, all present icons were
deleted!
See https://bugzilla.redhat.com/show_bug.cgi?id=1683375Closes: #3048
Approved by: alexlarsson
(cherry picked from commit a794c07e18)
(cherry picked from commit 767a9d5a43)
Closes: #3115
Approved by: alexlarsson
It's an error to call g_set_error() on an error that's already set, and
flatpak_dir_get_deploy_data() already sets FLATPAK_ERROR_NOT_INSTALLED
for us when necessary, so just pass on any errors to the caller of
flatpak_dir_get_origin(). In the case that the error is something else,
that should be treated as an error, because we never expect the deploy
directory to exist but not have a deploy file (see the implementations
of flatpak_dir_deploy() and flatpak_dir_undeploy()).
Closes: #3032
Approved by: alexlarsson
(cherry picked from commit b44c1a5799)
(cherry picked from commit 8ed03ef61e)
Closes: #3115
Approved by: alexlarsson
This works around an old polkit client library bug which would cause
a segfault in this situation. The bug was fixed long ago in upstream
polkit, but is still present in Debian 10 'buster', Ubuntu 19.04 'disco'
and all older releases, due to Debian/Ubuntu using a branch of polkit
to avoid the mozjs dependency. It should finally get fixed in Debian 11
and Ubuntu 19.10.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug-Debian: https://bugs.debian.org/923046Closes: #2997
Approved by: matthiasclasen
(cherry picked from commit 75b5b7c763)
(cherry picked from commit 62cb645171)
Closes: #3115
Approved by: alexlarsson
I spotted this line in the output from `flatpak history`:
Jun 4 16:17:20 deploy install com.discordapp.Discord x86_64 stable system flathub 8a0fc700c701 Installed %s from %s root (test) flatpak-system-helper (gnome-software) 1.3.3
This is because the format string is passed as the 'url' parameter, the
first format parameter (the ref) is passed as the 'format' parameter,
and 'origin' is ignored because (fortunately) as far as I know, no
character significant to printf (like '%') is permitted in ref names.
Fix this by passing a NULL 'url', like the neighbouring call in
flatpak_dir_deploy_update().
(cherry picked from commit dbc90df513)
(cherry picked from commit 5a16666bdc)
Closes: #3115
Approved by: alexlarsson
Toolbox [1] containers, like Flatpak containers, need to keep some
configuration files synchronized with the host. eg., /etc/localtime,
/etc/resolv.conf, etc.. The easiest way to keep them updated is to bind
mount the entire /etc from the host at some location inside the
container, and then replace the container's copy of the files with
symbolic links to the bind mounted copies.
However, this doesn't work when the host's copy of a configuration file
is itself a symbolic link to a location outside /etc. eg.,
/etc/localtime, which points somewhere inside /usr/share/zoneinfo. In
that case the host needs to be actively monitored via inotify(7) and
an updated copy of the file needs to maintained in a separate
directory, which can be bind mounted into the container.
This monitoring infrastructure is already present in Flatpak and it
makes sense to leverage it for Toolbox containers too.
However, Flatpak containers have only one user, the current one logged
into the host, whereas Toolbox containers have a longer list of users
like the host itself. Therefore, if the directory getting bind mounted
(ie., $XDG_RUNTIME_DIR/.flatpak-helper/monitor) isn't world readable,
then some users inside the container won't be able to read the
configuration file. eg., this will fail:
$ sudo -u operator cat /run/host/monitor/localtime
Relaxing the directory permissions on the host shouldn't pose a
security risk. They are copies of originals that are world readable
anyway.
[1] https://github.com/debarshiray/toolboxCloses: #2916
Approved by: alexlarsson
(cherry picked from commit bf96de941b)
Closes: #3115
Approved by: alexlarsson
We need to check whether the remote is gpg verified after handling
the oci case, because OCI is fine to update systemwide without gpg
verification (in fact it doesn't support verification).
This just reorders the code, matching what is done in the install
case already.
Closes: #2891
Approved by: alexlarsson
(cherry picked from commit 4c4c80b85d)
Closes: #3115
Approved by: alexlarsson
If the buffer is too small, readlink() returns the number of bytes that
would have been stored if the buffer had been large enough.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Closes: #2873
Approved by: matthiasclasen
(cherry picked from commit 89dc344ccb)
Closes: #3115
Approved by: alexlarsson
When an appstream update is cancelled while downloading
icons, properly fail. Otherwise, the next update attempt
will see an up-to-date timestamp, think everyhing is ok
and not download the missing icons.
Closes: https://github.com/flatpak/flatpak/issues/2835Closes: #2836
Approved by: alexlarsson
(cherry picked from commit f28e62add1)
Closes: #3115
Approved by: alexlarsson
Commit b20ee7e26 introduced a bug where flatpak_load_deploy_data()
returns NULL without setting @error in the case that there's a problem
reading the deploy file. I think this is causing a seg fault in
flatpak_installation_list_installed_refs() which tries to print the
error message after calling get_ref() (which indirectly loads the deploy
file).
I didn't personally verify that this fixes the GNOME Builder seg fault
that was reported, but it seems almost certain looking at the provided
stack trace, since get_ref() only returns NULL if
flatpak_dir_get_deploy_data() returns NULL, and that function clearly
sets the error pointer on failure except in flatpak_load_deploy_data().
I'm not sure why the deploy file doesn't exist (or is unreadable) for
the user who reported this seg fault.
Fixes https://github.com/flatpak/flatpak/issues/2717Closes: #2733
Approved by: matthiasclasen
(cherry picked from commit 7a5c024695)
Whenever we use $XDG_RUNTIME_DIR and expose it somehow in the sandbox
we fully resolve the path, because if (as happens on gentoo for instance)
it contains /var/run -> ../run, then flatpak thinks we need to
add the /var/run symlink in the runtime even though we already
exposed that.
Closes: #2710
Approved by: matthiasclasen
(cherry picked from commit 28e96288c4)
As shown by CVE-2019-5736, it is sometimes possible for the sandbox
app to access outside files using /proc/self/exe. This is not
typically an issue for flatpak as the sandbox runs as the user which
has no permissions to e.g. modify the host files.
However, when installing apps using extra-data into the system repo
we *do* actually run a sandbox as root. So, in this case we disable mounting
/proc in the sandbox, which will neuter attacks like this.
(cherry picked from commit 468858c1cbcdbcb27266deb5c7347b37adf3a9e4)
