The previous commit ensures that there isn't a heap overflow when
reading a huge delta path, but we should also just reject unreasonably
long paths. So we chose the arbitrary limit of PATH_MAX and assume that
anything beyond that arbitrary limit is probably abusive.
Helps: https://github.com/flatpak/flatpak/security/advisories/GHSA-jr92-2v97-wgvc
delta_read_data computed g_malloc(size + 1) where size came from the
delta stream. If size equals G_MAXSIZE, size + 1 wraps to zero and
g_malloc returns a minimal allocation, then g_input_stream_read_all
writes size bytes into it — a heap buffer overflow.
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-jr92-2v97-wgvc
The delta varint parser decoded into guint64 values which were then
passed to GLib I/O and allocation functions that take gsize. On 32-bit
systems where gsize is 32 bits this silently truncated the values.
Change the varint output and all delta operation size parameters to
gsize, and clamp the parsed value to G_MAXSIZE.
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-jr92-2v97-wgvc
The original reporter used various tricks to find a way for the sandboxed
app to overwrite these locations with symlinks, but for the purposes
of this test, I'm doing the setup outside the sandbox instead: probably
not all of these potential exploit routes are actually possible, but we
defend against all of them symmetrically.
Signed-off-by: Simon McVittie <smcv@collabora.com>
[backported to 1.18.x: adjust test numbering]
Replace path-based flatpak_mkdir_p calls in flatpak_ensure_data_dir
with glnx_chase_and_mkdirat(RESOLVE_NO_SYMLINKS) to prevent an app
from replacing subdirectories of its data dir with symlinks between
runs and having them followed during the next sandbox setup.
In flatpak_run_setup_base_argv, replace path-based --bind args for
the app cache/data/config/tmp directories with --bind-fd using fds
obtained via glnx_chaseat(RESOLVE_NO_SYMLINKS), preventing both
symlink following and TOCTOU races when setting up these bind mounts.
Assisted-by: Claude:opus-4.6
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-8688-9x26-hhxj
This use of symlinks to ensure that the canonicalized paths of
`$XDG_CACHE_HOME`, `$XDG_CONFIG_HOME`, `$XDG_DATA_HOME` are
`~/.cache`, `~/.config`, `~/.local/share` is the sort of thing that
could easily regress if not tested.
Signed-off-by: Simon McVittie <smcv@collabora.com>
[backported to 1.18.x: adjust test numbering]
A sandboxed app can replace ~/.var/app/$appid/.ld.so with a symlink,
causing regenerate_ld_cache to write files at an arbitrary location.
A concurrent app instance makes this a TOCTOU even after the initial
directory verification.
Replace all path-based operations with fd-based equivalents using
ld_so_dir_fd obtained via glnx_chase_and_mkdirat, and pass the
directory to bwrap via --bind-fd instead of --bind.
Assisted-by: Claude:opus-4.6
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-99wv-m8rp-g58x
This reverts commit 420ce91428.
Apparently we do depend on the session-set certificates in some
situations. We should fix that, but until someone puts in the effort to
do so, reverting this will at least unbreak things.
(cherry picked from commit e61ae2114c)
As per g_type_ensure's documentation it is technically incorrect to mark
_get_type fns with G_GNUC_CONST since they have side-effects on their
first run.
See https://gitlab.gnome.org/GNOME/glib/-/merge_requests/5223 for more
details.
(cherry picked from commit 59f9a7729f):
Instead of modifying the host-like run environment to clear the sandbox
environment, we'll use the new --clear-env flag which does the correct
thing.
Assisted-by: Claude:opus-4.6
Closes: #5271
(cherry picked from commit afbabdc32c):
This reverts commit a57f6bc372.
The run-environ from the calling instance is a host-like environment
(e.g. on NixOS it contains /nix/store paths). Passing it via --env
injects it into the sandbox payload environment where those paths don't
exist.
Revert the commit, so we pass run-environ as the envp for spawning
flatpak run again to let it make host-level decisions (DISPLAY,
FLATPAK_GL_DRIVERS, XDG_RUNTIME_DIR, etc.) without leaking into the
sandbox.
It also passes --clear-env unconditionally, because we'd build up the
environment, but the wrong one. We will implement --clear-env properly
again in the next few commits.
Closes: #6717
Fixes: a57f6bc3 ("portal: Clear the environment via flatpak arguments")
(cherry picked from commit b9fb6d4e25):
1. Removed unnecessary punctuation to help with readability
2. Loop over RES elements directly instead of indexing
3. Declare COMPGEN_OPTS only once vs redeclaring on each iteration
(cherry picked from commit 02fc85a3c2):
1. Use $() command substitution instead of legacy `` style
2. Properly quote COMPGEN_OPTS
3. Use readarray instead of relying on word splitting
(cherry picked from commit 21ae7723c0):
A few global variables were being created unecessarily still. This makes
sure that users shell environments are not polluted by extra variables
and that the completion doesn't modify unrelated variables
(cherry picked from commit 9047dabfaf):
Verify that the file object is valid before checking its cached path
to avoid a potential NULL pointer dereference.
Fixes: c4fce9e4 ("run: Error out if file forwarding of empty paths is attempted")
(cherry picked from commit f266346d6d):
The file forwarding feature errors out when the path or URI does not
lead to an actual file that can be forwarded. The empty path never
describes an actual file, so we always have to error out.
Without the check, we would get a NULL path from
`flatpak_file_get_path_cached` and crash later.
Closes: #6689
(cherry picked from commit c4fce9e42a):
`emit_progress` is called from a thread while another thread could
unexport/close the GDbusInterfaceSkeleton. If that happens, the
connection becomes NULL which is illegal for
`g_dbus_connection_emit_signal` which results in dereferencing an unset
error.
The fix is to lock the UpdateMonitorData, and then only emit the signal
if it is not closed.
(cherry picked from commit ce4a27e1fe):
We were handling null properties the same as missing properties
*except* that the MANDATORY flag allowed null properties but
not missing properties. Fix this, so null is disallowed by
MANDATORY.
When checking signatures, the image identity could only have
been NULL if it was null in the input file - so replace a
conditional check on it being non-null with an assertion.
(cherry picked from commit 906affa13b):
It's legitimate to have manifests listed in an image index that
have no platform object, and hence no architecture - avoid crashing
if we encounter such a manifest.
(cherry picked from commit b15828e119):
Mark all properties required by the OCI specification as required;
this eliminates a bunch of cases where we were assuming that
descriptor->digest was non-NULL, and potentially generating
critical errors from g_return_if_fail().
(cherry picked from commit fa4b413c02):
Both collect exports and flatpak_export_dir recursively export the
directories in their respective directory lists. Because of this,
including share/metainfo/releases when share/metainfo is already in the
lists is unnecessary and can cause exporting to fail because of
duplicate files.
(cherry picked from commit 4d3f0bbb79):
Signature downloads can use a different URL from the OCI registry
itself, so they must not depend on TLS options left on the reused curl
handle. Resolve certificates for the URI and pass them explicitly.
(cherry picked from commit 8ac2adefce):
Annotating the return as (transfer full) causes bindings to additionally
unref each element on array free, resulting in a use-after-free.
Change the annotation to (transfer container) so bindings know to unref
the array only.
Fixes: https://github.com/flatpak/flatpak/issues/6666
(cherry picked from commit 69bf837676):
It was solved partially in e0e1b20, but not for completely disallowed code path.
Steps to reproduce:
1. Run any flatpak with explicit --unshare option, e.g:
[user@user-standardpc ~]$ flatpak run --unshare=network --command=bash io.gitlab.librewolf-community
2. Inside flatpak shell spawn any executable:
[io.gitlab.librewolf-community ~]$ flatpak-spawn ls /
Result is:
error: Unknown option --noshare=network
(cherry picked from commit db70882df1):
In flatpak_pull_from_oci we can be in in the system helper where we pull
the mirrored OCI image into the system repo. However, to fetch the
signatures in GPG signed repos, we used a remote OciImageSource created
through `flatpak_remote_state_fetch_image_source`. This caused fetching
some data from the registry which we don't want in the deploy method,
and also fails if a token is required to access the repo.
This change fetches the signatures from the mirrored OCI repo instead of
pulling them from the remote OciImageSource. The signatures can come from
anywhere because we verify them against the GPG key in the system repo.
The important bit is the change in `flatpak_pull_from_oci` where we now
pass in the local image_source to fetch the signatures from, and in the
system helper, where we get the right metadata to check the signatures
against (eventually ends up in `flatpak_oci_signatures_verify`).
(cherry picked from commit 32baedaa7e):
* backports, local-alloc: Provide a backport of g_autofd
* build: Add meson.override_dependency('libglnx', libglnx_dep)
* fdio: Add support for name_to_handle_at
* chase: Add glnx_chase_and_mkdirat
Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
We found that there is a common use case where we need to get a
subdirectory (potentially multiple levels) which might not exist yet.
Adding another flag for this to GlnxChaseFlags is what systemd has done,
but creating a directory takes a mode, so the flag creates directories
with a fixed mode. This approach instead takes the mode as argument.
It takes a callback which gets called every time we try to open the next
segment of the path. This allows implementing more specific and advanced
use cases to be implemented without adding more complexity to the chase
algorithm itself.
The bare tuple unpack 'executable, = re.findall(...)' raises a cryptic
ValueError if the pattern matches zero or more than one line. Replace it
with an explicit length check and a clear error message.
Also pass count=1 to str.replace() so a /newroot prefix is only stripped
once and /app/ paths are passed through unchanged.
Sebastian Wick