I noticed this when running the test on an Arm Morello system where varargs
have bounds. g_variant_new() was trying to read an integer using va_arg(),
but since there was no argument it resulted in a bounds errors there.
On most other architectures this will just read whatever value is contained
in the next argument register and is not something that ASan can detect, so
it never resulted in test failures.
posix_memalign() requires the alignment to be a multiple of sizeof(void*),
and a power of 2. Passing 8 does not fulfil both of those constraints on
Arm Morello which resulted in a "posix_memalign failed" test failure.
Co-authored-by: Graeme Jenkinson <graeme@capabilitieslimited.co.uk>
When a cancellable is cancelled when we call g_cancellable_connect we
used to immediately call the provided callback, while this is fine we
actually had race in case the cancellable was about to be reset or in
the middle of a cancellation.
In fact it could happen that when we released the mutex, another thread
could reset the cancellable just before the callback is actually called
and so leading to call it with g_cancellable_cancelled() == FALSE.
So to handle this, make disconnect and reset function to wait for
connection emission to finish, not to break their assumptions.
This can be tested using some "brute-force" tests where multiple threads
are racing to connect and disconnect while others are cancelling and
resetting a cancellable, ensuring that all works as we expect.
We may need to avoid using a cached temp directory for testing purposes,
so let's provide an internal API to perform such task.
This implies removing GOnce and going with mutex-based version, but
that's still using atomic logic in most unix implementations anyways.
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()`.
Fixes: #861
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 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
The following few commits will add a couple of new fields to
`GVariantSerialised`, and they should be zero-filled by default.
Try and pre-empt that a bit by zero-filling `GVariantSerialised` by
default in a few places.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2121
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 timeout is just to stop the test hanging forever, so there’s no need
for it to be so short. It’s caused at least one spurious CI failure:
https://gitlab.gnome.org/GNOME/glib/-/jobs/2445023.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2837
This reverts commit 965061797d.
We are having trouble tracking down the cause of #2769. When the bug
occurs, we fail to set scheduler settings for the new thread pool
thread. This can have serious consequences and should not be ignored. In
retrospect, making this a critical instead of a fatal error has made it
more difficult to notice, debug, and fix. This operation needs to always
work, so let's crash when it fails.
This does not fix#2769, but will hopefully help.
