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.
[Backported to glib-2-74, resolving minor conflicts]
Signed-off-by: Simon McVittie <smcv@collabora.com>
This will become confusing when we start tracking the owner of a
well-known-name sender, and it's redundant anyway. Instead, track the
1 bit of data that we actually need: whether it's a well-known name.
Strictly speaking this too is redundant, because it's syntactically
derivable from the sender, but only via extra string operations.
A subsequent commit will add a data structure to keep track of the
owner of a well-known-name sender, at which point this boolean will
be replaced by the presence or absence of that data structure.
Signed-off-by: Simon McVittie <smcv@collabora.com>
No functional change, just removing some nesting. The check for whether
signal_data->subscribers is empty changes from a conditional that tests
whether it is into an early-return if it isn't.
A subsequent commit will add additional conditions that make us consider
a SignalData to be still in use and therefore not eligible to be removed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
No functional changes, except that the implicit ownership-transfer
for the rule field becomes explicit (the local variable is set to NULL
afterwards).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Subsequent changes will need to access these data structures from
on_worker_message_received(). No functional change here, only moving
code around.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Using these is a bit more clearly correct than repeating them everywhere.
To avoid excessive diffstat in a branch for a bug fix, I'm not
immediately replacing all existing occurrences of the same literals with
these names.
The names of these constants are chosen to be consistent with libdbus,
despite using somewhat outdated terminology (D-Bus now uses the term
"well-known bus name" for what used to be called a service name,
reserving the word "service" to mean specifically the programs that
have .service files and participate in service activation).
Signed-off-by: Simon McVittie <smcv@collabora.com>
On GNOME/glib#3268 there was some concern about whether this would
allow an attacker to send signals and have them be matched to a
GDBusProxy in this situation, but it seems that was a false alarm.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This somewhat duplicates test_connection_signals(), but is easier to
extend to cover different scenarios.
Each scenario is tested three times: once with lower-level
GDBusConnection APIs, once with the higher-level GDBusProxy (which
cannot implement all of the subscription scenarios, so some message
counts are lower), and once with both (to check that delivery of the
same message to multiple destinations is handled appropriately).
[Backported to glib-2-74, resolving conflicts in gio/tests/meson.build]
Signed-off-by: Simon McVittie <smcv@collabora.com>
A subsequent commit will need this. Copying all of g_set_str() into a
private header seems cleaner than replacing the call to it.
Helps: GNOME/glib#3268
Signed-off-by: Simon McVittie <smcv@collabora.com>
Technically we can’t rely on it being kept alive by the `message->body`
pointer, unless we can guarantee that the `GVariant` is always
serialised. That’s not necessarily the case, so keep a separate ref on
the arg0 value at all times.
This avoids a potential use-after-free.
Spotted by Thomas Haller in
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/3720#note_1924707.
[This is a prerequisite for having tests pass after fixing the
vulnerability described in glib#3268, because after fixing that
vulnerability, the use-after-free genuinely does happen during
regression testing. -smcv]
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
Helps: #3183, #3268
(cherry picked from commit 10e9a917be7fb92b6b27837ef7a7f1d0be6095d5)
Commit 4a4d9eb6624 initially switched Meson to find the python
program using find_program('python3'). Sadly that caused a
regression, since in some cases with MSVC it would fallback to
'meson.exe runpython', which is undesired.
However, that particular code was reverted back to an also
undesired lookup method, find_installation('python3'). This
way of finding python also breaks on Windows + MSVC, in particular
when setting it up as follows:
```
winget install python
winget install meson
```
This fails building GLib with:
> python3 not found
Fix that by not passing any argument to find_installation(), which
lets Meson figure it all out by itself.
(cherry picked from commit 255fa26b964bbcd22150dafbfe5ead0acf0b84ad)
In some cases (such as in our CI tests) we may not have any dbus session
set after launching, but we always assumed so.
In case we have not a session bus set, we only have to return early.
(cherry-picked from commit eee15225c74559f1ba02c78175a25be186cf1cf7)
It should be either a valid URI, or NULL. Passing empty strings or other
invalid URIs is no bueno.
(cherry-picked from commit 6f83f45db4b859839b81f07cc942a49834663ffc)
Currently it's nullable in g_simple_proxy_resolver_new(), but not in
g_simple_proxy_resolver_set_default_proxy() nor the property. Fix these.
(cherry-picked from commit 8a1f087a31c9fc0e50cd147d4ce11a4bfff647c0)
We don't need a cpp toolchain for building glib so lets just
automatically disable tests requiring one when not available.
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
(cherry-picked from commit d0b9ebbaacb5f69aaaad30829e04cd3a88f084cb)
Add missing call to _g_io_modules_ensure_extension_points_registered() to
GRegistryBackend, GNullSettingsBackend, and GNextstepSettingsBackend
(cherry-picked from commit 924da751c2d3ed4636223343ebaa7364b97f8f93)
Using GMemorySettingsBackend before any other GSettingsBackend would
cause the following error: "Tried to implement non-registered extension
point gsettings-backend". This is due to a missing call to
_g_io_modules_ensure_extension_points_registered() in the GMemorySettingsBackend
type definition which registers the gsettings-backend extension point.
(cherry-picked from commit 04255e45654bd49f1974a79baeafb33d228f6f71)
RFC 4422 appendix A defines the empty authorization identity to mean
the identity that the server associated with its authentication
credentials. In this case, this means whatever uid is in the
GCredentials object.
In particular, this means that clients in a different Linux user
namespace can authenticate against our server and will be authorized
as the version of their uid that is visible in the server's namespace,
even if the corresponding numeric uid returned by geteuid() in the
client's namespace was different. systemd's sd-bus has relied on this
since commit
1ed4723d38.
[Originally part of a larger commit; commit message added by smcv]
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry-picked from commit b51e3ab09e39c590c65a7be6228ecfa48a6189f6)
Sending an "initial response" along with the AUTH command is meant
to be an optional optimization, and clients are allowed to omit it.
We must reply with our initial challenge, which in the case of EXTERNAL
is an empty string: the client responds to that with the authorization
identity.
If we do not reply to the AUTH command, then the client will wait
forever for our reply, while we wait forever for the reply that we
expect the client to send, resulting in deadlock.
D-Bus does not have a way to distinguish between an empty initial
response and the absence of an initial response, so clients that want
to use an empty authorization identity, such as systed's sd-bus,
cannot use the initial-response optimization and will fail to connect
to a GDBusServer that does not have this change.
[Originally part of a larger commit; commit message added by smcv.]
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry-picked from commit a7d2e727eefcf883bb463ad559f5632e8e448757)
This is an interoperability fix. If the line is exactly "DATA\r\n",
the reference implementation of D-Bus treats this as equivalent to
"DATA \r\n", meaning the data block consists of zero hex-encoded bytes.
In practice, D-Bus clients send empty data blocks as "DATA\r\n", and
in fact sd-bus only accepts that, rejecting "DATA \r\n".
[Originally part of a larger commit; commit message added by smcv]
Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
Co-authored-by: Simon McVittie <smcv@collabora.com>
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry-picked from commit 764f071909df70622e79ee71323973c18c055c8c)
For all the memory allocator APIS, document
that they terminate the program on failure.
This was so far only mentioned in the long description,
and in the docs for g_try_malloc(). And with gi-docgen
style docs, the long description is going away.
(cherry-picked from commit 1df83acb876c8325dfadf96bafc5751e1d9e5447)
As per the rationale explained in the previous commit, we could end up
having the unused_threads value not to be conformant to what
g_thread_pool_get_num_threads() returns, because an about-to-be-unused
thread might not be counted yet as such, while the pool threads number
has been already decreased.
To avoid such scenario, and to make sure that when all the pool's
threads are stopped, they're unmarked as unused, let's increase the
unused_threads value earlier, while we still own the pool lock so that
it will always include the pool that is not used anymore, but not yet
queued.
As per this we can update the test, not to repeat the stop-unused call
as now we're sure that when the pool has no threads anymore, the unused
threads value is also updated accordingly.
Also adding a tests with multiple pools.
(cherry-picked from commit a275ee66796ab0d6d95ed8647f2170be9b136951)
GCC isn't smart enough to recognise that the assertion on the size of
N_PROPERTIES also affects the assertion on the GParamSpec array access,
so we need to coalesce the two checks into one to avoid an array-bounds
compiler warning.
(cherry-picked from commit 903c004b37d723972b07ecbdd880ae0d2c8b767d)
The search_total_results address is always going to be non-zero, so the
check will always evaluate to true, and GCC is kind enough to point this
out to us.
The appropriate fix is checking if the size of the search results array
is larger than zero, and if so, copy them into the total results array.
(cherry-picked from commit e08c954693fdcfceda2de59cca93a76125f4fca6)
While `gio_xdgmime` is unlocked, the data which `type` points to in the
xdgmime cache might get invalidated, leaving `type` as a dangling
pointer. That would not bode well for the `g_strdup (type)` call to
insert a new entry into the `type_comment_cache` once `gio_xdgmime` is
re-acquired.
This was spotted using static analysis, and the symptoms have not
knowingly been seen in the wild.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Coverity CID: #1474702
(cherry-picked from commit 45d4c525)
It's a busy text file, but we don't care much about specifics so we
can just return the generic busy error.
(cherry-picked from commit 6bc6b7ef3012487966a99a5db06d27000205ab38)
We used to perform unneeded lock/unlock dances to perform block, unblock
and disconnect actions, and these were potentially unsafe because we
might have looped in data that could be potentially be changed by other
threads.
We could have also done the same by saving the handlers ids in a
temporary array and eventually remove them, but I don't see a reason for
that since we can just keep all locked without the risk of creating
deadlocks.
Coverity CID: #1474757, #1474771, #1474429
(cherry-picked from commit ae14f3219a756fa99dbbbb54555f10dd48eb0fea)
The returned `SocketAddress` is going to be NULL when the stream of
socket addresses is finished
(cherry-picked from commit ac3fc84ff41ad1fbcad765d170f5d741813dc84e)
This is unlikely to be a bug in practice, as the certificate pointed to
by `root` should have a ref held on it as the issuer of another
certificate in the chain.
However, we can’t guarantee that’s how the `GTlsCertificate`
implementation behaves, so keep a temporary ref on `root` until it’s no
longer needed.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Coverity CID: #1489985
(cherry-picked from commit 2c75c392eb76a12c3fd18b63508b1f971a3afecd)
The `(transfer none)` behaviour for `parameter_type` and `state_type`
parameters is implicit with the `const` attribute, but was incorrectly
determined to be `(transfer full)` in the GIR.
Add explicit `(transfer none)` annotations for these two parameters.
(cherry-picked from commit 1eb1a47a50f31b2cea71cf8c94c8989727abb98c)
==24477==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffde020de20 at pc 0x7f2e6f6413f1 bp 0x7ffde020c9d0 sp 0x7ffde020c180
READ of size 4101 at 0x7ffde020de20 thread T0
#0 0x7f2e6f6413f0 in __interceptor_strlen.part.0 (/lib64/libasan.so.8+0x4c3f0)
#1 0x7f2e6ef4abee in g_build_path_va ../glib/gfileutils.c:1908
#2 0x7f2e6f085956 in g_test_build_filename_va ../glib/gtestutils.c:4294
#3 0x7f2e6f086684 in g_test_build_filename ../glib/gtestutils.c:4365
#4 0x403a33 in test_search_path_heap_allocation ../glib/tests/spawn-path-search.c:422
#5 0x7f2e6f0839a5 in test_case_run ../glib/gtestutils.c:2930
#6 0x7f2e6f0839a5 in g_test_run_suite_internal ../glib/gtestutils.c:3018
#7 0x7f2e6f0834ed in g_test_run_suite_internal ../glib/gtestutils.c:3035
#8 0x7f2e6f084879 in g_test_run_suite ../glib/gtestutils.c:3112
#9 0x7f2e6f084995 in g_test_run ../glib/gtestutils.c:2231
#10 0x40145f in main ../glib/tests/spawn-path-search.c:488
#11 0x7f2e6e31258f in __libc_start_call_main (/lib64/libc.so.6+0x2d58f)
#12 0x7f2e6e312648 in __libc_start_main_alias_1 (/lib64/libc.so.6+0x2d648)
#13 0x401524 in _start (/home/elmarco/src/gnome/glib/build/glib/tests/spawn-path-search+0x401524)
Address 0x7ffde020de20 is located in stack of thread T0 at offset 4256 in frame
#0 0x40387f in test_search_path_heap_allocation ../glib/tests/spawn-path-search.c:401
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
(cherry-picked from 15ce3c9b37c2767c82de249e60781439c9abaf78)
This may fix Coverity assuming that pspecs are leaked, which is causing
tens and tens of false positives in the latest Coverity reports for
GLib.
Ensure that the pspecs are sunk (if floating) even if adding them to the
class fails (due to validation failure or an identically named property
already existing).
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
(cherry-picked from commit 8f7df344b636d5fda3d05560f5142d5d8515662a)
