Specifically, this will allow ActivationFailure messages from our
own uid or from root, but reject them otherwise, even if the bus
configuration for who can own org.freedesktop.systemd1 is entirely
wrong due to something like CVE-2014-8148.
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98157
In principle this could lead to arbitrary memory overwrite via
a format string attack in the message received from systemd,
resulting in arbitrary code execution.
This is not believed to be an exploitable security vulnerability on the
system bus in practice: it can only be exploited by the owner of the
org.freedesktop.systemd1 bus name, which is restricted to uid 0, so
if systemd is attacker-controlled then the system is already doomed.
Similarly, if a systemd system unit mentioned in the activation failure
message has an attacker-controlled name, then the attacker likely already
has sufficient access to execute arbitrary code as root in any case.
However, prior to dbus 1.8.16 and 1.9.10, due to a missing check for
systemd's identity, unprivileged processes could forge activation
failure messages which would have gone through this code path.
We thought at the time that this was a denial of service vulnerability
(CVE-2015-0245); this bug means that it was in fact potentially an
arbitrary code execution vulnerability.
Bug found using -Wsuggest-attribute=format and -Wformat-security.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98157
The const void* 'value' pointer that is passed the address of a
uint32_t here eventually ends up in _dbus_marshal_write_basic(), which
casts it to a DBusBasicValue, a union type that has an alignment of
eight on 64-bit platforms and is therefore more-aligned than the
uint32.
The read of a value of a more-aligned type through a pointer to a less
-aligned type is undefined behaviour.
Fix by storing the uint32 in a DBusBasicValue and passing that instead.
Found by UBSan:
dbus/dbus/dbus-marshal-basic.c:832:14: runtime error: member access within misaligned address 0x7fdb8dac3a04 for type 'const union DBusBasicValue', which requires 8 byte alignment
0x7fdb8dac3a04: note: pointer points here
4a 87 b5 71 01 00 00 00 40 7d 01 00 00 61 00 00 10 3b ac 8d db 7f 00 00 2c 2a 3e 94 db 7f 00 00
^
#0 0x7fdb9444a2c3 in _dbus_marshal_write_basic dbus/dbus/dbus-marshal-basic.c:832
#1 0x7fdb943d22fb in _dbus_type_writer_write_basic_no_typecode dbus/dbus/dbus-marshal-recursive.c:1605
#2 0x7fdb943d64e9 in _dbus_type_writer_write_basic dbus/dbus/dbus-marshal-recursive.c:2327
#3 0x7fdb943c52a6 in write_basic_field dbus/dbus/dbus-marshal-header.c:318
#4 0x7fdb943c919e in _dbus_header_set_field_basic dbus/dbus/dbus-marshal-header.c:1321
#5 0x7fdb943e1349 in dbus_message_set_reply_serial dbus/dbus/dbus-message.c:1173
Signed-off-by: Marc Mutz <marc@kdab.net>
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98035
In Debian bug <https://bugs.debian.org/829348>, lightdm appears to
have been starting dbus-launch with at least one of the three
standard fds 0, 1, 2 (stdin, stdout, stderr) closed. This resulted
in the dbus-daemon's epoll_create1() returning a fd less than 3.
Later, _dbus_become_daemon() replaces fds 0-2 with /dev/null. As a
result, a subsequent call to _dbus_loop_add_watch() for the reload
pipe resulted in calling epoll_ctl on the non-epoll fd pointing to
/dev/null, which fails with EINVAL, resulting in the dbus-daemon
exiting unsuccessfully.
Unix programs are not normally expected to behave correctly when
launched with the standard fds not already open; but at the same time,
X11 autolaunching means that dbus-launch (and hence the dbus-daemon)
can get started from an arbitrarily precarious situation.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97008
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Thiago Macieira <thiago@kde.org>
(cherry picked from commit c8f73a2a3a)
This function opens stdin, stdout, stderr pointing to /dev/null
if they aren't already open. Optionally, it can also replace
whatever is available on those fds with /dev/null.
To allow for use in contexts where only async-signal-safe functions
should be used, such as between fork() and a following exec(),
this function does not use conventional libdbus error handling
(which would require malloc). Instead, it sets errno and returns
an explanatory string.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97008
Signed-off-by: Simon McVittie <smcv@debian.org>
Reviewed-by: Thiago Macieira <thiago@kde.org>
(cherry picked from commit 69123a6bd2)
It should probably be used (see #97298) but the fact that it isn't
is breaking compatibility with gcc 6, so apply a quick workaround
while we look into what's wrong here.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97282
(cherry picked from commit 21d6118081)
If dbus-daemon or systemd replied to our method call with an error,
we would report it as "invalid arguments" instead of the true error
name and message.
Same root cause as <https://bugs.freedesktop.org/show_bug.cgi?id=96653>.
Signed-off-by: Simon McVittie <smcv@debian.org>
Otherwise HAVE_STDINT_H will not be defined or the var will not be
picked up from cache so builds could fail with errors like:
| ../../dbus-1.10.8/dbus/dbus-internals.h:239:8: error: ‘uintptr_t’ undeclared (first use in this function)
Signed-off-by: Ioan-Adrian Ratiu <adrian.ratiu@ni.com>
[smcv: fix Autoconf underquoting]
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
If dbus is running as systemd service, dbus daemon is running with
oom_score_adj -900 by OOMScoreAdjust=-900. And children will also have
same value with dbus daemon.
To avoid this, set the child itself values after fork () to 0.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=32851
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Some D-Bus daemon versions set multiple addresses separated by semi-colon,
which breaks sourcing of the file.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=94746
Signed-off-by: Thiago Macieira <thiago@kde.org>
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
This should detect anything that is only a build failure when embedded
tests or verbose mode are enabled, such as the missing include fixed
in commit 4858faf. I'm not enabling embedded tests and verbose mode
orthogonally because we don't want CI builds to take too long.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
(cherry picked from commit 8f6fe1451e)
Ensure that the babysitter thread already owns its one reference to the
babysitter when it starts up, and eliminates the race condition.
This patch requires that DBusBabysitter refcounting is thread-safe
and is based on an analysis and proposal of Simon Mc Vittie.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95191
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Previously, 64-bit (LP64 or LLP64) platforms would have had 32 bits
of padding between pad2 and pad3. We want to guarantee that an ISO C
compiler will copy the entire struct when assigning between structs,
but padding is not guaranteed to be copied, so we want to ensure that
the struct is "packed".
Statically assert that the old ABI is compatible with the new ABI.
Reviewed-by: Thiago Macieira <thiago@kde.org>
[smcv: change >= to == as Thiago requested]
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=94136
We already asserted that DBusMessageIter must be at least as large
as DBusMessageRealIter (so that casting DBusMessageIter * to
DBusMessageRealIter * does not result in overflowing the stack
variable). Also assert that it must have alignment requirements at
least as strict as those of DBusMessageRealIter * (so that casting
does not increase the required alignment).
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Thiago Macieira <thiago@kde.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=94136
This is useful when making static assertions about our types'
properties.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Thiago Macieira <thiago@kde.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=94136
