If a seccomp policy is set up incorrectly so that it returns `EPERM` for
`close_range()` rather than `ENOSYS` due to it not being recognised, no
error would previously be reported from GLib, but some file descriptors
wouldn’t be closed, and that would cause a hung zombie process. The
zombie process would be waiting for one half of a socket to be closed.
Fix that by correctly propagating errors from `close_range()` back to the
parent process so they can be reported correctly.
Distributions which aren’t yet carrying the Docker fix to correctly
return `ENOSYS` from unrecognised syscalls may want to temporarily carry
an additional patch to fall back to `safe_fdwalk()` if `close_range()`
fails with `EPERM`. This change will not be accepted upstream as `EPERM`
is not the right error for `close_range()` to be returning.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2580
This code was skipping fsync on BTRFS because of an old guarantee about
the overwrite-by-rename behavior that no longer holds true. This has
been confirmed by the BTRFS developers to no longer be guaranteed since
Kernel 3.17 (August 2014), but it was guaranteed when this optimization
was first introduced in 2010.
This could result in empty files after crashes in applications using
g_file_set_contents(). Most prominently this might have been the cause
of dconf settings getting lost on BTRFS after crashes due to the
frequency with which such writes can happen in dconf.
See: https://gitlab.gnome.org/GNOME/dconf/-/issues/73
We currently dup all source fds to avoid possible conflation with the
target fds, but fail to consider that the result of a dup might itself
conflict with one of the target fds. Solve this the easy way by duping
all source_fds to values that are greater than the largest fd in
target_fds.
Fixes#2503
In case child_err_report_fd conflicts with one of the target_fds, the
code here is careful to dup child_err_report_fd in order to avoid
conflating the two. It was a good idea, but evidently was not tested,
because the newly-created fd is not created with CLOEXEC set. This means
it stays open in the child process, causing the parent to hang forever
waiting to read from the other end of the pipe. Oops!
The fix is simple: just set CLOEXEC. This removes our only usage of the
safe_dup() function, so it can be dropped.
Fixes#2506
If a path starts with more than two slashes, the `start` value was
previously incorrect:
1. As per the `g_path_skip_root()` call, `start` was set to point to
after the final initial slash. For a path with three initial
slashes, this is the character after the third slash.
2. The canonicalisation loop to find the first dir separator sets
`output` to point to the character after the first slash (and it
overwrites the first slash to be `G_DIR_SEPARATOR`).
3. At this point, with a string `///usr`, `output` points to the second
`/`; and `start` points to the `u`. This is incorrect, as `start`
should point to the starting character for output, as per the
original call to `g_path_skip_root()`.
4. For paths which subsequently include a `..`, this results in the
`output > start` check in the `..` loop below not skipping all the
characters of a preceding path component, which is then caught by
the `G_IS_DIR_SEPARATOR (output[-1])` assertion.
Fix this by resetting `start` to `output` after finding the final slash
to keep in the output, but before starting the main parsing loop.
Relatedly, split `start` into two variables: `after_root` and
`output_start`, since the variable actually has two roles in the two
parts of the function.
Includes a test.
This commit is heavily based on suggestions by Sebastian Wilhemi and
Sebastian Dröge.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
oss-fuzz#41563
Improve the performance of canonicalising filenames with many `..` or
`.` components, by modifying the path inline rather than calling
`memmove()`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2541
meson in git master now warns about a missing `check:` kwarg, and may
eventually change the default from false to true.
Take the opportunity to require `objcopy --help` to succeed -- it is
unlikely to fail, but if it does something insane happened.
We used to use a pipe for the dbus daemon stdout to read the defined
address, but that was already requiring a workaround to ensure that dbus
daemon children were then able to write to stdout.
However the current implementation is still causing troubles in some
cases in which the daemon is very verbose, leading to hangs when writing
to stdout.
As per this, just don't handle stdout ourself, but use instead a
specific pipe to get the address address. That can now be safely closed
once we've received the data we need.
This reverts commit d80adeaa96.
Fixes: #2537
This improves how we obtain the Windows release versions in
get_windows_version(), in turn g_get_os_info() for Windows 10/Server
2019 20H2 (2009) and later and Windows 11, by doing the following:
* Check the build number for non-server versions of Windows.
For Windows 11, the build number is 22000+, so we now correctly
indicate on Windows 11 that we are indeed running Windows 11.
* Check the DisplayVersion entry in the registry under
SOFTWARE\Microsoft\Windows NT\CurrentVersion if we obtained "2009"
from the ReleaseId entry, since DisplayVersion replaces ReleaseId
after Windows 10/Server 2019 20H2 (2009). This makes things more
clear for Windows releases after 20H2, where previously 20H2
and 21H1 were all identified as Windows 10 [Server] 2009.
This should fix issue #2443.
Unfortunately, we may well be likely to need to call RtlGetVersion() via
GetModuleHandle() + GetProcAddress(), so split out the call to RtlGetVersion()
into a private function of its own, so that we can reuse the same code in other
parts of GLib, so that we can:
* Determine better in a more fine-tuned way to determine whether we are on
Windows 10/11 and/or Server 2016/2019/2022, since we need to rely on the
build number.
* Just call RtlGetVersion() once, when needed, as that is all that is needed.
We could re-use the same function once to compare what we got when we
called RtlGetVersion() and do what is necessary there.
The code in `g_dbus_message_new_from_blob()` has now been fixed to
correctly error out on all truncated messages, so there’s no need for an
arbitrary programmer error if the input is too short to contain a valid
D-Bus message header.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2528
Perform strict bounds checking when reading data from the D-Bus message,
and propagate errors to the callers.
Previously, truncated D-Bus messages could cause out-of-bounds reads.
This is a security issue, but one which is only exploitable when
communicating with an untrusted peer (who might send malicious
messages). Almost all D-Bus traffic is with a session or system bus,
where the dbus-daemon or dbus-broker is trusted, and is known to have
already rejected malformed (malicious) messages.
Accordingly, this is only exploitable with peer-to-peer D-Bus
conversations with an untrusted peer.
(Includes some minor cleanups from Philip Withnall.)
oss-fuzz#17408
Fixes: #2528
