While backporting CVE-2024-34397 fixes I noticed that this comment
claimed that the reference count is immutable after construction, which
is clearly not true. In fact the reference count is the only
mutable field.
Signed-off-by: Simon McVittie <smcv@collabora.com>
To avoid adding a large block of macros to gdbusprivate.h, I've only
added a subset of the well-known error names. I chose to draw the
line by adding constants for the errors emitted via their string names
in GDBusConnection, but not for error names that are only mentioned
in `gdbuserror.c` or in tests.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Most D-Bus interfaces are domain-specific, but these interfaces from the
D-Bus Specification are intended to be commonly used in any context for
which they are found to be appropriate.
Most of these use `gdbusprivate.h`. One exception is that
`gio/tests/gdbus-example-*` redefine the constants locally: due to these
files' dual role as part of the unit tests and as sample code, it seems
desirable to ensure that they can still be compiled outside GLib.
Signed-off-by: Simon McVittie <smcv@collabora.com>
These well-known flags and replies are part of the D-Bus Specification,
and also exist with the same names in libdbus header files.
Moving them into a private header means that unit tests like
gdbus-proxy-threads and gdbus-subscribe don't have to reinvent them.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Unlike the various functions to call D-Bus methods, these sort their
arguments in a non-obvious order (bus name, interface, signal, path),
presumably aiming to sort the most-likely-to-be-used arguments first.
Signed-off-by: Simon McVittie <smcv@collabora.com>
These function arguments are arranged in the obvious order from
conceptually largest to smallest: (bus name, path, interface, method).
Signed-off-by: Simon McVittie <smcv@collabora.com>
This test was subscribing to the NameOwnerChanged signal with an
incorrect object path, so the callback would never be called. In this
particular case it doesn't actually matter, because the callback does
nothing anyway (the purpose of this particular test was to test that
the user-data is freed on unsubscription).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Almost identically to the previous commit, fix a similar latent bug in
`g_dbus_connection_export_action_group()`, which was not ready to handle
the fledgling `GActionGroupExporter` being freed early on an error
handling path.
See the previous commit message for details of the approach.
This includes a unit test.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3366
This latent bug wasn’t triggered until commit 3f30ec86c (or its
cherry-pick onto `glib-2-80`, 747e3af99, which was first released in
2.80.1).
That change means that `g_menu_exporter_free()` is now called on the
registration failure path by `g_dbus_connection_register_object()`
before it returns. The caller then tries to call `g_slice_free()` on the
exporter again. The call to `g_menu_exporter_free()` tries to
dereference/free members of the exporter which it expects to be
initialised — but because this is happening in an error handling path,
they are not initialised.
If it were to get any further, the `g_slice_free()` would then be a
double-free on the exporter allocation.
Fix that by making `g_menu_exporter_free()` robust to some of the
exporter members being `NULL`, and moving some of the initialisation
code higher in `g_dbus_connection_export_menu_model()`, and removing the
duplicate free code on the error handling path.
This includes a unit test.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3366
gio/glocalfileinfo.c has a struct 'ThumbMD5Context'
that's been unused since
commit d013d46b98 ("Replace the copy-and-paste MD5 digest generation
with GChecksum.")
Remove it.
Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
It's an array containing the list of sanitizers in use, normally it
contains a value, but in some cases may have more than one (e.g.
'address' and 'undefined').
And so use it to avoid repeated checks
In glocalfile we're allocating some temporary strings but we don't free
them on early returns, so free them once done and unset the variables
to prevent them being used incorrectly.
It looks like that finally also valgrind notices the same leaks as
address sanitizer does. It does it more randomly but it still happens,
so better to inform about until #2309 is resolved.
We're now caching arg0 but such value is not cleared when a new body is
set as it's in the connection filter test cases where we've a leak as
highlighted by both valgrind and leak sanitizer
In a D-Bus-Specification-compliant message bus, the owner of a well-known
name is a unique name. However, ibus has its own small implementation
of a message bus (src/ibusbus.c) in which org.freedesktop.IBus is
special-cased to also have itself as its owner (like org.freedesktop.DBus
on a standard message bus), and connects to that bus with the
G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION flag. The ability to do
this regressed when CVE-2024-34397 was fixed.
Relax the checks to allow the owner of a well-known name to be any valid
D-Bus name, even if it is not syntactically a unique name.
Fixes: 683b14b9 "gdbus: Track name owners for signal subscriptions"
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3353
Bug-Debian: https://bugs.debian.org/1070730
Bug-Debian: https://bugs.debian.org/1070736
Bug-Debian: https://bugs.debian.org/1070743
Bug-Debian: https://bugs.debian.org/1070745
Signed-off-by: Simon McVittie <smcv@debian.org>
The test case assumes signals will dispatched in a different order than
they're subscribed. In fact, signals can be dispatched in any order,
and are often dispatched in order.
This commit reorders the subscriptions so they're in order, which is
more logical, and also changes the code to only exit the event loops
when there are no pending handlers ready to dispatch.
This was highlighted (but not introduced) by
0144feb41f. Previously the test coverage
didn’t cover this branch, I think.
`iter` was leaked, and at this point `parameter` had never been set, so
clearing it was unnecessary.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3349
GDBusConnection sends each signal to recipients in a separate idle
callback, and there's no particular guarantee about the order in which
they're scheduled or dispatched. For the NameOwnerChanged signal that
reports the name becoming unowned, it's possible that g_bus_watch_name()
gets its idle callback called before the GDBusProxy:g-name-owner
machinery has updated the name owner, in which case the assertion
will fail.
Fixing GNOME/glib#3268 introduced a new subscription to NameOwnerChanged
which can alter the order of delivery, particularly in the case where
G_DBUS_PROXY_FLAGS_NO_MATCH_RULE was used (as tested in
/gdbus/proxy/no-match-rule). The resulting test failure is intermittent,
but reliably appears within 100 repetitions of that test.
Fixes: 511c5f5b "tests: Wait for gdbus-testserver to die when killing it"
Signed-off-by: Simon McVittie <smcv@debian.org>
If the file to be added is on a read-only filesystem, opening read/write
will fail with EROFS. In this case we should fall back to opening it
read-only, the same way we already do if write access is forbidden by
DAC or MAC.
An easy way to reproduce this test failure is to build and test GLib
in a podman container, with its source code read-only and its build
directory read/write:
podman run --rm -it \
-v $(pwd):$(pwd):ro \
-v $(pwd)/_build:$(pwd)/_build:rw \
-w $(pwd) ...
Before this commit, the dbus-appinfo test would fail, because opening
${srcdir}/gio/tests/org.gtk.test.dbusappinfo.flatpak.desktop read/write
would fail with EROFS.
For completeness, give similar handling to the other error codes
documented in Linux open(2) that might succeed if re-attempted using
read-only access: according to that documentation, we could get EPERM
if opening read/write is prevented by fcntl F_ADD_SEALS, or ETXTBSY
if the file is an executable that is currently being run.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This was a bug that existed during development of this branch; make sure
it doesn't come back.
This test fails with a use-after-free and crash if we comment out the
part of name_watcher_unref_watched_name() that removes the name watcher
from `map_method_serial_to_name_watcher`.
It would also fail with an assertion failure if we asserted in
name_watcher_unref_watched_name() that get_name_owner_serial == 0
(i.e. that GetNameOwner is not in-flight at destruction).
Signed-off-by: Simon McVittie <smcv@collabora.com>
The vulnerability reported as GNOME/glib#3268 can be characterized
as: these signals from an attacker should not be delivered to either
the GDBusConnection or the GDBusProxy, but in fact they are (in at
least some scenarios).
Reproduces: https://gitlab.gnome.org/GNOME/glib/-/issues/3268
Signed-off-by: Simon McVittie <smcv@collabora.com>
The expected result is that because TEST_CONN_SERVICE owns
ALREADY_OWNED_NAME but not (yet) OWNED_LATER_NAME, the signal will be
delivered to the subscriber for the former but not the latter.
Before #3268 was fixed, it was incorrectly delivered to both.
Reproduces: https://gitlab.gnome.org/GNOME/glib/-/issues/3268 (partially)
Signed-off-by: Simon McVittie <smcv@collabora.com>
Otherwise a malicious connection on a shared bus, especially the system
bus, could trick GDBus clients into processing signals sent by the
malicious connection as though they had come from the real owner of a
well-known service name.
Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3268
Signed-off-by: Simon McVittie <smcv@collabora.com>
We will use this in a subsequent commit to prevent signals from an
impostor from being delivered to a subscriber.
To avoid message reordering leading to misleading situations, this does
not use the existing mechanism for watching bus name ownership, which
delivers the ownership changes to other main-contexts. Instead, it all
happens on the single thread used by the GDBusWorker, so the order in
which messages are received is the order in which they are processed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If a connection has two signal subscriptions active for the same signal,
one with arg0 matching and one without, a signal which doesn’t contain
an arg0 value (i.e. `g_dbus_message_get_arg0()` returns `NULL`) will
cause `NULL` to be passed to `strcmp()` when checking for a match
against the signal subscription which *has* arg0 matching, causing a
crash.
Fix that by adding the obvious `NULL` check, and add a unit test.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Fixes: #3342
D-Bus Activation allows passing an array of parameters. Allow apps to
export actions that accept tuples to match the number of elements in the
parameters so the full potential of the D-Bus interface can be used.
Closes: https://gitlab.gnome.org/GNOME/glib/-/issues/3333
See the code comment. scan-build can’t handle analysis over the
refcounts, so consistently complains about potential use-after-free
errors in the code, essentially because:
* It understands `name_unref()`, but completely ignores `name_ref()`
* The code often calls `name_unref()` on the ‘wrong’ pointer, in the
sense that it knows that if another struct exists, that struct holds
a ref on a `Name`, but without actually having a pointer to the
`Name`. So the code calls `name_unref (name); name_unref (name)`.
That’s valid, but quite understandably looks like a recipe for a
use-after-free.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks that there can be a `NULL` pointer dereference of
`error` here because it doesn’t understand that the function return
value and `GError` are related: when a valid return value is returned,
the error is `NULL` and vice-versa.
Try and make that clearer to the static analyser by checking whether the
error is `NULL`, rather than the return value.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks there could be a `NULL` pointer dereference of
`t->data` here. It’s wrong, so add an assertion to try and help it
understand the control flow.
The loop is exited as soon as a target is found whose weight is greater
than or equal to a random value between 0 and the sum of all the weights
in the set of remaining targets in the loop. By definition, the last
target in the loop always satisfies this condition, so a target will
always be chosen, and hence `t` will never be `NULL` within the loop.
`t->data` will never be `NULL` by construction of the target list.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build thinks that `data` could be leaked. It’s not, though; it’s
passed as the `user_data` to `g_dbus_connection_register_object()` along
with its free function.
Try and persuade scan-build that there’s no leak by annotating the
transfer.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
scan-build was complaining that `dest_hostname` and `dest_protocol` were
used after being freed, which could potentially happen if the code is
built with `G_DISABLE_CHECKS`. This is a false positive, because the
state of types in the program should be the same regardless of whether
`G_DISABLE_CHECKS` is used.
However, the code did smell. If we are trying to free things and return
gracefully if the underlying socket address enumerator returns something
of the wrong type, why not free the rest of the function’s state, or
skip the invalid address and move on to the next one? Or if we are trying
to make an assertion, why bother freeing some temporary data at all?
This halfway house doesn’t make sense.
So turn the `g_return_val_if_fail()` into a full assertion.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
After a lot of loop unwinding, during which I think it might have lost
its knowledge that `cache->buffer != NULL` (from a prior check on line
765), scan-build seems to think that there can be a `NULL` pointer
dereference of `cache->buffer` within `cache_magic_compare_to_data()`.
There can’t be. Add an assertion to try and help the analyser.
Upstreamed as
https://gitlab.freedesktop.org/xdg/xdgmime/-/merge_requests/38.
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #1767
