Commit Graph

118 Commits

Author SHA1 Message Date
Sebastian Wilhelmi
c74177337d gdbusmessage: Disallow zero-length elements in arrays
They are not allowed in the specification, and can lead to infinite
loops when parsing.

That’s a security issue if your application is accepting D-Bus messages
from untrusted peers (perhaps in a peer-to-peer connection). It’s not
exploitable when your application is connected to a bus (such as the
system or session buses), as the bus daemons (dbus-daemon or
dbus-broker) filter out such broken messages and don’t forward them.

Arrays of zero-length elements are disallowed in the D-Bus
specification: https://dbus.freedesktop.org/doc/dbus-specification.html#container-types

oss-fuzz#41428, #41435
Fixes: #2557
2022-01-26 13:04:49 +00:00
Sebastian Wilhelmi
6499ad5356 gdbusmessage: Disallow empty structures/tuples in D-Bus messages
They are disallowed in the specification:
https://dbus.freedesktop.org/doc/dbus-specification.html#container-types

Helps: #2557
2022-01-26 13:04:49 +00:00
Sebastian Dröge
5fcd2495f9 De-duplicate g_nearest_pow() implementation
And put it as static inline function into a private shared header instead.
2021-11-25 14:05:42 +02:00
Philip Withnall
1f8390c002 gdbusmessage: Remove arbitrary restriction on minimum D-Bus blob length
The code in `g_dbus_message_new_from_blob()` has now been fixed to
correctly error out on all truncated messages, so there’s no need for an
arbitrary programmer error if the input is too short to contain a valid
D-Bus message header.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2528
2021-11-23 12:57:29 +00:00
Sebastian Wilhelmi
4b9a27a868 gdbusmessage: Add more bounds checking when parsing D-Bus messages
Perform strict bounds checking when reading data from the D-Bus message,
and propagate errors to the callers.

Previously, truncated D-Bus messages could cause out-of-bounds reads.

This is a security issue, but one which is only exploitable when
communicating with an untrusted peer (who might send malicious
messages). Almost all D-Bus traffic is with a session or system bus,
where the dbus-daemon or dbus-broker is trusted, and is known to have
already rejected malformed (malicious) messages.

Accordingly, this is only exploitable with peer-to-peer D-Bus
conversations with an untrusted peer.

(Includes some minor cleanups from Philip Withnall.)

oss-fuzz#17408
Fixes: #2528
2021-11-23 12:32:31 +00:00
nitinosiris
a70df97c63 Added test case for g_dbus_is_error_name 2021-06-16 08:06:03 +05:30
Philip Withnall
ac8f4a0db5 gdbus: Add various missing (nullable) or (not nullable) annotations
This is the result of checking each `Returns:` line in these files. I’ve
only considered nullability and not other (potentially missing or
incorrect) annotations.

Including suggestions by Simon McVittie.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #2227
2021-06-03 10:02:37 +01:00
Sebastian Dröge
705a59a315 gio: Add missing nullable annotations 2020-11-11 13:15:21 +02:00
Simon McVittie
fc1f4969bf gdbus: Document the intended semantics of handles and fds
In the D-Bus wire protocol, the handle type (G_VARIANT_TYPE_HANDLE, h)
is intended to be an index/pointer into the implementation's closest
equivalent of GUnixFDList: its numeric value has no semantic meaning
(in the same way that the numeric values of pointers have no semantic
meaning), but a handle with value n acts as a reference to the nth fd
in the fd list.

GDBus provides a fairly direct mapping from the wire protocol to the
C API, which makes it technically possible to attach and use fds
without ever referring to them in the message body, and some
GLib-centric D-Bus APIs rely on this.

However, the other major implementations of D-Bus (libdbus and sd-bus)
transparently replace file descriptors with handles when building
messages, and transparently replace handles with file descriptors when
parsing messages. This means they cannot implement D-Bus APIs that do
not follow the conventional meaning of handles as indexes/pointers into
an equivalent of GUnixFDList.

For interoperability, we should encourage D-Bus API designers to follow
the convention, even though code written against GDBus doesn't strictly
need to do so.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2020-10-28 11:52:22 +00:00
Philip Withnall
7afd4071d6 gdbusmessage: Drop redundant uint ≥ 0 checks
They are always true.

Coverity CID: #1430645, #1430674, #1430884, #1430974
Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-08-18 10:39:46 +01:00
Philip Withnall
00bfb3ab44 tree: Fix various typos and outdated terminology
This was mostly machine generated with the following command:
```
codespell \
    --builtin clear,rare,usage \
    --skip './po/*' --skip './.git/*' --skip './NEWS*' \
    --write-changes .
```
using the latest git version of `codespell` as per [these
instructions](https://github.com/codespell-project/codespell#user-content-updating).

Then I manually checked each change using `git add -p`, made a few
manual fixups and dropped a load of incorrect changes.

There are still some outdated or loaded terms used in GLib, mostly to do
with git branch terminology. They will need to be changed later as part
of a wider migration of git terminology.

If I’ve missed anything, please file an issue!

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2020-06-12 15:01:08 +01:00
Philip Withnall
8f938ebe27 gdbusmessage: Fix swapped signatures in error messages
Signed-off-by: Philip Withnall <withnall@endlessm.com>

Fixes: #2081
2020-04-03 12:42:49 +01:00
Philip Withnall
5054b48b7c gdbusmessage: Limit recursion of variants in D-Bus messages
This is the analogue of commit 7c4e6e9fbe, but applied to the
`GDBusMessage` parser, which does its own top-level parsing of the
variant format in D-Bus messages.

Previously, this code allowed arbitrary recursion of variant containers,
which could lead to a stack overflow. Now, that recursion is limited to
64 levels, as per the D-Bus specification:

https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-marshaling-signature

This includes a new unit test.

oss-fuzz#14870

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2019-11-18 17:53:55 +00:00
Philip Withnall
96d792197a gdbusmessage: Move variable initialisation to declaration time
Tidies up the code a bit, but introduces no functional changes.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2019-11-15 10:12:59 +00:00
Philip Withnall
25636e50e0 gdbusmessage: Fix comparisons out of range for enum types
This was warning on macOS.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Fixes: #887
2019-06-17 15:45:30 +01:00
Philip Withnall
1b50643c99 gio: Fix various compiler warnings when compiling with G_DISABLE_ASSERT
Mostly unused variables which are only used in a g_assert() call
otherwise.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #1708
2019-03-08 19:46:21 +00:00
Ernestas Kulik
566e1d61a5 gdbus: Avoid printing null strings
This mostly affects the 2.56 branch, but, given that GCC 9 is being
stricter about passing null string pointers to printf-like functions, it
might make sense to proactively fix such calls.

gdbusauth.c: In function '_g_dbus_auth_run_server':
gdbusauth.c:1302:11: error: '%s' directive argument is null
[-Werror=format-overflow=]
 1302 |           debug_print ("SERVER: WaitingForBegin, read '%s'",
 line);
       |

gdbusmessage.c: In function ‘g_dbus_message_to_blob’:
gdbusmessage.c:2730:30: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
 2730 |       tupled_signature_str = g_strdup_printf ("(%s)", signature_str);
      |
2019-01-29 11:06:39 +01:00
Philip Withnall
31b0ba18c1 gdbusmessage: Fix check on upper limit of message size
There was a typo in the figure checked against. Add a unit test.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://gitlab.gnome.org/GNOME/glib/issues/1642
2019-01-28 14:37:22 +00:00
Philip Withnall
0ff5e5cd32 gdbusmessage: Gracefully handle message signatures with invalid types
With the changes to limit GVariant type nesting (commit 7c4e6e9fbe),
it’s now possible to have a valid type signature which is not a valid
GVariant type when enclosed in parentheses (to make it a tuple).

Check for that when parsing the signature field in a D-Bus message.

Includes a unit test.

oss-fuzz#11120

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-11-15 09:22:38 +00:00
Philip Withnall
e03d5a335b gdbusmessage: Check for valid GVariantType when parsing a variant blob
The code was checking whether the signature provided by the blob was a
valid D-Bus signature — but that’s a superset of a valid GVariant type
string, since a D-Bus signature is zero or more complete types. A
GVariant type string is exactly one complete type.

This meant that a D-Bus message with a header field containing a variant
with an empty type signature (for example) could cause a critical
warning in the code parsing it.

Fix that by checking whether the string is a valid type string too.

Unit test included.

oss-fuzz#9810

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-10-23 17:01:51 +13:00
Philip Withnall
968f1c6cad gdbusmessage: Fix a typo in a documentation comment
Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-10-23 17:01:51 +13:00
Philip Withnall
50b544e2c0 gdbusmessage: Clarify error returns for g_dbus_message_new_from_blob()
Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-10-23 17:01:51 +13:00
Philip Withnall
94a9ab3c03 gdbusmessage: Improve documentation for g_dbus_message_get_header()
The caller is responsible for checking the type of the returned
GVariant.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-10-23 17:01:51 +13:00
Philip Withnall
af712bbce1 gdbusmessage: Validate type of message header signature field
Parsing a D-Bus message with the signature field in the message header
of type other than ‘g’ (GVariant type signature) would cause a critical
warning. Instead, we should return a runtime error.

Includes a test.

oss-fuzz#9825

Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-10-23 17:01:51 +13:00
Philip Withnall
ab09a5f5a9 gdbusmessage: Fix a typo in the documentation
Signed-off-by: Philip Withnall <withnall@endlessm.com>
2018-08-07 11:04:00 +01:00
Ting-Wei Lan
c61f01f0ff build: Define MAJOR_IN_TYPES when major is defined in sys/types.h
Autoconf macro AC_HEADER_MAJOR doesn't define a macro in config.h when
major is defined in sys/types.h. This was not a problem because major
is assumed to be always available. However, commit aefffa3fbc
changes this assumption in order to fix build on systems without major,
which causes code using major to be disabled on systems putting major
in sys/types.h.

This commit defines a new macro MAJOR_IN_TYPES for both autotools and
meson builds to make major useful on these systems again.
2018-06-07 23:05:50 +08:00
Philip Withnall
07f75f6cc2 gdbusmessage: Make a translatable message translatable with plurals
Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=658713
2018-02-02 10:07:12 +01:00
Philip Withnall
e644bfa37e gio: Add some casts for printf() formatting statbufs on Solaris
Apparently Solaris defines statbuf fields as long when Linux doesn’t, in
some cases. Cast down to the type expected by the printf() format
placeholder.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=749652
2017-11-28 14:17:09 +00:00
Daniel Macks
190f64a0fb Avoid setting unused variables (-Wself-assign)
Setting a variable and then assigning it to itself avoids
-Wunused-but-set-variable but this specific trick is now caught by
-Wself-assign. Instead, actually use the value or don't bother
assigning it at all:

gdbusauth.c: call g_data_input_stream_read_byte() in void context
gdbusauthmechanismsha1.c: value is actually used
gdbusmessage.c: use consistent preprocessor-token protection
gthreadedresolver.c: skip over bytes in data blob
httpd.c: do something useful with the value

https://bugzilla.gnome.org/show_bug.cgi?id=745723
2017-09-11 22:14:18 +01:00
Philip Withnall
5cddde1fb2 Consistently save errno immediately after the operation setting it
Prevent the situation where errno is set by function A, then function B
is called (which is typically _(), but could be anything else) and it
overwrites errno, then errno is checked by the caller.

errno is a horrific API, and we need to be careful to save its value as
soon as a function call (which might set it) returns. i.e. Follow the
pattern:
  int errsv, ret;
  ret = some_call_which_might_set_errno ();
  errsv = errno;

  if (ret < 0)
    puts (strerror (errsv));

This patch implements that pattern throughout GLib. There might be a few
places in the test code which still use errno directly. They should be
ported as necessary. It doesn’t modify all the call sites like this:
  if (some_call_which_might_set_errno () && errno == ESOMETHING)
since the refactoring involved is probably more harmful than beneficial
there. It does, however, refactor other call sites regardless of whether
they were originally buggy.

https://bugzilla.gnome.org/show_bug.cgi?id=785577
2017-08-03 10:21:13 +01:00
Philip Withnall
18f8b77c04 gio: Use g_strerror() instead of strerror()
This marginally improves thread safety, and marginally improves
consistency.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=733821
2017-06-21 11:20:29 +01:00
Sébastien Wilmet
3bf4a720c3 gio/: LGPLv2+ -> LGPLv2.1+
Sub-directories inside gio/ already processed in a previous commit:
- fam/
- gdbus-2.0/ (which contains only codegen/)
- gvdb/
- inotify/
- tests/
- win32/
- xdgmime/

Other sub-directories inside gio/:
- completion/: no license headers
- kqueue/: not LGPL, BSD-style license

https://bugzilla.gnome.org/show_bug.cgi?id=776504
2017-05-29 19:53:34 +02:00
declan
aefffa3fbc gdbusmessage: Don’t use major()/minor() if they’re unavailable
https://bugzilla.gnome.org/show_bug.cgi?id=777030
2017-04-28 15:26:28 +01:00
Emmanuele Bassi
f952fdf3fc Drop trailing semi-colon from G_DEFINE_ macro
It's unnecessary, and only adds visual noise; we have been fairly
inconsistent in the past, but the semi-colon-less version clearly
dominates in the code base.

https://bugzilla.gnome.org/show_bug.cgi?id=669355
2017-04-10 10:38:31 +01:00
Christian Hergert
18a33f72db introspection: use (nullable) or (optional) instead of (allow-none)
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.
2016-11-22 14:14:37 -08:00
Christophe Fergeau
a4012abbdf gdbus: Fix leak in g_dbus_message_print()
This leak only happens when using G_DBUS_DEBUG=message or similar, but
it's nicer to be valgrind-clean even in these cases.

https://bugzilla.gnome.org/show_bug.cgi?id=736810
2016-11-22 15:01:31 -05:00
Lukasz Skalski
59a996db9b GDBusMessage: Fix segfault if DEBUG_SERIALIZER is enabled
https://bugzilla.gnome.org/show_bug.cgi?id=729730
2016-11-22 14:59:49 -05:00
Piotr Drąg
10c490cdfe Use Unicode in translatable strings
See https://developer.gnome.org/hig/stable/typography.html

https://bugzilla.gnome.org/show_bug.cgi?id=772221
2016-10-12 21:30:42 +02:00
Philip Withnall
bc01109618 gdbusmessage: Fix a minor memory leak on an error path
If g_dbus_message_to_blob() fails at all, it will leak its mbuf. Spotted
by running the gdbus-serialization test under Valgrind — so there is a
justification for leak-free tests after all!
2015-04-22 00:02:06 +01:00
Ross Lagerwall
226c292b6a gio: Prevent hang writing to a large GMemoryBuffer
Fix a hang due to overflow by using unsigned numbers and explicitly
checking if the number overflows to zero.  This also fixes the previous
logic which assigned an int which may be negative to an unsigned number
resulting in sign extension and strange results.

Use gsize rather than int to allow for large buffers on 64 bit machines.

https://bugzilla.gnome.org/show_bug.cgi?id=727988
2014-10-30 20:15:47 +00:00
Ryan Lortie
2268628565 gdbus: Properly fix encoding of double arrays
It turns out that this bug actually would (sometimes) impact any sort of
fixed-sized array with an alignment requirement of 8 due to incorrectly
counting the alignment inserted between the (aligned 4) array length and
the actual data.

Fix this properly and remove the exception for doubles.

https://bugzilla.gnome.org/show_bug.cgi?id=732754
2014-07-24 15:51:21 +02:00
Stef Walter
627b49b390 gio: Fix regression encoding an array of doubles
Take the simple slow path in this case. Encoding a double
takes special precautions as you can see in append_value_to_blob()
and friends.

https://bugzilla.gnome.org/show_bug.cgi?id=732754
2014-07-23 15:44:40 +02:00
Ryan Lortie
5d0e295fb8 GDBusMessage: add some G_UNLIKELY on error paths
More for documentation than anything else...
2014-07-09 11:19:58 -04:00
Ryan Lortie
5463c8cedb GDBusMessage: fast-path decoding of fixed arrays
Instead of creating a separate GVariant for each of the 'y's in an 'ay',
use g_variant_new_fixed_array().

https://bugzilla.gnome.org/show_bug.cgi?id=732754
2014-07-09 10:43:26 -04:00
Ryan Lortie
958da1e9dc GDBusMessage: fast-path encoding of fixed arrays
Instead of handling each item separately, handle the array as a whole.

https://bugzilla.gnome.org/show_bug.cgi?id=732754
2014-07-09 10:43:26 -04:00
Ryan Lortie
d108ada4b9 GDBusMessage: simplify byteswapping
Remove some duplication by simplifying our byteswap checks.

https://bugzilla.gnome.org/show_bug.cgi?id=732754
2014-07-07 15:02:54 -04:00
Evan Nemerson
570b27b9ac gio: port annotations from the Vala metadata.
https://bugzilla.gnome.org/show_bug.cgi?id=730493
2014-05-23 10:04:06 -07:00
Lukasz Skalski
7c205ae90d gio: cleanup gdbusmessage.c file
* removed passing GError to ensure_input_padding() function
  - it was necessary before commit 3e5214c15c
  when we used GData*Streams and GMemoryInputStream with
  g_seekable_seek() - now it's useless,

* removed checking return value of ensure_input_padding()
  function - in previous implementation (like above)
  g_seekable_seek() could return FALSE - now it's always TRUE,

* removed passing GError to g_memory_buffer_read_*() functions
  and checking returned value - it also has been inherited after
  old implementation with g_data_input_stream_read_*() functions
  - now it's also useless

* cleaned up code formatting,

https://bugzilla.gnome.org/show_bug.cgi?id=729875
2014-05-09 10:21:22 -04:00
Matthias Clasen
42cf80780b Docs: Big entity cleanup
Strip lots of entity use from |[ ]| examples (which are now
implicit CDATA). Also remove many redundant uses of <!-- -->.
2014-02-01 12:00:30 -05:00
Matthias Clasen
17f51583a8 Docs: Convert examples to |[ ]| 2014-01-31 21:56:33 -05:00