We have never checked the <!DOCTYPE> of busconfig XML since the libxml
parser was removed in 2013, and the libxml parser was broken before
that anyway. The recommended Expat parser (our only parser since 2013)
does not appear to have ever validated this, so now does not seem like
the time to start. Just ignore the <!DOCTYPE> if there is one.
(We never validated this particularly strictly anyway;
<!DOCTYPE busconfig SYSTEM "http://example.com/bees"> would have been
treated as perfectly valid.)
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
This is an easy bit of missing test coverage detected by running the
test suite with gcov.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
These do not appear in code coverage statistics, and `git grep`
reveals that they are unused.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
As noted in GLib commit c879f50f, gcc's interpretation of the malloc
attribute has become more strict over time, which could result in
miscompilation. The new definition is that in addition to assuming
that the returned memory block is newly-allocated, gcc now assumes
that it does not contain any valid pointers. This is OK for
uninitialized or zero-initialized memory returned by dbus_malloc()
or dbus_malloc0(), but not valid for dbus_realloc(), which might be
used for a dynamically-sized array of (structures containing)
valid pointers.
See https://gitlab.gnome.org/GNOME/glib/issues/1465
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107741
This saves around 32% of the size of the archive.
[smcv: Rebased onto current master]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107630
The changelogs (and the commits themselves, converted to git format)
are still in the git history if anyone needs them.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
https://bugs.freedesktop.org/show_bug.cgi?id=107630
Running the "embedded tests" through valgrind revealed that before this
commit, we would have been willing to read up to 3 bytes off the end of
a message if the message is truncated part way through a boolean. Any
practical allocator will round up allocations to the next 32-bit (or
larger) boundary, so in practice this will not leave the memory buffer
(and in particular did not crash during unit testing), but it could read
uninitialized contents.
On little-endian CPUs, an attacker might be able to use this to learn
whether up to 3 bytes of uninitialized memory in the dbus-daemon
were all-zero (their crafted message would be relayed) or not (their
connection would be disconnected for sending an invalid message). On
big-endian CPUs, an attacker might be able to use this to learn whether
up to 3 bytes were all-zeroes (relayed to a cooperating peer), 0-2
bytes of all-zeroes followed by 0x01 (relayed to a cooperating peer),
or something else (disconnected). This is not believed to be exploitable
to leak interesting information.
Fixes: 62e46533 "hardcode dbus_bool_t to 32 bits"
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107332
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Thiago Macieira <thiago@kde.org>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
It's just painfully slow, particularly when we fork (as we do in
test-bus to test service activation).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
Instead of having separate test wrappers for the cases that do and
don't take a DBusConnection, we can just pass a NULL DBusConnection
to the one that doesn't.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
These tests are very reliant on their custom LOG_COMPILER,
which AX_VALGRIND_CHECK replaces.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
We don't need to do this for connections that were never set up
with the main loop.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
If re-initializing the string fails, it will be left in a state
where it has a length of 0 and a NULL buffer. That's valid to
"free", but not valid to pass to rmdir().
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
Not all of these tests will be fully valgrind-clean yet (or perhaps
ever), but it's easier to add this to all of them than to think
about it.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
AX_VALGRIND_CHECK overrides LOG_COMPILER, which means we can't rely
on running under glib-tap-test.sh. Default to TAP mode by modifying
our (effective) argv instead.
If you really want the default behaviour (unstructured output) this
can still be achieved by adding some arguments that are a no-op,
such as `-m quick`.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
test_server_setup() takes a reference to the DBusServer, so we need
to release that ref by calling test_server_shutdown().
test_server_shutdown() also disconnects the server, so we don't need
to do that.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
If an implementation fails to listen, and a subsequent implementation
succeeds, then we would have leaked this. Detected by running
tests/loopback.c under valgrind.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
We currently get away with this because the connection isn't fully
freed before we exit, but the connection is meant to own the result
of _dbus_connection_get_credentials() (it's "(transfer none)" in
GLib terminology).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
If they didn't, we'd probably leak the server and/or the error.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
The only place this is set in practice is in dbus-server-win.c, which
does not set the error. If it did, dbus_server_listen() would leak it.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
Using strncpy (buffer, str, strlen (str)) is a "code smell" that
might indicate a serious bug (it effectively turns strncpy into
strcpy), and gcc 8 now warns about it. In fact we avoided the bug
here, but it wasn't at all obvious.
We already checked that path_len is less than or equal to
_DBUS_MAX_SUN_PATH_LENGTH, which is 99, chosen to be strictly less
than the POSIX minimum sizeof(sun_path) >= 100, so we couldn't
actually be overflowing the available buffer.
The new static assertion in this commit matches a comment above the
definition of _DBUS_MAX_SUN_PATH_LENGTH: we define
_DBUS_MAX_SUN_PATH_LENGTH to 99, because POSIX says struct
sockaddr_un's sun_path member is at least 100 bytes (including space
for a \0 terminator). dbus will now fail to compile on
platforms that are non-POSIX-compliant in this way, except for Windows.
We zeroed the struct sockaddr_un before writing into it, so stopping
one byte short of the end of sun_path ensures that we get \0
termination.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107350
Reviewed-by: Thiago Macieira <thiago@kde.org>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
In gcc 8, -Wall -Wextra includes -Wcast-function-type, which warns
about passing an extra (unwanted) parameter to callbacks. Instead
of using g_list_foreach(), open-code the equivalent.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349
Reviewed-by: Thiago Macieira <thiago@kde.org>
In gcc 8, -Wall -Wextra includes -Wcast-function-type, which warns
about passing an extra (unwanted) parameter to callbacks. Instead
of using _dbus_list_foreach(), open-code the equivalent here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349
Reviewed-by: Thiago Macieira <thiago@kde.org>
In gcc 8, -Wall -Wextra includes -Wcast-function-type, which warns
about passing an extra (unwanted) parameter to callbacks. Instead
of using _dbus_list_foreach(), add a function to do what we actually
wanted here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349
Reviewed-by: Thiago Macieira <thiago@kde.org>
This was the only remaining symbol using the long prefix. Renaming it
gives us one consistent rule: symbols starting with dbus are public,
symbols starting with _dbus are not.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349
Reviewed-by: Thiago Macieira <thiago@kde.org>
As in the previous commit, this prefix is meaningless in translation
units that don't get compiled into libdbus.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349
Reviewed-by: Thiago Macieira <thiago@kde.org>
The naming convention dbus_internal_do_not_use_foo() was for functions
that had to be exported by libdbus but called by the embedded tests.
This is obsolete (in favour of _dbus_foo()) now that we have
DBUS_PRIVATE_EXPORT, and is doubly useless in this case because these
functions aren't even in libdbus - they're local to dbus-message-util.c.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349
Reviewed-by: Philip Withnall <withnall@endlessm.com>
In gcc 8, -Wall -Wextra includes -Wcast-function-type, which warns
about casting a function pointer to an incompatible type. In this
case the cast was because we were ignoring the void * argument, which
in this case is NULL. Since this function is only used within
dbus-message-util.c anyway, we might as well just use the correct
signature and remove the cast.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349
Reviewed-by: Thiago Macieira <thiago@kde.org>
This is a bit more convenient than fetching it as-needed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105656
