This introduces no real functional changes (except when compiling with
G_DISABLE_ASSERT, in which case it fixes the test). Mostly just a code
cleanup.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The code was checking whether the signature provided by the blob was a
valid D-Bus signature — but that’s a superset of a valid GVariant type
string, since a D-Bus signature is zero or more complete types. A
GVariant type string is exactly one complete type.
This meant that a D-Bus message with a header field containing a variant
with an empty type signature (for example) could cause a critical
warning in the code parsing it.
Fix that by checking whether the string is a valid type string too.
Unit test included.
oss-fuzz#9810
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Parsing a D-Bus message with the signature field in the message header
of type other than ‘g’ (GVariant type signature) would cause a critical
warning. Instead, we should return a runtime error.
Includes a test.
oss-fuzz#9825
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When validating a string to see if it’s valid UTF-8, we pass a gsize to
g_utf8_validate(), which only takes a gssize. For large gsize values,
this will result in the gssize actually being negative, which will
change g_utf8_validate()’s behaviour to stop at the first nul byte. That
would allow subsequent nul bytes through the string validator, against
its documented behaviour.
Add a test case.
oss-fuzz#10319
Signed-off-by: Philip Withnall <withnall@endlessm.com>
As with the previous commit, when getting a child from a serialised
tuple, check its offset against the length of the serialised data of the
tuple (excluding the length of the offset table). The offset was already
checked against the length of the entire serialised tuple (including the
offset table) — but a child should not be able to start inside the
offset table.
A test is included.
oss-fuzz#9803
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When getting a child from a serialised variable array, check its offset
against the length of the serialised data of the array (excluding the
length of the offset table). The offset was already checked against the
length of the entire serialised array (including the offset table) — but a
child should not be able to start inside the offset table.
A test is included.
oss-fuzz#9803
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Previously, GVariant has allowed ‘arbitrary’ recursion on GVariantTypes,
but this isn’t really feasible. We have to deal with GVariants from
untrusted sources, and the nature of GVariantType means that another
level of recursion (and hence, for example, another stack frame in your
application) can be added with a single byte in a variant type signature
in the input. This gives malicious input sources far too much leverage
to cause deep stack recursion or massive memory allocations which can
DoS an application.
Limit recursion to 128 levels (which should be more than enough for
anyone™), document it and add a test. This is, handily, also the limit
of 64 applied by the D-Bus specification (§(Valid Signatures)), plus a
bit to allow wrapping of D-Bus messages in additional layers of
variants.
oss-fuzz#9857
Signed-off-by: Philip Withnall <withnall@endlessm.com>
When checking whether a serialised GVariant tuple is in normal form,
it’s possible for `offset_ptr -= offset_size` to underflow and wrap
around, resulting in gvs_read_unaligned_le() reading memory outside the
serialised GVariant bounds.
See §(Tuples) in gvariant-serialiser.c for the documentation on how
tuples are serialised. Briefly, all variable-length elements in the
tuple have an offset to their end stored in an array of offsets at the
end of the tuple. The width of each offset is in offset_size. offset_ptr
is added to the start of the serialised tuple to get the offset which is
currently being examined. The offset array is in reverse order compared
to the tuple elements, hence the subtraction.
The bug can be triggered if a tuple contains a load of variable-length
elements, each of whose length is actually zero (i.e. empty arrays).
Includes a unit test.
oss-fuzz#9801
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The CI image which this glib-2-56 branch was previously using has been
deleted from the registry (master has moved on). Tweak the CI
configuration to use the glib-2-58 image, since there doesn’t seem to be
much point in adding an image just for glib-2-56, given how little
development is still needed on this branch.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
`read_netlink_messages()` is the callback attached to the netlink socket
(G_IO_IN). It calls `g_socket_receive_message()`. There is a race
condition that if the socket is closed while there is a pending call, we
will try to receive on a closed socket, which fails.
To avoid this, we switch the order of the operations around: first
destroy the source and then close the socket.
This is not a correct way to check if `g_socket_new_from_fd()` failed.
Instead just see if it returned `NULL` itself.
This was preventing the netlink monitor from being initialised.
Closes#1518
If something which looks like a closing tag is left unfinished, but
isn’t paired to an opening tag in the document, the error handling code
would do a null pointer dereference. Avoid that, at the cost of
introducing a new translatable error message.
Includes a test case, courtesy of pdknsk.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1461
(cherry picked from commit fccef3cc822af74699cca84cd202719ae61ca3b9)
When formatting the error messages for markup parsing errors, the parser
was unconditionally reading a UTF-8 character from the input buffer —
but the buffer might end with a partial code sequence, resulting in
reading off the end of the buffer by up to three bytes.
Fix this and add a test case, courtesy of pdknsk.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1462
(cherry picked from commit cec71705406f0b2790422f0c1aa0ff3b4b464b1b)
The source callback for a GCancellable should have the cancellable itself
as first argument.
This was not the case, and when this code was hit, we were instead trying
to treat the pointer as a CommunicateState reference and thus wrongly
deferencing it, causing a memory error and a crash.
7efd76dd6796f8 added these configure time tests to work around a bug
with older Android. Since the test didn't take Windows into account it
wrongfully applied the workaround on Windows too, breaking the build.
With meson this wasn't an issue since the check is skipped on Windows there
and our CI didn't catch this issue.
Change the test to run on Android only for meson and autotools.
This also makes it clear that the test+code can be dropped again if we stop
supporting older Android versions at some point.
These functions may be OK to leave the G_GNUC_MALLOC attribute on,
because the only valid pointers in the storage areas they return are,
themselves, new pointers.
However, it’s a lot easier to remove the attributes now than to try and
diagnose miscompilations in future. The performance impact of this is
likely to be unmeasurable. If there are performance problems caused by
this, then they can be profiled and fixed case-by-case in future,
bearing in mind the possibility for miscompilation if G_GNUC_MALLOC is
readded.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1465
These are our most critically incorrect uses of G_GNUC_MALLOC. See the
previous commit for details.
(Backport to 2.56: Drop the grcbox.h changes.)
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1465
Thanks to some great investigation by Benjamin Moody, it’s clear that
our documentation and usage of G_GNUC_MALLOC has fallen behind GCC’s
interpretation of the malloc attribute, meaning that recent versions of
GCC could miscompile code which uses G_GNUC_MALLOC incorrectly.
Update the documentation of G_GNUC_MALLOC to match the current GCC
documentation (for GCC 8.2). Following commits will drop our use of
G_GNUC_MALLOC from inappropriate functions.
Specifically, the change in GCC’s interpretation of the malloc attribute
which could cause miscompilation is that returned storage areas are now
assumed to not contain valid pointers — so realloc() cannot have the
malloc attribute, and neither can a function which returns a newly
allocated structure with fields initialised to other pointers.
(Backport to 2.56: Change the documentation to mention GLib 2.56.3
rather than GLib 2.58.)
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1465
It’s consistently timing out on the CI, which makes sense since it does
333 iterations of spawning a subprocess. Give it a bit more breathing
room.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The network monitor portal interface is changing.
Version 2 is no longer using properties, but getters
instead (this lets the portal apply access control
and avoid sending information to non-networked
sandboxes).
To support both version 1 and 2 of the interface,
we stop using generated code and instead deal with
the api differences in our own code, which is not
too difficult.
Support version 1 as well
When using g_network_monitor_get_default() from another thread, it’s
possible for network-changed events to be processed after an instance of
GNetworkMonitor has been disposed, causing use-after-free problems.
Fix that by moving some of the initialisation into the GInitable.init()
chain, rather than in a main context idle callback.
This includes a unit test which probabilistically reproduces the bug
(but can’t do so deterministically due to it being a race condition).
Commit amended by Philip Withnall <withnall@endlessm.com> before
pushing.
https://bugzilla.gnome.org/show_bug.cgi?id=793727
While mountpoints are *not* symlinks, strictly speaking,
they works in a similar enough way, so consider them to be
symlinks for the purpose of querying local file info.
On Windows st_ctime field is the file creation time.
POSIX mandates that field to be the file state change time.
Naturally, glib code interpreted st_ctime as POSIX suggested,
and the result was bad.
Fix this by introducing special W32-only logic for setting
attributes from st_ctime field.
Fixes issue #1452.
Releasing GVolumeMonitor before g_volume_mount finish cause that
g_volume_get_mount returns NULL, because the mount is not correctly
propagated to the volume.
(Backported from commit 88b8ebb5dde0512fd1e098efe4c217111876d252 with
minor merge conflicts.)
https://gitlab.gnome.org/GNOME/glib/issues/1458
