Previously, the hash table indexed by uid (or gid) took ownership of the
single reference to the heap-allocated struct, and the hash table
indexed by username (or group name) had a borrowed pointer to the same
struct that exists in the other hash table.
However, this can break down if you have two or more distinct usernames
that share a numeric identifier. This is generally a bad idea, because
the user-space model in such situations does not match the kernel-space
reality, and in particular there is no effective kernel-level security
boundary between such users, but it is sometimes done anyway.
In this case, when the second username is looked up in the userdb, it
overwrites (replaces) the entry in the hash table that is indexed by
uid, freeing the DBusUserInfo. This results in both the key and the
value in the hash table that is indexed by username becoming dangling
pointers (use-after-free), leading to undefined behaviour, which is
certainly not what we want to see when doing access control.
An equivalent situation can occur with groups, in the rare case where
a numeric group ID has two names (although I have not heard of this
being done in practice).
Solve this by reference-counting the data structure. There are up to
three references in practice: one held temporarily while the lookup
function is populating and storing it, one held by the hash table that
is indexed by uid, and one held by the hash table that is indexed by
name.
Simon McVittie [Tue, 30 Jun 2020 18:13:17 +0000 (19:13 +0100)]
userdb: Make lookups return a const pointer
This makes it more obvious that the returned pointer points to a
struct owned by the userdb, which must not be freed or have its
contents modified, and is only valid to dereference until the next
modification to the userdb's underlying hash tables (which in practice
means until the lock is released, because after that we have no
guarantees about what might be going on in another thread).
Simon McVittie [Thu, 16 Apr 2020 13:45:11 +0000 (14:45 +0100)]
sysdeps-unix: On MSG_CTRUNC, close the fds we did receive
MSG_CTRUNC indicates that we have received fewer fds that we should
have done because the buffer was too small, but we were treating it
as though it indicated that we received *no* fds. If we received any,
we still have to make sure we close them, otherwise they will be leaked.
On the system bus, if an attacker can induce us to leak fds in this
way, that's a local denial of service via resource exhaustion.
[Backport to dbus-1.10: Change signedness of iterator due to
commit ab8cb96e "_dbus_read_socket_with_unix_fds: make n_fds unsigned"
not having been applied to this branch.]
Each connection that is an active monitor holds a pointer to its own
link in this list, via BusConnectionData.link_in_monitors. We can't
validly free the list while these pointers exist: that would be a
use-after-free, when each connection gets disconnected and tries to
remove itself from the list.
Instead, let each connection remove itself from the list, then assert
that the list has become empty.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 23 Apr 2018 16:38:56 +0000 (17:38 +0100)]
doc: Install highlight.pack.js if present
Newer versions of yelp-build use this instead of a jQuery syntax
highlighter.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=106171 Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: Also add it to .gitignore as suggested] Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 49ad5b110fd5f5f4e41405d98007a11d8eb741f7)
Simon McVittie [Sat, 21 Apr 2018 18:35:41 +0000 (19:35 +0100)]
doc: Only install ancillary files from yelp-build if they exist
Newer versions of yelp-build don't install jquery.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=106171 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit bab857fb6f75ffe0ac3771de4b8272ad97623a2c)
Simon McVittie [Thu, 30 May 2019 11:58:28 +0000 (12:58 +0100)]
test: Add basic test coverage for DBUS_COOKIE_SHA1
We don't actually complete successful authentication, because that
would require us to generate a cookie and compute the correct SHA1,
which is difficult to do in a deterministic authentication script.
However, we do assert that dbus#269 (CVE-2019-12749) has been fixed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 30 May 2019 11:53:03 +0000 (12:53 +0100)]
auth: Reject DBUS_COOKIE_SHA1 for users other than the server owner
The DBUS_COOKIE_SHA1 authentication mechanism aims to prove ownership
of a shared home directory by having the server write a secret "cookie"
into a .dbus-keyrings subdirectory of the desired identity's home
directory with 0700 permissions, and having the client prove that it can
read the cookie. This never actually worked for non-malicious clients in
the case where server uid != client uid (unless the server and client
both have privileges, such as Linux CAP_DAC_OVERRIDE or traditional
Unix uid 0) because an unprivileged server would fail to write out the
cookie, and an unprivileged client would be unable to read the resulting
file owned by the server.
Additionally, since dbus 1.7.10 we have checked that ~/.dbus-keyrings
is owned by the uid of the server (a side-effect of a check added to
harden our use of XDG_RUNTIME_DIR), further ruling out successful use
by a non-malicious client with a uid differing from the server's.
Joe Vennix of Apple Information Security discovered that the
implementation of DBUS_COOKIE_SHA1 was susceptible to a symbolic link
attack: a malicious client with write access to its own home directory
could manipulate a ~/.dbus-keyrings symlink to cause the DBusServer to
read and write in unintended locations. In the worst case this could
result in the DBusServer reusing a cookie that is known to the
malicious client, and treating that cookie as evidence that a subsequent
client connection came from an attacker-chosen uid, allowing
authentication bypass.
This is mitigated by the fact that by default, the well-known system
dbus-daemon (since 2003) and the well-known session dbus-daemon (in
stable releases since dbus 1.10.0 in 2015) only accept the EXTERNAL
authentication mechanism, and as a result will reject DBUS_COOKIE_SHA1
at an early stage, before manipulating cookies. As a result, this
vulnerability only applies to:
* system or session dbus-daemons with non-standard configuration
* third-party dbus-daemon invocations such as at-spi2-core (although
in practice at-spi2-core also only accepts EXTERNAL by default)
* third-party uses of DBusServer such as the one in Upstart
Avoiding symlink attacks in a portable way is difficult, because APIs
like openat() and Linux /proc/self/fd are not universally available.
However, because DBUS_COOKIE_SHA1 already doesn't work in practice for
a non-matching uid, we can solve this vulnerability in an easier way
without regressions, by rejecting it early (before looking at
~/.dbus-keyrings) whenever the requested identity doesn't match the
identity of the process hosting the DBusServer.
Signed-off-by: Simon McVittie <smcv@collabora.com> Closes: https://gitlab.freedesktop.org/dbus/dbus/issues/269 Closes: CVE-2019-12749
Signed-off-by: Thomas Zimmermann <tdz@users.sourceforge.net> Reviewed-by: Simon McVittie <smcv@debian.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=97357
(cherry picked from commit ddbc44adb2709f6dc248364f02b8b4207ea5a1af)
Simon McVittie [Fri, 17 Aug 2018 14:42:17 +0000 (15:42 +0100)]
activation: Don't leak if delivering activation message is forbidden
This is technically a denial of service because the dbus-daemon will
run out of memory eventually, but it's a very slow and noisy one,
because all the rejected messages are also very likely to have
been logged to the system log.
Detected by AddressSanitizer.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/234 Reviewed-by: pwithnall
Simon McVittie [Thu, 4 Oct 2018 16:26:42 +0000 (17:26 +0100)]
ci: Mark many Gitlab jobs to be run manually
freedesktop.org Gitlab doesn't currently have enough test runners
available to run all of this every time. For higher-risk changes
(for example those that change the build system) we can run the
complete set through the web UI.
Simon McVittie [Wed, 3 Oct 2018 16:25:43 +0000 (17:25 +0100)]
ci: Add Gitlab-CI configuration
This uses the same shell scripts as Travis-CI, with slightly different
settings. We use Docker containers for all our Gitlab-CI runs, so take
the opportunity to use Debian 9 'stretch' as our baseline, and
relegate Ubuntu 14.04 'trusty' to to a secondary build.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=108177 Acked-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit 60933c09e9e891f74f0102fabe22d29a1a7ae5c5)
Simon McVittie [Wed, 3 Oct 2018 16:51:35 +0000 (17:51 +0100)]
ci: Explicitly install cmake
Travis-CI workers have cmake preinstalled, but Gitlab-CI Docker images
typically don't.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=108177 Acked-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit 907832e00849ca454322052981dbb122ea537506)
Simon McVittie [Wed, 3 Oct 2018 16:51:49 +0000 (17:51 +0100)]
ci: Teach ci-install.sh to install wine on Debian 9 'stretch'
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=108177 Acked-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit 408b222a9fc61327cd7be385b6705f30f0c38802)
Simon McVittie [Tue, 25 Jul 2017 11:43:01 +0000 (12:43 +0100)]
travis-ci: Enable/disable more features in various builds
In the debug build, enable features that are off by default. In the
reduced build, explicitly disable features, some of which are
on by default. In the legacy build, check that we can compile the
default feature-set without inotify, dnotify, systemd, etc.
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: Rebase onto 1.13.x branch, fix minor conflicts] Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101354
(cherry picked from commit 3c031ef5aa1f7f53c6344781cb38b78abe44dc96)
Simon McVittie [Thu, 23 Aug 2018 08:01:03 +0000 (09:01 +0100)]
Do not apply __attribute__((__malloc__)) to dbus_realloc()
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
Simon McVittie [Thu, 12 Jul 2018 18:11:05 +0000 (19:11 +0100)]
validate_body_helper: Bounds-check before validating booleans
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>
(cherry picked from commit e93a775e68daeda5c95984452aee6327e31c17dd)
Simon McVittie [Mon, 23 Jul 2018 17:52:01 +0000 (18:52 +0100)]
sysdeps: Reassure gcc 8 that we are not overflowing struct sockaddr_un
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>
(cherry picked from commit f429631365ba59a1749438af2184cab138a31772)
Simon McVittie [Mon, 23 Jul 2018 17:25:18 +0000 (18:25 +0100)]
build: Disable new gcc 8 warning -Wcast-function-type
The foreach(list, (DBusForeachFunction) free, NULL) idiom seems too
entrenched to remove it from stable branches.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Simon McVittie [Tue, 20 Feb 2018 11:45:39 +0000 (11:45 +0000)]
Add a unit test for the dbus-daemon resetting its fd limit
Reviewed-by: David King <dking@redhat.com>
[smcv: Fix typo in cmake macro name] Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105165
(cherry picked from commit 49ca421997d91d3e01626b2c92a826e6a5db0b2f)
Simon McVittie [Tue, 20 Feb 2018 12:20:35 +0000 (12:20 +0000)]
cmake: Check for getrlimit, setrlimit
This gives us feature parity with the Autotools build system for this
particular area, and in particular means a system dbus-daemon built
with cmake can expand its fd limit.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105165
(cherry picked from commit a146724f2f7610bc0a968d03a3f20481c03a6a37)
David King [Wed, 7 Feb 2018 14:37:24 +0000 (14:37 +0000)]
bus: raise fd limits before dropping privs
Startup ordering was changed in #92832 to ensure that SELinux audit
messages could be sent. As a side effect, the raising of file descriptor
limits was moved to after the dropping of root privileges, resulting in
the limit change always failing.
Move the raise_file_descriptor_limit() call to ensure that it is called
before dropping root privileges.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105165
Bug-RedHat: https://bugzilla.redhat.com/show_bug.cgi?id=1529044
[smcv: Call raise_file_descriptor_limit() even if !context->user] Reviewed-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 6e42964f5f850f4108fd8f7f3cd385ab4d60f9f6)
Simon McVittie [Mon, 25 Sep 2017 15:19:39 +0000 (16:19 +0100)]
dbus-send: Reassure the compiler that secondary_type is initialized
It's initialized to a non-trivial value whenever container_type
is DBUS_TYPE_DICT_ENTRY, and subsequently only used if
container_type is DBUS_TYPE_DICT_ENTRY, but Debian's gcc 7.2.0-7
doesn't seem to be able to infer that any more, causing build failure
under -Werror=maybe-uninitialized.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102979 Reviewed-by: Philip Withnall <withnall@endlessm.com>
Simon McVittie [Mon, 25 Sep 2017 13:57:38 +0000 (14:57 +0100)]
monitor: use the addressed_recipient to select matches
This means we respect the destination keyword in arguments to
BecomeMonitor.
In bus_dispatch(), this means that we need to defer capturing until
we have decided whether there is an addressed recipient; so instead
of capturing once, we capture at each leaf of the decision tree.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92074 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk> Reviewed-by: Lars Uebernickel <lars@uebernic.de>
(cherry picked from commit f3be583b40dadfd78ddefbc9fb3fa182bafde949) Signed-off-by: Simon McVittie <smcv@collabora.com>
Alan Coopersmith [Fri, 11 Aug 2017 01:50:36 +0000 (18:50 -0700)]
Fix -Werror=declaration-after-statement build failure on Solaris
dbus-sysdeps-unix.c: In function ‘_dbus_read_credentials_socket’:
dbus-sysdeps-unix.c:2061:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
adt_session_data_t *adth = NULL;
^
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102145 Reviewed-by: Philip Withnall <withnall@endlessm.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
sysdeps: increase listen() backlog of AF_UNIX sockets to SOMAXCONN
Previously, the listen() backlog was set to an arbitrary 30. This means
that if dbus-daemon is overloaded only 30 more connections may be queued
by the kernel, before connect() fails with EAGAIN. (Note that EAGAIN !=
EINPROGRESS -- the latter is what is returned if a connection is queued
and being processed for asynchronous sockets; EAGAIN in this case is
really an error, that cannot be recovered from).
Most software simply sets SOMAXCONN as backlog for AF_UNIX sockets, to
allow queuing of as many connections as the kernel allows. SOMAXCONN is
128 on Linux, which is not particularly high, but at least higher than
30.
This patch changes dbus-daemon to do the same.
I noticed this when flooding dbus-daemon with a lot of connections,
where it pretty quickly ceased to respond, much earlier than it really
should.
Note that the backlog has nothing to do with the number of concurrent
connections allowed, it simply controls how many queued, but not
accept()ed connections there may be on the listening socket.
(cherry picked from commit 12bd6e893c91430fdbdf8a27087d4a792b04eef9)
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95264
Bug-Debian: https://bugs.debian.org/872144 Reviewed-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Thiago Macieira <thiago@kde.org>
Simon McVittie [Fri, 21 Jul 2017 09:46:39 +0000 (10:46 +0100)]
config-loader-expat: Tell Expat not to defend against hash collisions
By default, Expat uses cryptographic-quality random numbers as a salt for
its hash algorithm, and since 2.2.1 it gets them from the getrandom
syscall on Linux. That syscall refuses to return any entropy until the
kernel's CSPRNG (random pool) has been initialized. Unfortunately, this
can take as long as 40 seconds on embedded devices with few entropy
sources, which is too long: if the system dbus-daemon blocks for that
length of time, important D-Bus clients like systemd and systemd-logind
time out and fail to connect to it.
We're parsing small configuration files here, and we trust them
completely, so we don't need to defend against hash collisions: nobody
is going to be crafting them to cause pathological performance.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101858 Signed-off-by: Simon McVittie <smcv@debian.org> Tested-by: Christopher Hewitt <hewitt@ieee.org> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Simon McVittie [Fri, 7 Jul 2017 11:12:24 +0000 (12:12 +0100)]
test/name-test: Be compatible with Python 3
configure.ac will detect PYTHON=python3 if there is no python
executable in the PATH.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101716 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Wed, 5 Jul 2017 14:32:40 +0000 (15:32 +0100)]
tests: Make tests fail if they try to connect to the real session bus
It is too easy for a developer working in an environment that has a
session bus to write tests that pass locally, but fail in minimal
environments. This is also risky because the tests might do
destructive things on the developer's real session bus. We can avoid
connecting to the session bus by consistently removing its address
from the environment, and replacing it with something that will
always fail.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101698
Simon McVittie [Wed, 5 Jul 2017 14:30:05 +0000 (15:30 +0100)]
test/dbus-daemon: Unset DBUS_SESSION_BUS_ADDRESS
When we intend to exercise the default behaviour in the absence of
DBUS_SESSION_BUS_ADDRESS (but with an XDG_RUNTIME_DIR present), it would
help if we unset DBUS_SESSION_BUS_ADDRESS. Otherwise we'll just connect
to the real session bus, if there is one.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101698
Simon McVittie [Wed, 5 Jul 2017 14:28:08 +0000 (15:28 +0100)]
name-test: Backport dbus-run-session wrapper from git master
test-pending-call-disconnected relies on being run under a session bus.
On master, the TESTS in this directory all get that treatment, but
in dbus-1.10 they do not. This caused test-pending-call-disconnected
to fail in minimal environments like travis-ci where there is no
developer-initiated session bus.
Backport part of commit ec6b220 "name-test: run most C tests directly,
not via run-test.sh" to wrap it in dbus-run-session. This is better
than putting it in run-test.sh because this way, its TAP output is
parsed directly by Automake.
It also has the side benefit of exercising dbus-run-session in the
automated tests.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101698
Simon McVittie [Tue, 4 Jul 2017 14:38:57 +0000 (15:38 +0100)]
dbus_message_iter_open_container: Don't leak signature on failure
If we run out of memory while calling _dbus_type_writer_recurse()
(which is impossible for most contained types, but can happen for
structs and dict-entries), then the memory we allocated in the call to
_dbus_message_iter_open_signature() will still be allocated, and we
have to free it in order to return to the state of the world prior to
calling open_container().
One might reasonably worry that this change can break callers that use
this (incorrect) pattern:
if (!dbus_message_iter_open_container (outer, ..., inner))
{
dbus_message_iter_abandon_container (outer, inner);
goto fail;
}
/* now we know inner is open, and we must close it later */
However, testing that pattern with _dbus_test_oom_handling()
demonstrates that it already dies with a DBusString assertion failure
even before this commit.
This is all concerningly fragile, and I think the next step should be
to zero out DBusMessageIter instances when they are invalidated, so
that a "double-free" is always detected.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
(cherry picked from commit 031aa2ceb3dfff373e7b398dfc5d020d77262512)
Simon McVittie [Tue, 4 Jul 2017 13:13:15 +0000 (14:13 +0100)]
dbus_message_iter_append_basic: Don't leak signature if appending fd fails
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
(cherry picked from commit 8384e795516066960bb9fcfbfe138f569420edb9)
Simon McVittie [Tue, 4 Jul 2017 12:31:38 +0000 (13:31 +0100)]
dbus_message_append_args_valist: Don't leak memory on inappropriate type
Found by source code inspection while trying to debug an unrelated
leak.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
(cherry picked from commit 6b7bdb105b120b3db312de93af94af1bb6a2a474)
Simon McVittie [Mon, 5 Jun 2017 17:16:42 +0000 (18:16 +0100)]
transport: Don't pile up errors for semicolon-separated components
If we somehow get an autolaunch address with multiple
semicolon-separated components, and one of them fails, then we will
hit an assertion failure when we try the next one.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101257
(cherry picked from commit ecdcb86bff42d2bb9cac617bf79f0aa3d47676d9)
Simon McVittie [Wed, 6 May 2015 08:17:06 +0000 (09:17 +0100)]
Doxyfile.in: do not put timestamps in HTML
The build timestamp is not particularly useful (the version number of
the package is already present in the HTML), and it prevents the build
from being reproducible. See <https://reproducible-builds.org/> for more
information.
Signed-off-by: Simon McVittie <smcv@debian.org> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=100692
(cherry picked from commit 0310ead0022b3537392869cc2ed3296ba1a7c17d)
Philip Withnall [Wed, 5 Apr 2017 10:36:12 +0000 (11:36 +0100)]
test: Fix a couple of memory leaks in test-corrupt
Spotted while testing bug #100568.
Signed-off-by: Philip Withnall <withnall@endlessm.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=100568
Philip Withnall [Wed, 5 Apr 2017 10:35:27 +0000 (11:35 +0100)]
test: Fix reading off the end of an array in test-corrupt
One level of pointer indirection too many when passing the arguments to
dbus_message_append_args().
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=100568 Signed-off-by: Philip Withnall <withnall@endlessm.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
Initialize SELinux and Apparmor after capabilities are set
avc_init() in the SELinux code path is creating a new thread, we need to
set to capabilities before it gets created so it has the permission to
send audit messages.
It also make more sense to open the audit netlink before the different
logging callbacks are set.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92832
[smcv: add comments explaining why initialization must happen in this
specific order] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=857660 Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
(cherry picked from commit a3a5935a0a038c3b44c61ce5719f0f7e647b96c6)
Simon McVittie [Wed, 15 Feb 2017 17:24:14 +0000 (17:24 +0000)]
activation test: Fix time-of-check/time-of-use bug waiting to happen
Creating a directory is atomic, stat'ing it to see whether to remove
it is very much not.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Simon McVittie [Wed, 15 Feb 2017 16:32:04 +0000 (16:32 +0000)]
Change _dbus_create_directory to fail for existing directories
If we don't trap EEXIST and its Windows equivalent, we are unable to
detect the situation where we create an ostensibly unique
subdirectory in a shared /tmp, but an attacker has already created it.
This affects dbus-nonce (the nonce-tcp transport) and the activation
reload test.
Add a new _dbus_ensure_directory() for the one case where we want it to
succeed even on EEXIST: the DBUS_COOKIE_SHA1 keyring, which we know
we are creating in our own trusted "official" $HOME. In the new
transient service support on Bug #99825, ensure_owned_directory()
would need the same treatment.
We are not treating this as a serious security problem, because the
nonce-tcp transport is rarely enabled on Unix and there are multiple
mitigations.
The nonce-tcp transport creates a new unique file with O_EXCL and 0600
(private to user) permissions, then overwrites the requested filename
via atomic-overwrite, so the worst that could happen there is that an
attacker could place a symbolic link matching the name of a directory
we are going to create, causing a dbus-daemon configured for nonce-tcp
to traverse the symlink and atomically overwrite a file named "nonce"
in a directory of the attacker's choice, with new random contents that
are not known to the attacker. This seems unlikely to be exploitable
for anything worse than denial of service in practice. In mainline
Linux since 3.6, this attack is also defeated by the
fs.protected_symlinks sysctl, which many distributions enable by default.
The activation reload test suffers from a classic symlink attack
due to time-of-check/time-of-use errors in its implementation, but as
part of the developer-only "embedded tests" that are only intended
to be run on a trusted machine, it is not treated as security-sensitive.
That code path will be fixed in a subsequent commit.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Simon McVittie [Mon, 28 Nov 2016 16:38:37 +0000 (16:38 +0000)]
travis-ci: Add and use infrastructure to build and test in Docker
Debian stable, Debian testing and Ubuntu LTS provide a reasonable
spectrum of old and new distributions. I'm only doing one build on
each to avoid a combinatorial explosion of options.
The Docker images don't have any deb-src apt sources set up, so don't
use `apt-get build-dep`; just include dependencies manually.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98889
Simon McVittie [Mon, 28 Nov 2016 13:11:48 +0000 (13:11 +0000)]
travis-ci: add an install script instead of open-coding it in .travis.yml
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
[smcv: move comment to install script as suggested] Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98889
Simon McVittie [Mon, 28 Nov 2016 13:04:13 +0000 (13:04 +0000)]
travis-ci: introduce maybe_fail_tests() to make test failure more obvious
Taken from the version I added to OSTree.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98889
projects / thirdparty / dbus.git / log
