Instead of announcing ahead of time how many tests will be run, just
log 1..32 or similar at the end, from the done_testing function.
This should go some way towards preventing unnecessary cherry-pick
conflicts when tests added to this script get backported.
Resolves: https://github.com/flatpak/flatpak/issues/6793
Signed-off-by: Simon McVittie <smcv@collabora.com>
Cherry-picked-from: eb240aa067
This is the equivalent of the function of the same name in Perl's
Test::More. If we already emitted a test plan, it asserts that the
number of tests we planned to do equals the number we actually did.
If not, it assumes that however many tests we have done, that's all of
the tests that we intend to do - this can be useful in test scripts
that routinely increase in length, like test-run.sh which is becoming
rather long (and has frequent conflicts for the "plan" line when we
cherry-pick new test coverage to older branches).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Cherry-picked-from: f7a88aca00
This is approximately the equivalent of 'plan' in Perl Test::More:
it announces how many tests we plan to run.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Cherry-picked-from: 724ac88751
This will help us to emit correct TAP syntax, without having to always
declare up-front how many tests we are going to run.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Cherry-picked-from: ea4321912b
fd_map_remap_fd assigns a temporary fd number (++max_fd) that may later
be used as a conflict resolution target. If the source fd passed to
fd_map_remap_fd is higher than max_fd, conflict resolution can assign
a replacement value that collides with that source fd. In
child_setup_func, the dup2 for the conflicting entry then overwrites
the source fd before it is remapped, delivering the wrong fd to the
child process.
In practice this manifests as sandbox-expose-fd-ro passing a random fd
(e.g. a socket) instead of the intended O_PATH fd to flatpak run via
--ro-bind-fd, producing errors like:
error: /proc/self/fd/21 resolves to non-absolute path socket:[423446]
Similar error have been observed in Epiphany/WebKit:
https://bugs.webkit.org/show_bug.cgi?id=305344
Cherry-picked-from: 22eec07b5a
While I'm changing the translatable string anyway, this also
incorporates a suggestion from #6753 to quote the invalid character, so
that output is clearer in the case where the invalid character is
whitespace.
Thanks: Christian Stadelmann
Signed-off-by: Simon McVittie <smcv@collabora.com>
Cherry-picked-from: 5f3f7a8b98
Until now we have been using `%c` to show invalid characters, but in
general that will corrupt our output if the input is non-ASCII,
because the individual bytes of a UTF-8 string are not valid UTF-8 alone.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Cherry-picked-from: ffcb20f781
The code used the connection name, which is the unique sender name of
the system-helper itself. Fix this and use the sender name of the
invocation.
Cherry-picked-from: 100b450034
The cache_dirs_in_use tracks the cache dirs, and which pull it is
associated with. It can also be associated with no pull anymore in which
case the value is NULL.
The iteration over cache_dirs_in_use thus needs to handle NULL values.
Cherry-picked-from: a7bf3fbf29
Reject summaries, subsummaries, and summary indexes that are not in
GVariant normal form before passing them to the generated variant
accessor framework, which performs direct pointer arithmetic without
structural validation.
Cherry-picked-from: 5a6c442c05
We require glib 2.46, so we can't rely on g_hash_table_new_similar.
Fixes: 17cb1135 ("context: Keep fallback-x11 separate from x11 conditionals")
Closes: #6775
Cherry-picked-from: 507d4b7df6
* glnx_file_copy_at: Use O_PATH fd for xattr and ownership operations
* xattrs: Drop fd-based code paths from get_xattrs_impl
* xattrs: Use /proc/self/fd path in glnx_fd_set_all_xattrs
* xattrs: Use /proc/self/fd path in glnx_fd_get_all_xattrs
* chase: Handle EAGAIN from openat2
The EAGAIN handling in openat2 fixes a bug which would prevent
extensions from being populated in the sandbox:
Closes: #6783
Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
After creating the symlink, open it with O_PATH to pin the inode and
verify the target matches. All subsequent operations (xattrs, chown) go
through the pinned fd instead of path-based operations.
From openat2(2):
EAGAIN how.resolve contains either RESOLVE_IN_ROOT or
RESOLVE_BENEATH, and the kernel could not ensure that a
".." component didn't escape (due to a race condition or
potential attack). The caller may choose to retry the
openat2() call.
We should handle this by simply retrying the syscall.
apply_extra_data() used GFile path-based operations that follow
symlinks: --ro-bind/--bind with resolved paths, chmod on the extra
directory path, and flatpak_cp_a for the extra/export merge.
These are not independently exploitable: apply_extra_data only runs
after extract_extra_data has already created files/extra as a real
directory, so there is no symlink left to follow. The apply_extra
script itself runs in a restricted sandbox with dropped caps and no
/proc. The extra/export merge only adds to the app's own export
directory, which is already app-controlled and whose desktop file
Exec keys are rewritten to flatpak run.
Replace with fd-relative operations as defense in depth:
glnx_chaseat with GLNX_CHASE_RESOLVE_NO_SYMLINKS and
GLNX_CHASE_RESOLVE_BENEATH for directory traversal,
--ro-bind-fd/--bind-fd for bwrap mounts, fchmod on the directory fd,
and flatpak_cp_a_at for the export merge.
Helps: https://github.com/flatpak/flatpak/security/advisories/GHSA-fqx6-vh4p-42cg
[smcv: Use glnx_fd_reopen() to reopen O_PATH fd as readable]
Co-authored-by: Simon McVittie <smcv@collabora.com>
extract_extra_data() had two vulnerabilities:
1. It resolved "files/extra" using g_file_resolve_relative_path()
which follows symlinks. A crafted OSTree commit with "files" as a
symlink causes extra-data blobs to be written at the symlink target.
On system installs this runs as root via the system helper, which
validates signatures and checksums but not tree structure.
2. It used g_file_get_child(extradir, name) where name comes from
xa.extra-data-sources in the commit metadata. Names containing ".."
escape the extra/ directory. This is exploitable through the normal
build flow: flatpak build-export rejects "/" but not "..".
Replace GFile path operations with fd-relative operations: open
"files" with glnx_chaseat using GLNX_CHASE_RESOLVE_NO_SYMLINKS,
create "extra" with glnx_chase_and_mkdirat using
GLNX_CHASE_RESOLVE_BENEATH, validate extra-data names against ".",
"..", and "/", and write with glnx_file_replace_contents_at anchored
to the extra directory fd.
[smcv: Open checkoutdir_dfd before trying to open its files subdir]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-fqx6-vh4p-42cg
Reject extra-data names containing ".", "..", or "/" early during the
download phase. The name is used to construct a local cache file path
via flatpak_build_file() which uses g_file_resolve_relative_path(),
so ".." components could traverse outside the cache directory.
In practice this is not exploitable: the local cache path is only
used for a g_file_query_exists() check followed by g_file_load_contents(),
and the loaded content is verified against the expected sha256 from
the commit metadata.
Helps: https://github.com/flatpak/flatpak/security/advisories/GHSA-fqx6-vh4p-42cg
ensure_extensions() used g_file_resolve_relative_path() with
ext->directory, which comes from runtime/SDK metadata and can contain
".." components. This allowed a malicious extension to write outside
the build directory.
Replace the GFile-based path resolution with glnx_chase_and_mkdirat()
using GLNX_CHASE_RESOLVE_BENEATH | GLNX_CHASE_RESOLVE_NO_SYMLINKS,
which rejects any path that escapes the directory fd.
Convert the rest of flatpak_builtin_build_init to fd-relative
operations (mkdirat, symlinkat, glnx_opendirat, glnx_shutil_rm_rf_at,
glnx_file_replace_contents_at, flatpak_cp_a_at) anchored to a
base_dfd opened from the build directory.
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-8qxj-x646-phcm
We have two use cases for removing refs: uninstalling, and pruning of
undeployed refs. Pruning undeployed refs is something we want anyone to
be able to do, because they can also pull updates and then not deploy
them. Uninstalling arbitrary refs on the other hand is problematic, and
its possible to remove the AppStream ref, and a deployed ref.
So we split removing a ref into a function which removes any ref, and
use it internally to implement e.g. uninstalling, and a function to
remove refs which are safe to remove. Only the latter one will escalate
through the system helper, making it harder for unprivileged users to
get into a state which can be exploited.
See the previous commit for such a state.
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-q4gr-vc25-57m5
Hardlink targets were not rebased to the destination directory,
allowing a crafted archive to hardlink arbitrary host files into the
extraction directory.
Also improve the documentation on how this all works, because it is all
a bit subtle.
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-9rww-v4mm-x4jg
Subpaths passed via D-Bus to the system helper are not validated, so
a value containing ".." components is accepted.
The only reason why this isn't exploitable is because we resolve the
path relative to `root` via g_file_resolve_relative_path, where root is
a OstreeRepoFile which will return FALSE when it encounters ".." or
symlinks.
Harden the code by using glnx_chase_and_mkdirat with RESOLVE_BENEATH to
create parent directories, and check out into the resulting fd.
The paths up to files/ are not attacker-controlled, and
g_file_replace_contents with G_FILE_CREATE_REPLACE_DESTINATION already
replaces symlinks rather than following them, so this is not a security
fix. The only reason why is that the file is in the root of the attacker
controlled directory.
This is a bit subtle, so lets do the right thing and concistently use
fd-relative operations when doing anything inside attacker/app
controlled directories.
Helps: https://github.com/flatpak/flatpak/security/advisories/GHSA-w69g-9x8j-7p8f
Extension content is controlled by the extension developer. When setting
up the sandbox, Flatpak accesses the .ref file and iterates merge_dirs
within the extension's files directory on the host using path-based
operations that follow symlinks. A malicious extension can place symlinks
that escape the extension directory, causing Flatpak's host process to
access arbitrary host paths. This discloses host directory listings to
the sandboxed application via the resulting merge symlinks.
Use glnx_chaseat with GLNX_CHASE_RESOLVE_BENEATH to confine path
resolution to the extension's files directory.
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-w69g-9x8j-7p8f
Validate extension directory, subdir_suffix, add_ld_path, and
merge_dirs at parse time, rejecting absolute paths and paths with
.. components that escape the intended mount point. This prevents
extension metadata from mounting content at unintended locations
inside the sandbox.
Helps: https://github.com/flatpak/flatpak/security/advisories/GHSA-w69g-9x8j-7p8f
Use glnx_chaseat to ensure that all the attacker-controlled paths end up
inside the basefd directory, and then AT_SYMLINK_NOFOLLOW to make sure
that the last path component won't escape from the basefd.
On kernels >= 6.6 we can now use fchmodat() with AT_SYMLINK_NOFOLLOW,
but on older kernels that didn't work, so if necessary fall back to
opening the file with O_NOFOLLOW and then calling fchmod() on it.
Because this is the last syscall that used the previous (flawed)
validation mechanism, we can now remove the validation helpers and be
sure that everything is using the glnx-chaseat()-based replacements.
[smcv: Separated from a larger commit for better reviewability]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-qrwq-7qwx-q9rp
Use glnx_chaseat to ensure that the attacker-controlled symlink name
ends up inside the basefd directory. Note that the symlink *target* is
also attacker-controlled, but it's OK for them to be able to set any
target of their choice: that can't immediately cause traversal outside
the base directory.
[smcv: Separated from a larger commit for better reviewability]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Helps: https://github.com/flatpak/flatpak/security/advisories/GHSA-qrwq-7qwx-q9rp
Again, use glnx_chaseat to ensure that all the attacker-controlled paths
end up inside the basefd directory. These two syscalls are a bit more
complicated, and take two paths.
[smcv: Separated from a larger commit for better reviewability]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Helps: https://github.com/flatpak/flatpak/security/advisories/GHSA-qrwq-7qwx-q9rp
Use glnx_chaseat to ensure that all the attacker-controlled paths end up
inside the basefd directory.
This requires some new helper functions, which will be used in more
complicated syscalls' implementations in subsequent commits.
[smcv: Separated from a larger commit for better reviewability]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Helps: https://github.com/flatpak/flatpak/security/advisories/GHSA-qrwq-7qwx-q9rp
We pass them on to internal functions which assume that they are valid,
and specifically also create paths which contain those strings which can
be used for path traversal attacks.
Let's simply consistently validate all of those arguments.
Resolves: https://github.com/flatpak/flatpak/security/advisories/GHSA-v2gw-v9h5-9q4x
[backported to 1.18.x: previous incomplete validity checks weren't on this branch]
Sebastian Wick