Return `G_VARIANT_PARSE_ERROR_RECURSION` from `g_variant_parse()` if a
typedecl is found within a text-form variant which would cause any part
of the variant to exceed the maximum allowed recursion/nesting depth.
This fixes an oversight when `G_VARIANT_MAX_RECURSION_DEPTH` was
implemented, which allowed typedecls to effectively multiply the size of
an array if `g_variant_parse()` was parsing a text-form variant without
a top-level concrete type specified.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #2782
oss-fuzz#49462
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
Change a condition from one to an equivalent one to shut up a
`scan-build` warning about potentially dereferencing a `NULL` value.
This introduces no functional changes, as it’s not actually possible to
dereference a `NULL` value here (but `scan-build` can’t link the
nullability of `error` to the nullability of `result`).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The token parsing done by g_variant_parse() uses recursive function
calls, so at some point it will hit the stack limit. As with previous
changes to `GVariantType` parsing (commit 7c4e6e9fbe), limit the level
of nesting of containers parsed by g_variant_parse() to something
reasonable. We guarantee 64 levels of nesting, which should be enough
for anyone, and is the same as what we guarantee for types.
oss-fuzz#10286
Signed-off-by: Philip Withnall <withnall@endlessm.com>
These squash various warnings from `scan-build`. None of them are
legitimate bugs, but some of them do improve code readability a bit.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #1767
glib/gvariant-parser.c: In function ‘number_get_value’:
glib/gvariant-parser.c:1924:46: error: operand of ?: changes signedness from ‘int’ to ‘guint64’ {aka ‘long unsigned int’} due to unsignedness of other operand [-Werror=sign-compare]
return g_variant_new_int16 (negative ? -((gint16) abs_val) : abs_val);
^~~~~~~~~~~~~~~~~~~
glib/gvariant-parser.c:1934:46: error: operand of ?: changes signedness from ‘int’ to ‘guint64’ {aka ‘long unsigned int’} due to unsignedness of other operand [-Werror=sign-compare]
return g_variant_new_int32 (negative ? -((gint32) abs_val) : abs_val);
^~~~~~~~~~~~~~~~~~~
glib/gvariant-parser.c:1944:46: error: operand of ?: changes signedness from ‘long int’ to ‘guint64’ {aka ‘long unsigned int’} due to unsignedness of other operand [-Werror=sign-compare]
return g_variant_new_int64 (negative ? -((gint64) abs_val) : abs_val);
^~~~~~~~~~~~~~~~~~~
glib/gvariant-parser.c:1954:47: error: operand of ?: changes signedness from ‘int’ to ‘guint64’ {aka ‘long unsigned int’} due to unsignedness of other operand [-Werror=sign-compare]
return g_variant_new_handle (negative ? -((gint32) abs_val) : abs_val);
^~~~~~~~~~~~~~~~~~~
glib/gvariant-parser.c: In function ‘g_variant_parse_error_print_context’:
glib/gvariant-parser.c:2785:17: error: comparison of integer expressions of different signedness: ‘gint’ {aka ‘int’} and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
if (point >= strlen (source_str))
^~
Previously pattern_coalesce incorrectly concluded that maybe type is not
present when one pattern starts with `M` and other pattern with anything
else than `M` or `m`. This is false when the other pattern is `*`, since
it includes the maybe type.
When parsing GVariant text format strings, we do a limited form of type
inference. The algorithm for type inference for nested array child types
is not complete, however (and making it complete, at least with a naive
implementation, would make it O(N^2), which is not worth it) and so some
text format arrays were triggering an assertion failure in the error
handling code.
Fix that by making the error handling code a little more relaxed, in the
knowledge that our type inference algorithm is not complete. See the
comment added to the code.
This includes a test case, provided by oss-fuzz.
oss-fuzz#11578
Signed-off-by: Philip Withnall <withnall@endlessm.com>
And add tests.
There wasn’t actually a bug on x86_64 before, but it was making use of
undefined behaviour, and hence triggering ubsan warnings. Make the code
more explicit, and avoid undefined behaviour.
oss-fuzz#12686
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Rather than prefixing unsigned numbers with unary minus operators and
expecting the implicit cast to carry the correct value through, add an
explicit cast to a signed type before the unary minus is applied.
In all four cases, an overflow check has already been done.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://gitlab.gnome.org/GNOME/glib/issues/1655
When parsing an escaped Unicode character in a text format GVariant
string, such as '\U0001F415', the code uses g_ascii_strtoull(). This,
unexpectedly, accepts minus signs, which can cause an assertion failure
when input like '\u-FF4' is presented for parsing.
Validate that there are no leading sign characters when parsing.
This shouldn’t be considered a security bug, because the GVariant text
format parser should not be used on untrusted input.
oss-fuzz#11576
Signed-off-by: Philip Withnall <withnall@endlessm.com>
token_stream_prepare() was over-reading at the start of bytestring
literals (`b'blah'`).
Add tests for that, and for some other situations regarding bytestring
literal parsing, in order to try and get full branch coverage of that
bit of code.
oss-fuzz#9805
Signed-off-by: Philip Withnall <withnall@endlessm.com>
The token_stream_peek() functions were not doing any bounds checking, so
could potentially read 1 byte off the end of the input blob. This was
never noticed, since the input stream is almost always a nul-terminated
string. However, g_variant_parse() does allow non-nul-terminated strings
to be used with a @limit parameter, and the bugs become apparent under
valgrind if that parameter is used.
This includes modifications to the test cases to cover the
non-nul-terminated case.
Spotted by ossfuzz.
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.
The scanning to find the end of a positional parameter designator in
GVariant text format (e.g. '%i') is currently broken in case the 'end'
pointer is not specified.
The scan is controlled by a somewhat complicated loop that needs to deal
properly with cases like (123, %(ii)) [where '%(ii)' is to be taken
together, but the final ')' not].
This loop missed the case where a format string passed to
g_variant_new_parsed() ended immediately after such a conversion, with a
nul character. In this case the 'end' pointer is NULL, so the only way
we can find the end is by scanning for nul in the string.
In case of g_variant_new_parsed() [which is what this code was designed
to be used for], the bug is somewhat unlikely in practice: the only way
that a valid text-form GVariant could ever contain a positional
parameter replacement at the end of the string is if this positional
parameter were the only thing being returned. In that case, the user
would likely have opted for a more direct approach.
Unfortunately, this code is also active in the tokenisation phase of
g_variant_parse(), before positional parameters are rejected as invalid
for that case. Anyone who calls this function with a nul-terminated
string (and no end pointer) is vulnerable to a crash from malicious user
input. This can be seen, at the very least with many commandline tools:
$ dconf write /x '%i'
Segmentation fault
We fix this problem by searching for the nul character in this case, in
addition to comparing the end pointer.
This problem is almost certainly limited to being able to cause crashes.
The loop in question only performs reads and, in the security-sensitive
case, the token will be quickly rejected after the loop is finished
(since it starts with '%' and the 'app' pointer is unset). This is
further mitigated by the fact that there are no known cases of GVariant
text format being used as part of a protocol at a privilege barrier.
Unlike, say, g_variant_new(), which returns a floating reference.
g_variant_parse() returns a non-floating one, so must always have
g_variant_unref() called on the result.
Slightly expand on the documentation about casting varargs when
constructing GVariants, and link to it from all the functions where it’s
a necessary consideration.
Add an example of passing flags to a ‘t’ type variable (guint64).
Assuming the flags enum does not have many members, the flag variable
will be 32 bits wide, and needs an explicit cast to be passed into
g_variant_new() as a 64-bit value.
https://bugzilla.gnome.org/show_bug.cgi?id=712837
This was a feature intended from the very beginning that somehow never
got written. It's a way to replace these sort of error messages out of
the GVariant parser:
1-2,10-15:unable to find a common type
with something in the style of the Vala compiler:
unable to find a common type:
[1, 2, 3, 'str']
^ ^^^^^
https://bugzilla.gnome.org/show_bug.cgi?id=715028
Most GErrors, such as GSomethingError, have a function to get
their quark that looks like g_something_error_quark(),
so bindings (such as gtkmm) would expect GVariantParseError
to have g_variant_parse_error_quark(). Instead this had
g_variant_parser_get_error_quark().
This deprecates the old function and adds the correct one,
making life easier for gtkmm (and maybe others).
https://bugzilla.gnome.org/show_bug.cgi?id=708212
Back in the far-off twentieth century, it was normal on unix
workstations for U+0060 GRAVE ACCENT to be drawn as "‛" and for U+0027
APOSTROPHE to be drawn as "’". This led to the convention of using
them as poor-man's ‛smart quotes’ in ASCII-only text.
However, "'" is now universally drawn as a vertical line, and "`" at a
45-degree angle, making them an `odd couple' when used together.
Unfortunately, there are lots of very old strings in glib, and also
lots of new strings in which people have kept up the old tradition,
perhaps entirely unaware that it used to not look stupid.
Fix this by just using 'dumb quotes' everywhere.
https://bugzilla.gnome.org/show_bug.cgi?id=700746
Parsing wrongly-typed GVariant text format data is a well-defined
operation and it ought to result in a GError. We do that for most
cases, but 'v' and 'ay' were being treated differently. Fix those as
well.
This allows compilation with clang without errors, even when
-Wformat-nonliteral is active (as long as there are no real cases of
non literal formatting).
https://bugzilla.gnome.org/show_bug.cgi?id=691608
Strings matching /%[a-z]/ are special syntax for gtk-doc.
Bug: https://bugzilla.gnome.org/show_bug.cgi?id=632049
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Ryan Lortie <desrt@desrt.ca>
