Alban Crequy [Tue, 20 May 2014 13:37:37 +0000 (14:37 +0100)]
CVE-2014-3477: deliver activation errors correctly, fixing Denial of Service
How it should work:
When a D-Bus message activates a service, LSMs (SELinux or AppArmor) check
whether the message can be delivered after the service has been activated. The
service is considered activated when its well-known name is requested with
org.freedesktop.DBus.RequestName. When the message delivery is denied, the
service stays activated but should not receive the activating message (the
message which triggered the activation). dbus-daemon is supposed to drop the
activating message and reply to the sender with a D-Bus error message.
However, it does not work as expected:
1. The error message is delivered to the service instead of being delivered to
the sender. As an example, the error message could be something like:
An SELinux policy prevents this sender from sending this
message to this recipient, [...] member="MaliciousMethod"
If the sender and the service are malicious confederates and agree on a
protocol to insert information in the member name, the sender can leak
information to the service, even though the LSM attempted to block the
communication between the sender and the service.
2. The error message is delivered as a reply to the RequestName call from
service. It means the activated service will believe it cannot request the
name and might exit. The sender could activate the service frequently and
systemd will give up activating it. Thus the denial of service.
The following changes fix the bug:
- bus_activation_send_pending_auto_activation_messages() only returns an error
in case of OOM. The prototype is changed to return TRUE, or FALSE on OOM
(and its only caller sets the OOM error).
- When a client is not allowed to talk to the service, a D-Bus error message
is pre-allocated to be delivered to the client as part of the transaction.
The error is not propagated to the caller so RequestName will not fail
(except on OOM).
[fixed a misleading comment -smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=78979 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Colin Walters <walters@verbum.org>
Simon McVittie [Tue, 5 Jun 2012 12:27:23 +0000 (13:27 +0100)]
Fix distcheck: remove potentially-read-only files from builddir
During distcheck, the srcdir is read-only. During "make all", cp may
preserve the read-only status of the file copied from the srcdir,
resulting in failure to overwrite it with an identical file during
"make check" (which depends on all-local).
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Mon, 10 Jun 2013 17:06:47 +0000 (18:06 +0100)]
CVE-2013-2168: _dbus_printf_string_upper_bound: copy the va_list for each use
Using a va_list more than once is non-portable: it happens to work
under the ABI of (for instance) x86 Linux, but not x86-64 Linux.
This led to _dbus_printf_string_upper_bound() crashing if it should
have returned exactly 1024 bytes. Many system services can be induced
to process a caller-controlled string in ways that
end up using _dbus_printf_string_upper_bound(), so this is a denial of
service.
Simon McVittie [Tue, 2 Oct 2012 08:34:48 +0000 (09:34 +0100)]
activation helper: when compiled for tests, do not reset system bus address
Otherwise, the tests try to connect to the real system bus, which will
often fail - particularly if you run the tests configured for the default
/usr/local (with no intention of installing the result), in which case
the tests would try to connect to /usr/local/var/run/dbus/system_bus_socket.
Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=52202
Geoffrey Thomas [Fri, 28 Sep 2012 05:02:06 +0000 (22:02 -0700)]
activation-helper: Ensure DBUS_STARTER_ADDRESS is set correctly
The fix for CVE-2012-3524 filters out all environment variables if
libdbus is used from a setuid program, to prevent various spoofing
attacks.
Unfortunately, the activation helper is a setuid program linking
libdbus, and this creates a regression for launched programs using
DBUS_STARTER_ADDRESS, since it will no longer exist.
Fix this by hardcoding the starter address to the default system bus
address.
Signed-off-by: Geoffrey Thomas <gthomas@mokafive.com> Signed-off-by: Colin Walters <walters@verbum.org>
Colin Walters [Wed, 22 Aug 2012 14:03:34 +0000 (10:03 -0400)]
CVE-2012-3524: Don't access environment variables or run dbus-launch when setuid
This matches a corresponding change in GLib. See
glib/gutils.c:g_check_setuid().
Some programs attempt to use libdbus when setuid; notably the X.org
server is shipped in such a configuration. libdbus never had an
explicit policy about its use in setuid programs.
I'm not sure whether we should advertise such support. However, given
that there are real-world programs that do this currently, we can make
them safer with not too much effort.
Better to fix a problem caused by an interaction between two
components in *both* places if possible.
How to determine whether or not we're running in a privilege-escalated
path is operating system specific. Note that GTK+'s code to check
euid versus uid worked historically on Unix, more modern systems have
filesystem capabilities and SELinux domain transitions, neither of
which are captured by the uid comparison.
On Linux/glibc, the way this works is that the kernel sets an
AT_SECURE flag in the ELF auxiliary vector, and glibc looks for it on
startup. If found, then glibc sets a public-but-undocumented
__libc_enable_secure variable which we can use. Unfortunately, while
it *previously* worked to check this variable, a combination of newer
binutils and RPM break it:
http://www.openwall.com/lists/owl-dev/2012/08/14/1
So for now on Linux/glibc, we fall back to the historical Unix version
until we get glibc fixed.
On some BSD variants, there is a issetugid() function. On other Unix
variants, we fall back to what GTK+ has been doing.
Reported-by: Sebastian Krahmer <krahmer@suse.de> Signed-off-by: Colin Walters <walters@verbum.org>
Conflicts:
dbus/dbus-sysdeps-unix.c
Antoine Jacoutot [Wed, 25 Apr 2012 18:04:07 +0000 (19:04 +0100)]
use cp and mkdir -p instead of install within source tree
$(INSTALL) and $(INSTALL_DATA) try to change ownerships to root:bin when
copying tests to builddir. Presumably this is a difference in behaviour
between GNU and BSD install(1): the one in GNU coreutils doesn't try-and-fail
to change ownership if you're not root.
[Commit message added by smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=48127 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
David Zeuthen [Thu, 12 Apr 2012 15:16:48 +0000 (11:16 -0400)]
Avoid using monotonic time in the DBUS_COOKIE_SHA1 authentication method
When libdbus-1 moved to using monotonic time support for the
DBUS_COOKIE_SHA1 authentication was broken, in particular
interoperability with non-libdbus-1 implementations such as GDBus.
The problem is that if monotonic clocks are available in the OS,
_dbus_get_current_time() will not return the number of seconds since
the Epoch so using it for DBUS_COOKIE_SHA1 will violate the D-Bus
specification. If both peers are using libdbus-1 it's not a problem
since both ends will use the wrong time and thus agree. However, if
the other end is another implementation and following the spec it will
not work.
First, we change _dbus_get_current_time() back so it always returns
time since the Epoch and we then rename it _dbus_get_real_time() to
make this clear. We then introduce _dbus_get_monotonic_time() and
carefully make all current users of _dbus_get_current_time() use it,
if applicable. During this audit, one of the callers,
_dbus_generate_uuid(), was currently using monotonic time but it was
decided to make it use real time instead.
Signed-off-by: David Zeuthen <davidz@redhat.com> Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=48580
Martin Pitt [Mon, 23 Jan 2012 11:11:24 +0000 (11:11 +0000)]
Port to glib 2.31.x g_thread API
g_thread_init() is deprecated since glib 2.24, call g_type_init() instead.
Bump glib requirement accordingly.
g_thread_create is deprecated since 2.31, use g_thread_new() instead. When
building with a glib earlier than 2.31, provide a backwards compatibility shim.
[Added a comment about why we're using g_type_init() in a test that
doesn't otherwise use GObject -smcv]
[Applied to 1.4 despite just being a deprecation fix because it also fixes
linking with GLib 2.32, in which gthread has been removed from gobject's
Requires and moved to Requires.private, Debian #665665 -smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=44413
Bug-Debian: http://bugs.debian.org/665665 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Marc Mutz [Thu, 16 Feb 2012 07:43:40 +0000 (08:43 +0100)]
dbus-protocol.h: compile under C++11
C++11 compilers have a feature called 'user-defined string literals' which
allow arbitrary string suffixes to have user-defined meaning.
This makes code that concatenates macros with string literals without
intervening whitespace illegal under C++11. Fortunately, string literal
concatenation has allowed intervening whitespace since the dawn of time,
so the solution is to simply pad with spaces.
Tested (header) with GCC 4.7 (trunk).
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=46147 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Jack Nagel [Fri, 16 Dec 2011 06:21:21 +0000 (00:21 -0600)]
docs: correctly invoke man2html
man2html expects to find its input on stdin, so just passing the
filename will cause it to hang waiting for input.
[man2html 1.6g as shipped in Debian seems to be fine with files on the
command line, but apparently other versions aren't? -smcv]
Signed-off-by: Jack Nagel <jacknagel@gmail.com> Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=43875
Someone seems to have merged part of master into 1.4. Again. Let's go
back to the "last known good" point (the branch-point of some 1.4
branches I had locally), then we can cherry-pick the changes that
should have gone in.
Simon McVittie [Thu, 3 Feb 2011 17:59:07 +0000 (17:59 +0000)]
_dbus_listen_tcp_socket: avoid leaking listen_fd in unlikely circumstances
If getaddrinfo (with port == 0) succeeds, the kernel gives us a port when
we first listen on a socket, we jump back to redo_lookup_with_port,
and getaddrinfo (with the nonzero port) fails, we leak listen_fd and all
the fds in it.
From the department of "without static analysis we'd never have spotted
this", or possibly "backward goto considered harmful".
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=29881
Bug-NB: NB#180486 CID-2389 Reviewed-by: Will Thompson <will.thompson@collabora.co.uk>
Jesper Dam [Fri, 28 Oct 2011 13:54:30 +0000 (15:54 +0200)]
Find dbus-daemon executable next to dbus shared libaray on windows.
If the dbus shared library and the daemon executable are both in a dir
that is not part of the default search path and dbus-1.dll is dynamically
loaded with LoadLibrary(), it will fail to locate and launch the daemon
without this patch.
Simon McVittie [Thu, 21 Jul 2011 12:21:29 +0000 (13:21 +0100)]
update_desktop_file_entry: initialize return value properly, and actually return it
Since 1.4.4 (commit 75cfd97f) this function always returned FALSE. As far
as I can see this was actually harmless, because both of its callers
ignore any error that is not NoMemory (and treat it the same as success).
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=39230 Reviewed-by: Will Thompson <will.thompson@collabora.co.uk>
Simon McVittie [Tue, 20 Sep 2011 17:44:25 +0000 (18:44 +0100)]
Fix maintainer-upload-docs target to work out-of-tree
The STATIC_DOCS, DTDS and all dist_ files except XMLTO_OUTPUT are in the
source tree. The XMLTO_OUTPUT and the man2html output are in the build
tree, and the BONUS_FILES already have $(srcdir) in their names.
Also change the rules that generate the dbus-docs directory so that if
they fail, they leave behind a temporary directory, rather than leaving
behind a dbus-docs directory that causes make to not run those rules
if re-run; and change the rules to scp files to the server, to put a
trailing "/" on paths, ensuring that the tarball won't be uploaded as
"www" if the www directory doesn't already exist.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=41047 Reviewed-by: Will Thompson <will.thompson@collabora.co.uk>
Simon McVittie [Fri, 5 Aug 2011 12:40:44 +0000 (13:40 +0100)]
Use {}, not semicolon, when the statement of an "if" does nothing
The uses in bus/activation.c are also probably wrong because they ignore
the result of the test, but that's orthogonal.
(<https://bugs.freedesktop.org/show_bug.cgi?id=39858>)
projects / thirdparty / dbus.git / log
