While checking the validity of a `GVariantTypeInfo` is good, this code
path is very hot, and I’ve never seen these assertions catch a bug in
practice.
Lean more towards the performance side of the performance/correctness
tradeoff in this case, by removing the assertions here.
They remain in place in a number of other `GVariantTypeInfo` code paths,
so invalid `GVariantTypeInfo` pointers should hopefully still be caught
quickly.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
These functions were previously calling
`g_variant_serialised_n_children()` twice just to validate the input, in
the case that the input was a serialised variant.
That’s not necessary, and checking the number of children in a
serialised variant is not necessarily cheap.
Move the checks around so that the number of children is only checked
once on each code path. This doesn’t introduce any functional changes.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
As with commit 9ae59bd647882bcb33103331255a5149d2fb90d2, deeply nested
maybes in an array can be exploited by a malicious caller to cause a
geometric increase in processing time and number of `GVariant` instances
handled by the `g_variant_print()` code.
Optimise this by skipping recursing through most of the chain of maybes,
thus avoiding all the setup checks in each recursive call.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
oss-fuzz#54577
When printing a `GVariant`.
This introduces no functional changes, but should speed things up a
little bit when printing out arrays.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fix a regression that appeared after adding support for nanosecond
timestamps to set_mtime_atime(). User-visible effect: when copying a
file from a gvfs MTP mountpoint to the local filesystem, the file's
mtime is set to 0.
This behavior happens when setting G_FILE_ATTRIBUTE_TIME_MODIFIED first,
then G_FILE_ATTRIBUTE_TIME_MODIFIED_USEC. Setting the second attribute
ends up in set_mtime_atime() with mtime_usec_value set, and mtime_value
== NULL. When mtime_value is NULL, the tv_sec part of the timestamp
should be fetched by lazy_stat(), but set_mtime_atime() fails to assign
it properly, and tv_sec stays at 0, leading to losing the main part of
the timestamp.
Fix the issue by setting times_n[1].tv_sec to the value fetched from
lazy_stat().
Fixes: b33ef610deef ("Add functionality to preserve nanosecond timestamps")
Fixes: 15cb123c824c ("glocalfileinfo: don't call both utimes and utimensat")
Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
When launching a registered handler we compose the command-line
string using the registered command-line template. Applications
expect files in their command-line as local paths rather than
complete URI strings.
For example,
"Program.exe" "%1"
Should expand to
"Program.exe" "C:\file.dat"
Rather than
"Program.exe" "file:///C:\file.dat"
Fixes https://gitlab.gnome.org/GNOME/glib/-/issues/2843
Backport !3163 “gvariant: Check offset table doesn’t fall outside variant bounds and speed up text parsing” to glib-2-74
See merge request GNOME/glib!3164
If a variant is trusted, that means all its children are trusted, so
ensure that their checked offsets are set as such.
This allows a lot of the offset table checks to be avoided when getting
children from trusted serialised tuples, which speeds things up.
No unit test is included because this is just a performance fix. If
there are other slownesses, or regressions, in serialised `GVariant`
performance, the fuzzing setup will catch them like it did this one.
This change does reduce the time to run the oss-fuzz reproducer from 80s
to about 0.7s on my machine.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2841
oss-fuzz#54314
When dereferencing the first entry in the offset table for a tuple,
check that it doesn’t fall outside the bounds of the variant first.
This prevents an out-of-bounds read from some non-normal tuples.
This bug was introduced in commit 73d0aa81c2575a5c9ae77d.
Includes a unit test, although the test will likely only catch the
original bug if run with asan enabled.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2840
oss-fuzz#54302
musl doesn’t define them itself, presumably because they’re not defined
in POSIX. glibc does define them. Thankfully, the values used in glibc
match the values used internally in other musl macros.
Define the values as a fallback. As a result of this, we can get rid of
the `g_assert_if_reached()` checks in `siginfo_t_to_wait_status()`.
This should fix catching signals from a subprocess when built against
musl.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2852
On some systems only `futex_time64` exists (e.g. riscv32) while on
others only `futex` exists (old Linux, 64 bit platforms), so it is
necessary to check for both and try calling both at runtime.
Additionally use the correct `struct timespec` definition. There is not
necessarily any relation between the libc's definition and the kernel's.
Specifically, the libc headers might use 64-bit `time_t` while the kernel
headers use 32-bit `__kernel_old_time_t` on certain systems.
To get around this problem we
a) check if `futex_time64` is available, which only exists on 32-bit
platforms and always uses 64-bit `time_t`.
b) otherwise (or if that returns `ENOSYS`), we call the normal `futex`
syscall with the `struct timespec` used by the kernel, which uses
`__kernel_long_t` for both its fields. We use that instead of
`__kernel_old_time_t` because it is equivalent and available in the
kernel headers for a longer time.
The array of offsets is little-endian, even on big-endian architectures
like s390x.
Fixes: ade71fb5 "gvariant: Don’t allow child elements to overlap with each other"
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/2839
Signed-off-by: Simon McVittie <smcv@collabora.com>
If it takes one more `GMainContext` cycle than expected for the
`activate` signals to be handled, the `GApplication` under test can be
released too early, and the test will fail due to not seeing a high
enough value of `n_activations`.
Hopefully avoid that by moving the release to a low priority idle
callback.
This fix is only hopeful because I’ve only been able to reproduce the
failure on FreeBSD CI and not locally.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2835
This solves problems with validating untrusted inputs from D-Bus, where
invalid numbers of added and removed menu entries, and positions, could
be specified.
Original patch from
https://bugzilla.gnome.org/show_bug.cgi?id=728733#c7, tweaked by Philip
Withnall to add a few code comments and make
`G_MENU_EXPORTER_MAX_SECTION_SIZE` public so callers can check their
inputs against it if they want. Also tweaked to use `g_warning()` instead
of the nonexistent `g_dbus_warning()`.
Backport 2.74: Made the new public symbol internal-only to avoid adding
new API in a stable release series.
Fixes: #861
This avoids needing to always serialise a variant before byteswapping it.
With variants in non-normal forms, serialisation can result in a large
increase in size of the variant, and a lot of allocations for leaf
`GVariant`s. This can lead to a denial of service attack.
Avoid that by changing byteswapping so that it happens on the tree form
of the variant if the input is in non-normal form. If the input is in
normal form (either serialised or in tree form), continue using the
existing code as byteswapping an already-serialised normal variant is
about 3× faster than byteswapping on the equivalent tree form.
The existing unit tests cover byteswapping well, but need some
adaptation so that they operate on tree form variants too.
I considered dropping the serialised byteswapping code and doing all
byteswapping on tree-form variants, as that would make maintenance
simpler (avoiding having two parallel implementations of byteswapping).
However, most inputs to `g_variant_byteswap()` are likely to be
serialised variants (coming from a byte array of input from some foreign
source) and most of them are going to be in normal form (as corruption
and malicious action are rare). So getting rid of the serialised
byteswapping code would impose quite a performance penalty on the common
case.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2797
If `g_variant_byteswap()` was called on a non-normal variant of a type
which doesn’t need byteswapping, it would return a non-normal output.
That contradicts the documentation, which says that the return value is
always in normal form.
Fix the code so it matches the documentation.
Includes a unit test.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2797
The entries in an offset table (which is used for variable sized arrays
and tuples containing variable sized members) are sized so that they can
address every byte in the overall variant.
The specification requires that for a variant to be in normal form, its
offset table entries must be the minimum width such that they can
address every byte in the variant.
That minimality requirement was not checked in
`g_variant_is_normal_form()`, leading to two different byte arrays being
interpreted as the normal form of a given variant tree. That kind of
confusion could potentially be exploited, and is certainly a bug.
Fix it by adding the necessary checks on offset table entry width, and
unit tests.
Spotted by William Manley.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2794
This improves a slow case in `g_variant_get_normal_form()` where
allocating many identical default values for the children of a
variable-sized array which has a malformed offset table would take a lot
of time.
The fix is to make all child values after the first invalid one be
references to the default value emitted for the first invalid one,
rather than identical new `GVariant`s.
In particular, this fixes a case where an attacker could create an array
of length L of very large tuples of size T each, corrupt the offset table
so they don’t have to specify the array content, and then induce
`g_variant_get_normal_form()` into allocating L×T default values from an
input which is significantly smaller than L×T in length.
A pre-existing workaround for this issue is for code to call
`g_variant_is_normal_form()` before calling
`g_variant_get_normal_form()`, and to skip the latter call if the former
returns false. This commit improves the behaviour in the case that
`g_variant_get_normal_form()` is called anyway.
This fix changes the time to run the `fuzz_variant_binary` test on the
testcase from oss-fuzz#19777 from >60s (before being terminated) with
2.3GB of memory usage and 580k page faults; to 32s, 8.3MB of memory
usage and 1500 page faults (as measured by `time -v`).
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2540
oss-fuzz#19777
This is equivalent to what `GVariantIter` does, but it means that
`g_variant_deep_copy()` is making its own `g_variant_get_child_value()`
calls.
This will be useful in an upcoming commit, where those child values will
be inspected a little more deeply.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2121
Building a `GVariant` using entirely random data may result in a
non-normally-formed `GVariant`. It’s always possible to read these
`GVariant`s, but the API might return default values for some or all of
their components.
In particular, this can easily happen when randomly generating the
offset tables for non-fixed-width container types.
If it does happen, bytewise comparison of the parsed `GVariant` with the
original bytes will not always match. So skip those checks.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2121
The past few commits introduced the concept of known-good offsets in the
offset table (which is used for variable-width arrays and tuples).
Good offsets are ones which are non-overlapping with all the previous
offsets in the table.
If a bad offset is encountered when indexing into the array or tuple,
the cached known-good offset index will not be increased. In this way,
all child variants at and beyond the first bad offset can be returned as
default values rather than dereferencing potentially invalid data.
In this case, there was no information about the fact that the indexes
between the highest known-good index and the requested one had been
checked already. That could lead to a pathological case where an offset
table with an invalid first offset is repeatedly checked in full when
trying to access higher-indexed children.
Avoid that by storing the index of the highest checked offset in the
table, as well as the index of the highest good/ordered offset.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2121
This is similar to the earlier commit which prevents child elements of a
variable-sized array from overlapping each other, but this time for
tuples. It is based heavily on ideas by William Manley.
Tuples are slightly different from variable-sized arrays in that they
contain a mixture of fixed and variable sized elements. All but one of
the variable sized elements have an entry in the frame offsets table.
This means that if we were to just check the ordering of the frame
offsets table, the variable sized elements could still overlap
interleaving fixed sized elements, which would be bad.
Therefore we have to check the elements rather than the frame offsets.
The logic of checking the elements up to the index currently being
requested, and caching the result in `ordered_offsets_up_to`, means that
the algorithmic cost implications are the same for this commit as for
variable-sized arrays: an O(N) cost for these checks is amortised out
over N accesses to O(1) per access.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2121
This reduces a few duplicate calls to `g_variant_type_info_query()` and
explains why they’re needed.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2121
If different elements of a variable sized array can overlap with each
other then we can cause a `GVariant` to normalise to a much larger type.
This commit changes the behaviour of `GVariant` with non-normal form data. If
an invalid frame offset is found all subsequent elements are given their
default value.
When retrieving an element at index `n` we scan the frame offsets up to index
`n` and if they are not in order we return an element with the default value
for that type. This guarantees that elements don't overlap with each
other. We remember the offset we've scanned up to so we don't need to
repeat this work on subsequent accesses. We skip these checks for trusted
data.
Unfortunately this makes random access of untrusted data O(n) — at least
on first access. It doesn't affect the algorithmic complexity of accessing
elements in order, such as when using the `GVariantIter` interface. Also:
the cost of validation will be amortised as the `GVariant` instance is
continued to be used.
I've implemented this with 4 different functions, 1 for each element size,
rather than looping calling `gvs_read_unaligned_le` in the hope that the
compiler will find it easy to optimise and should produce fairly tight
code.
Fixes: #2121
