Commit ebbeb318 "gvariant: Tweak GVariantIter heap allocation size"
changed the code path that allocates iterators on the heap to use
the full size of the public iterator struct (including padding),
but didn't change the matching g_slice_free() call. g_slice_free() is
implemented as basically g_free_sized(), so the size passed to it
should match the size that was allocated.
Fixes: ebbeb318 "gvariant: Tweak GVariantIter heap allocation size"
Signed-off-by: Simon McVittie <smcv@collabora.com>
This re-works commit 608b846f38, because
it didn’t actually fix the warning on msys2-clang64.
Rather than go through contortions to try and squash that warning, just
allocate the full size struct instead. If people are worried about the
allocation size here, they really should already be using a stack
allocated GVariantIter instead.
Fixes CI failure https://gitlab.gnome.org/GNOME/glib/-/jobs/6444549.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
For reasons unknown to me, the public `GVariantIter` struct is declared
as being 16 pointers in size, but the private internal implementation of
it, `struct heap_iter`, is smaller (10 pointers).
Internally, the `GVariantIter` code casts iters around between
`GVariantIter`, `struct heap_iter` and `struct stack_iter` (which is
smaller still) quite merrily — presumably with appropriate magic checks
in place because we’ve never had a bug report about it.
However, allocating `struct heap_iter` (10 pointers) and immediately
casting it to `GVariantIter` (16 pointers) is now triggering a new
`-Walloc-size` warning in clang 22, which is fair enough.
Because C is serious about type safety, let’s go through some
contortions to squash that warning while continuing to do weird and
inadvisable things.
Diagnosed by Luca Bacci in
https://gitlab.gnome.org/GNOME/glib/-/issues/3922#note_2722553; fixes CI
failure https://gitlab.gnome.org/GNOME/glib/-/jobs/6360619:
```
FAILED: [code=1] glib/libglib-2.0-0.dll.p/gvariant.c.obj
"C:\msys64\clang64\bin/ccache.EXE" "cc" "-Iglib/libglib-2.0-0.dll.p" "-Iglib" "-I../glib" "-I." "-I.." "-fvisibility=hidden" "-fdiagnostics-color=always" "-D_FILE_OFFSET_BITS=64" "-Wall" "-Winvalid-pch" "-Wextra" "-Wpedantic" "-Werror" "-std=gnu99" "-O0" "-g" "-D_GNU_SOURCE" "-DUNICODE" "-D_UNICODE" "-fno-strict-aliasing" "-DG_ENABLE_DEBUG" "-Wfloat-conversion" "-Wimplicit-fallthrough" "-Wmisleading-indentation" "-Wmissing-field-initializers" "-Wnonnull" "-Wnull-dereference" "-Wunused" "-Wno-unused-parameter" "-Wno-cast-function-type" "-Wno-pedantic" "-Wno-format-zero-length" "-Wno-variadic-macros" "-Werror=format=2" "-Werror=init-self" "-Werror=missing-include-dirs" "-Werror=pointer-arith" "-Werror=unused-result" "-Wstrict-prototypes" "-Wno-bad-function-cast" "-Werror=implicit-function-declaration" "-Werror=missing-prototypes" "-Werror=pointer-sign" "-Wno-string-plus-int" "-Wno-typedef-redefinition" "-mms-bitfields" "-DG_LOG_DOMAIN=\"GLib\"" "-DGLIB_COMPILATION" "-DPCRE2_STATIC" -MD -MQ glib/libglib-2.0-0.dll.p/gvariant.c.obj -MF "glib/libglib-2.0-0.dll.p/gvariant.c.obj.d" -o glib/libglib-2.0-0.dll.p/gvariant.c.obj "-c" ../glib/gvariant.c
../glib/gvariant.c:2992:27: error: allocation of insufficient size '80' for type 'GVariantIter' (aka 'struct _GVariantIter') with size '128' [-Werror,-Walloc-size]
2992 | iter = (GVariantIter *) g_slice_new (struct heap_iter);
| ^
../glib/gslice.h:56:33: note: expanded from macro 'g_slice_new'
56 | #define g_slice_new(type) ((type*) g_slice_alloc (g_slice_alloc_size (type)))
| ^
1 error generated.
```
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
...and thus for generating coverage data, GIR data, and docs.
At the same time we set msys2-mingw32 as weekly-schedule or
manual.
MSYS2 has deprecated the MINGW32 environment since quite some:
https://www.msys2.org/news/#2023-12-13-starting-to-drop-some-32-bit-packages
(Backport 2.88: Minor cherry pick conflict fixes in
supported-platforms.md)
The following error has just popped up in mingw32 CI, which presumably
means the compiler has just been upgraded under our feet again:
```
../glib/gnulib/vasnprintf.c: In function '_g_gnulib_vasnprintf':
../glib/gnulib/vasnprintf.c:2429:21: error: variable 'flags' set but not used [-Werror=unused-but-set-variable=]
2429 | int flags = dp->flags;
| ^~~~~
cc1.exe: all warnings being treated as errors
```
This highlighted a bug in the version of gnulib we’re using, where the
`flags` variable was assigned but never used.
In lieu of updating our gnulib copylib version (which I don’t have the
energy to do right now; see #2603), copy across the fix for this issue
from upstream.
It’s upstream commit 5527d5c548702b89d217bbe58036996066a709b6.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
We can do this because `gdbusauthmechanism.h` is a private header.
Hook it up to the existing `reject_reason` code in each
`GDBusAuthMechanism` implementation, as all three implementations
currently intermingle reject reasons from the server and client code, so
there would currently be no benefit to having a separate server and
client implementation of `*_get_reject_reason()`.
This new private API will be used in a new unit test in the following
commit.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
The D-Bus specification says the cookie ID has to be non-negative, but
we weren’t checking that (or checking that it was non-empty).
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Without validation, the server could send a malicious context which
contains path traversal characters, allowing it to exfiltrate a SHA-1
hashed copy of arbitrary data from the client’s file system.
To exploit this successfully would require the client to choose to
connect peer-to-peer to a malicious D-Bus server and to choose the SHA-1
authentication mechanism in preference to all the other mechanisms. This
is vanishingly unlikely.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3931
This fixes the addition happening with type `uint32_t`, rather than the
expected `gssize`. Because of the narrower type, this could have
resulted in integer overflow for large header lengths, bypassing the
message length check lower down in the function.
This is not exploitable in practice unless communicating peer-to-peer
with a malicious D-Bus peer, as invalid messages like this would be
filtered out by the message bus (such as `dbus-daemon`).
Rework the arithmetic to use checked arithmetic functions into a
`size_t` (largely as suggested by Simon McVittie), and add some unit
tests. Simply doing the arithmetic in the `gssize` domain would not fix
the issue on 32-bit systems.
Based on a report by Koda Reef, albeit reverse engineered as the report
did not contain a proof of concept of the vulnerability.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3933
If this method was called on a key file key which has an empty value,
`len == 0` and this leads to a one-byte under-read off the start of the
key file buffer.
Spotted by linhlhq as #YWH-PGM9867-200. The suggested fix is theirs, and
the unit test is adapted from their report. I added the fuzzing test.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3930
If the line terminator is longer than a single byte, and the current
line extends to the end of the buffer, and the buffer (which is a
`GString`) is near a power of two in length (as that’s how `GString`s
are allocated) it’s possible for the `memcmp()` which checks the
terminator to read off the end of the string buffer.
Fix that by checking the terminator length against the last character
before calling `memcmp()`. Add a unit test.
Spotted by linhlhq as #YWH-PGM9867-199. The fix is theirs (validated by
me), and the unit test is adapted from their proof of concept.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3925
It’s easy to confuse `compile_opts` (`PCRE2_*` flags) and
`orig_compile_opts` (`GRegexCompileFlags` flags). Rename them to
`pcre2_compile_opts` and `regex_compile_opts`, respectively, to make it
a bit less confusing.
Unfortunately the type checker can’t help us here as the `PCRE2_*` flags
are defined by `pcre2` using `#define`s rather than an `enum`, so
`pcre2_compile_opts` has to stay as a `uint32_t` rather than a more
specific type.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3919
`GRegex.compile_opts` contains the options using PCRE2’s flags;
`GRegex.orig_compile_opts` contains them in `GRegexCompileFlags` format,
and that’s what we want here.
Spotted by linhlhq as #YWH-PGM9867-194. The suggested fix is theirs, and
the unit test is adapted from their report.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3919
In `G_REGEX_RAW` mode, the input string is treated as a byte array
(basically ASCII) rather than a unichar array. Accordingly, the case
changing code for substitutions needs to operate on bytes with
`G_REGEX_RAW`, rather than operating on unichars.
This fixes a potential buffer overflow when trying to do a case change
on a match of a set of bytes which are a truncated multi-byte UTF-8
encoding at the end of the input buffer.
Spotted by linhlhq as #YWH-PGM9867-193. I adapted their reproducer as
the unit test, but implemented the fix in `gregex.c` independently.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3918
Otherwise it’s possible to create a non-`NULL` but invalid `GDateTime`,
which breaks all kinds of internal assumptions.
Spotted by linhlhq as #YWH-PGM9867-191. Thanks to them for providing a
suggested fix and a test case, which I have adapted and validated.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3917
If the input string (which might be malicious, as the program is
choosing to escape it) ends with 0xc2 (a UTF-8 prefix byte), the code
wasn’t previously checking the input string length was long enough to
contain the following byte before reading it.
Fix that by switching from `g_utf8_get_char()` to
`g_utf8_get_char_validated()`.
Spotted by linhlhq as #YWH-PGM9867-187. Thank you for the suggested fix
and unit test, which have been adapted into this commit.
Also spotted by oss-fuzz after we added a fuzz test for this function.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3916
oss-fuzz#498475244
This allows a single byte out-of-bounds read off the end of the
(potentially untrusted) byte array backing a `GVariant` when it’s
being checked for normal form.
I can’t see how this could practically be exploited, but it’s certainly
a security bug as the `GVariant` normal form checking code is supposed
to be robust to malicious inputs.
Spotted by linhlhq as #YWH-PGM9867-190, and fix and reproducer provided
by them too, thanks. Confirmed and turned into a unit test by me.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3915
