scan-build thinks that `gvs_variable_sized_array_is_normal()` can do a
`NULL` pointer dereference on `value.data` when `value.size == 0`. This
isn’t possible, because `offsets.length == 0` always when `value.size ==
0`, but that’s a bit of a complex relationship which the static analyser
can’t work out.
Give it some help by adding an assertion.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
When piecewise validating the offset table for a variable sized array,
it’s possible that the offset table (`offsets.array`) won’t actually
have been set by `gvs_variable_sized_array_get_frame_offsets()` iff the
serialised `GVariant` is not in normal form.
Add an additional check to guard against this. This will result in an
empty child variant being returned, as with other error handling paths
in `gvs_variable_sized_array_get_child()`.
This is a true positive spotted by scan-build. Thanks, scan-build.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
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
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>
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
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
This should quell a scan-build error about dereferencing `member_info`
when it’s `NULL` at the end of the function, due to having zero
iterations of the `for` loop.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Add SPDX license (but not copyright) headers to all files which follow a
certain pattern in their existing non-machine-readable header comment.
This commit was entirely generated using the command:
```
git ls-files glib/*.[ch] | xargs perl -0777 -pi -e 's/\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/\n \*\n \* SPDX-License-Identifier: LGPL-2.1-or-later\n \*\n \* This library is free software; you can redistribute it and\/or\n \* modify it under the terms of the GNU Lesser General Public/igs'
```
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #1415
This fixes a bug in 7c4e6e9fbe473de0401c778c6b0c4aad27d5145a.
The original approach in that commit accidentally only checked the depth
at the leaf nodes in the variant tree, whereas actually the depth should
be checked before recursing to avoid stack overflow.
It neglected to consider that `g_variant_serialised_is_normal()` would
be recursed into by some of the `DISPATCH(_is_normal)` cases. When that
happened, the depth check was after the recursion so couldn’t prevent a
stack overflow.
Fixes: #2572
Rather than duplicating the alignment checks when constructing a new
GVariant, re-use the alignment checks from GVariantSerialised. This
ensures that the same checks are done everywhere in the GVariant code.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1342
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>
All glib/*.{c,h} files have been processed, as well as gtester-report.
12 of those files are not licensed under LGPL:
gbsearcharray.h
gconstructor.h
glibintl.h
gmirroringtable.h
gscripttable.h
gtranslit-data.h
gunibreak.h
gunichartables.h
gunicomp.h
gunidecomp.h
valgrind.h
win_iconv.c
Some of them are generated files, some are licensed under a BSD-style
license and win_iconv.c is in the public domain.
Sub-directories inside glib/:
deprecated/: processed in a previous commit
glib-mirroring-tab/: already LGPLv2.1+
gnulib/: not modified, the code is copied from gnulib
libcharset/: a copy
pcre/: a copy
tests/: processed in a previous commit
https://bugzilla.gnome.org/show_bug.cgi?id=776504
If we have an input parameter (or return value) we need to use (nullable).
However, if it is an (inout) or (out) parameter, (optional) is sufficient.
It looks like (nullable) could be used for everything according to the
Annotation documentation, but (optional) is more specific.
This probably won’t crash, as it can only happen if (size == 0), but
add a check to be safe, and to shut up the static analyser.
This case can be reached with the following call:
gvs_read_unaligned_le(NULL, 0)
which can be called from:
gvs_tuple_get_child(value, index_)
with (value.data == NULL) and (value.size == 0).
Found by scan-build.
https://bugzilla.gnome.org/show_bug.cgi?id=715164
GVariant has the concept of fixed-sized types (ie: types for which all
values of the type will have the same size). Examples are booleans,
integers, doubles, etc. Tuples containing only these types are also
fixed size.
When GVariant is trying to deal with a fixed-sized value for which it
doesn't have a sufficient backing store (eg: the case where a
fixed-sized value was created with g_variant_new_data() with an
incorrect number of bytes) it denotes this by setting the size of the
value to the correct fixed size but using a NULL data pointer.
This is well-documented in several code comments and also in the public
API documentation for g_variant_get_data() which describes the situation
number which NULL could be returned.
The decision to deal with this case in this way was changed at the last
minute around the time that GVariant was merged -- originally we had an
elaborate setup involving allocating an internal buffer of sufficient
size to be shared between all invalid values.
Unfortunately, when making this change a small detail was missed.
gvs_tuple_get_child() (the function responsible for deserialising
tuples) was updated to properly check for this case (and it contains a
comment about why it must). gvs_tuple_is_normal() (the function
responsible for verifying if a tuple is in normal form) was not.
We add the check now.
Note that this problem does not exist with any other container type
because tuples are the only container capable of being fixed-sized. All
other container types (arrays, maybes, variants) can contain a variable
number of items or items of variable types (note: we consider dictionary
entries to be two-tuples). The code for validating non-container values
also contains a check for the case of NULL data.
The problem also does not occur in the only other function dealing with
serialised tuples: gvs_tuple_n_children(). Whereas other container
types would have to inspect the serialised data to determine the number
of children, for tuples it can be determined directly from the type.
String validation was done by checking if the string was valid utf8 and
ensuring that the first non-utf8 character was the last character (ie:
the nul terminator).
No check was actually done to make sure that this byte actually
contained a nul, however, so it was possible that you could have a
string like "hello\xff" with length 6 that would correctly validate.
Fix that, and test it.
FreeBSD's malloc() sometimes returns unaligned memory if you are
requesting small sizes. This can get GVariant into trouble. For
example, consider the type "mmi" containing the value "just nothing".
According to the type signature, the memory containing this should be
aligned to a boundary of 4 since it might contain an int. The
serialised size of this value is 1 byte, however, and when you ask
FreeBSD to allocate memory of that size, it knows you can't put an int
into it so it doesn't bother aligning it.
This patch modifies the GVariant serialiser to not assert the alignment
constraint in the case that the size of the serialised data is smaller
than its own alignment requirement.
- modify serialiser validation function to enforce utf8 encoding
- add documentation to g_variant_new_string(), g_variant_get_string(),
g_variant_dup_string()
- add 2 new test cases to check that it works
Merge GVariant variable arguments support and put it under tests.
Also, remove the hack of the test case directly '#include'ing .c files
from glib/. Instead, create a non-installed gvariant-internal.h that
the tests can include and make the symbols in it visible on the symbol
table of the shared library. These symbols (as they are present in no
installed header files) are not part of the API of GLib.
Increase test coverage in a few other areas.
