[dbus/f20] Backport patches from dbus-1.6
Colin Walters
walters at fedoraproject.org
Wed Jul 2 22:31:31 UTC 2014
commit 2fbfaed9f8e2b1fc6f3534867bf79459a03abdc2
Author: Colin Walters <walters at verbum.org>
Date: Wed Jul 2 18:31:14 2014 -0400
Backport patches from dbus-1.6
- Fixes CVE-2014-3477 (fd.o#78979)
- Fixes CVE-2014-3532 (fd.o#80163)
- Fixes CVE-2014-3533 (fd.o#80469)
- Resolves #1115636
...77-deliver-activation-errors-correctly-fi.patch | 150 ++++++++++++++++++++
...MANYREFS-when-sending-recursive-fds-SCM_R.patch | 116 +++++++++++++++
...ontains-two-messages-with-fds-don-t-corru.patch | 40 +++++
dbus.spec | 15 ++-
4 files changed, 320 insertions(+), 1 deletions(-)
---
diff --git a/0001-CVE-2014-3477-deliver-activation-errors-correctly-fi.patch b/0001-CVE-2014-3477-deliver-activation-errors-correctly-fi.patch
new file mode 100644
index 0000000..374ace1
--- /dev/null
+++ b/0001-CVE-2014-3477-deliver-activation-errors-correctly-fi.patch
@@ -0,0 +1,150 @@
+From cab1c56bb9d70469128d2ae1c40539c0d3b30f13 Mon Sep 17 00:00:00 2001
+From: Alban Crequy <alban.crequy at collabora.co.uk>
+Date: Tue, 20 May 2014 14:37:37 +0100
+Subject: [PATCH] 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 at collabora.co.uk>
+Reviewed-by: Colin Walters <walters at verbum.org>
+---
+ bus/activation.c | 27 ++++++++++++++++++++-------
+ bus/activation.h | 3 +--
+ bus/services.c | 5 +++--
+ 3 files changed, 24 insertions(+), 11 deletions(-)
+
+diff --git a/bus/activation.c b/bus/activation.c
+index ea48a26..280cc01 100644
+--- a/bus/activation.c
++++ b/bus/activation.c
+@@ -1162,14 +1162,11 @@ bus_activation_service_created (BusActivation *activation,
+ dbus_bool_t
+ bus_activation_send_pending_auto_activation_messages (BusActivation *activation,
+ BusService *service,
+- BusTransaction *transaction,
+- DBusError *error)
++ BusTransaction *transaction)
+ {
+ BusPendingActivation *pending_activation;
+ DBusList *link;
+
+- _DBUS_ASSERT_ERROR_IS_CLEAR (error);
+-
+ /* Check if it's a pending activation */
+ pending_activation = _dbus_hash_table_lookup_string (activation->pending_activations,
+ bus_service_get_name (service));
+@@ -1186,6 +1183,9 @@ bus_activation_send_pending_auto_activation_messages (BusActivation *activation
+ if (entry->auto_activation && (entry->connection == NULL || dbus_connection_get_is_connected (entry->connection)))
+ {
+ DBusConnection *addressed_recipient;
++ DBusError error;
++
++ dbus_error_init (&error);
+
+ addressed_recipient = bus_service_get_primary_owners_connection (service);
+
+@@ -1193,8 +1193,22 @@ bus_activation_send_pending_auto_activation_messages (BusActivation *activation
+ if (!bus_dispatch_matches (transaction,
+ entry->connection,
+ addressed_recipient,
+- entry->activation_message, error))
+- goto error;
++ entry->activation_message, &error))
++ {
++ /* If permission is denied, we just want to return the error
++ * to the original method invoker; in particular, we don't
++ * want to make the RequestName call fail with that error
++ * (see fd.o #78979, CVE-2014-3477). */
++ if (!bus_transaction_send_error_reply (transaction, entry->connection,
++ &error, entry->activation_message))
++ {
++ bus_connection_send_oom_error (entry->connection,
++ entry->activation_message);
++ }
++
++ link = next;
++ continue;
++ }
+ }
+
+ link = next;
+@@ -1203,7 +1217,6 @@ bus_activation_send_pending_auto_activation_messages (BusActivation *activation
+ if (!add_restore_pending_to_transaction (transaction, pending_activation))
+ {
+ _dbus_verbose ("Could not add cancel hook to transaction to revert removing pending activation\n");
+- BUS_SET_OOM (error);
+ goto error;
+ }
+
+diff --git a/bus/activation.h b/bus/activation.h
+index 97f25b1..fc5d426 100644
+--- a/bus/activation.h
++++ b/bus/activation.h
+@@ -62,8 +62,7 @@ dbus_bool_t dbus_activation_systemd_failure (BusActivation *activation,
+
+ dbus_bool_t bus_activation_send_pending_auto_activation_messages (BusActivation *activation,
+ BusService *service,
+- BusTransaction *transaction,
+- DBusError *error);
++ BusTransaction *transaction);
+
+
+ #endif /* BUS_ACTIVATION_H */
+diff --git a/bus/services.c b/bus/services.c
+index 01a720e..584485b 100644
+--- a/bus/services.c
++++ b/bus/services.c
+@@ -588,8 +588,9 @@ bus_registry_acquire_service (BusRegistry *registry,
+ activation = bus_context_get_activation (registry->context);
+ retval = bus_activation_send_pending_auto_activation_messages (activation,
+ service,
+- transaction,
+- error);
++ transaction);
++ if (!retval)
++ BUS_SET_OOM (error);
+
+ out:
+ return retval;
+--
+1.8.3.1
+
diff --git a/0001-Handle-ETOOMANYREFS-when-sending-recursive-fds-SCM_R.patch b/0001-Handle-ETOOMANYREFS-when-sending-recursive-fds-SCM_R.patch
new file mode 100644
index 0000000..8d05a7b
--- /dev/null
+++ b/0001-Handle-ETOOMANYREFS-when-sending-recursive-fds-SCM_R.patch
@@ -0,0 +1,116 @@
+From 8c7176019fbc2e8fee41d93ce82ac2603fe57d67 Mon Sep 17 00:00:00 2001
+From: Alban Crequy <alban.crequy at collabora.co.uk>
+Date: Tue, 24 Jun 2014 17:57:14 +0100
+Subject: [PATCH] Handle ETOOMANYREFS when sending recursive fds (SCM_RIGHTS)
+
+Since Linux commit 25888e (from 2.6.37-rc4, Nov 2010), sendmsg() on Unix
+sockets returns -1 errno=ETOOMANYREFS ("Too many references: cannot splice")
+when the passfd mechanism (SCM_RIGHTS) is "abusively" used recursively by
+applications. A malicious client could use this to force a victim system
+service to be disconnected from the system bus; the victim would likely
+respond by exiting. This is a denial of service (fd.o #80163,
+CVE-2014-3532).
+
+This patch silently drops the D-Bus message on ETOOMANYREFS and does not close
+the connection.
+
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=80163
+Reviewed-by: Thiago Macieira <thiago at kde.org>
+[altered commit message to explain DoS significance -smcv]
+Reviewed-by: Simon McVittie <simon.mcvittie at collabora.co.uk>
+---
+ dbus/dbus-sysdeps.c | 14 ++++++++++++++
+ dbus/dbus-sysdeps.h | 1 +
+ dbus/dbus-transport-socket.c | 34 +++++++++++++++++++++++++++++++++-
+ 3 files changed, 48 insertions(+), 1 deletion(-)
+
+diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c
+index 04fb8d7..8ed7da9 100644
+--- a/dbus/dbus-sysdeps.c
++++ b/dbus/dbus-sysdeps.c
+@@ -760,6 +760,20 @@ _dbus_get_is_errno_epipe (void)
+ }
+
+ /**
++ * See if errno is ETOOMANYREFS
++ * @returns #TRUE if errno == ETOOMANYREFS
++ */
++dbus_bool_t
++_dbus_get_is_errno_etoomanyrefs (void)
++{
++#ifdef ETOOMANYREFS
++ return errno == ETOOMANYREFS;
++#else
++ return FALSE;
++#endif
++}
++
++/**
+ * Get error message from errno
+ * @returns _dbus_strerror(errno)
+ */
+diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
+index eee9160..df4c5e0 100644
+--- a/dbus/dbus-sysdeps.h
++++ b/dbus/dbus-sysdeps.h
+@@ -373,6 +373,7 @@ dbus_bool_t _dbus_get_is_errno_eagain_or_ewouldblock (void);
+ dbus_bool_t _dbus_get_is_errno_enomem (void);
+ dbus_bool_t _dbus_get_is_errno_eintr (void);
+ dbus_bool_t _dbus_get_is_errno_epipe (void);
++dbus_bool_t _dbus_get_is_errno_etoomanyrefs (void);
+ const char* _dbus_strerror_from_errno (void);
+
+ void _dbus_disable_sigpipe (void);
+diff --git a/dbus/dbus-transport-socket.c b/dbus/dbus-transport-socket.c
+index 544d00a..26d2b73 100644
+--- a/dbus/dbus-transport-socket.c
++++ b/dbus/dbus-transport-socket.c
+@@ -646,12 +646,44 @@ do_writing (DBusTransport *transport)
+ {
+ /* EINTR already handled for us */
+
+- /* For some discussion of why we also ignore EPIPE here, see
++ /* If the other end closed the socket with close() or shutdown(), we
++ * receive EPIPE here but we must not close the socket yet: there
++ * might still be some data to read. See:
+ * http://lists.freedesktop.org/archives/dbus/2008-March/009526.html
+ */
+
+ if (_dbus_get_is_errno_eagain_or_ewouldblock () || _dbus_get_is_errno_epipe ())
+ goto out;
++
++ /* Since Linux commit 25888e (from 2.6.37-rc4, Nov 2010), sendmsg()
++ * on Unix sockets returns -1 errno=ETOOMANYREFS when the passfd
++ * mechanism (SCM_RIGHTS) is used recursively with a recursion level
++ * of maximum 4. The kernel does not have an API to check whether
++ * the passed fds can be forwarded and it can change asynchronously.
++ * See:
++ * https://bugs.freedesktop.org/show_bug.cgi?id=80163
++ */
++
++ else if (_dbus_get_is_errno_etoomanyrefs ())
++ {
++ /* We only send fds in the first byte of the message.
++ * ETOOMANYREFS cannot happen after.
++ */
++ _dbus_assert (socket_transport->message_bytes_written == 0);
++
++ _dbus_verbose (" discard message of %d bytes due to ETOOMANYREFS\n",
++ total_bytes_to_write);
++
++ socket_transport->message_bytes_written = 0;
++ _dbus_string_set_length (&socket_transport->encoded_outgoing, 0);
++ _dbus_string_compact (&socket_transport->encoded_outgoing, 2048);
++
++ /* The message was not actually sent but it needs to be removed
++ * from the outgoing queue
++ */
++ _dbus_connection_message_sent_unlocked (transport->connection,
++ message);
++ }
+ else
+ {
+ _dbus_verbose ("Error writing to remote app: %s\n",
+--
+1.8.3.1
+
diff --git a/0001-If-loader-contains-two-messages-with-fds-don-t-corru.patch b/0001-If-loader-contains-two-messages-with-fds-don-t-corru.patch
new file mode 100644
index 0000000..e272bce
--- /dev/null
+++ b/0001-If-loader-contains-two-messages-with-fds-don-t-corru.patch
@@ -0,0 +1,40 @@
+From b9c338e32390f953d4c9772daef31187a117b376 Mon Sep 17 00:00:00 2001
+From: Simon McVittie <simon.mcvittie at collabora.co.uk>
+Date: Wed, 11 Jun 2014 12:24:20 +0100
+Subject: [PATCH] If loader contains two messages with fds, don't corrupt the
+ second
+
+There were two bugs here: we would previously overwrite the unused
+fds with the already-used fds instead of the other way round, and
+we would copy n bytes where we should have copied n ints.
+
+Additionally, sending crafted messages in a chosen sequence to a victim
+system service could cause an invalid file descriptor to be present
+when dbus-daemon tries to forward one of those crafted messages to the
+victim, causing sendmsg() to fail with EBADF, which resulted in
+disconnecting the victim service, which would likely respond to that
+by exiting. This is a denial of service (fd.o #80469, CVE-2014-3533).
+
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=79694
+Bug: https://bugs.freedesktop.org/show_bug.cgi?id=80469
+Reviewed-by: Alban Crequy <alban.crequy at collabora.co.uk>
+---
+ dbus/dbus-message.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
+index a34ea1d..fc61ae7 100644
+--- a/dbus/dbus-message.c
++++ b/dbus/dbus-message.c
+@@ -4129,7 +4129,7 @@ load_message (DBusMessageLoader *loader,
+
+ message->n_unix_fds_allocated = message->n_unix_fds = n_unix_fds;
+ loader->n_unix_fds -= n_unix_fds;
+- memmove(loader->unix_fds + n_unix_fds, loader->unix_fds, loader->n_unix_fds);
++ memmove (loader->unix_fds, loader->unix_fds + n_unix_fds, loader->n_unix_fds * sizeof (loader->unix_fds[0]));
+ }
+ else
+ message->unix_fds = NULL;
+--
+1.8.3.1
+
diff --git a/dbus.spec b/dbus.spec
index 8f0785f..4d0914a 100644
--- a/dbus.spec
+++ b/dbus.spec
@@ -17,7 +17,7 @@ Summary: D-BUS message bus
Name: dbus
Epoch: 1
Version: 1.6.12
-Release: 8%{?dist}
+Release: 9%{?dist}
URL: http://www.freedesktop.org/software/dbus/
#VCS: git:git://git.freedesktop.org/git/dbus/dbus
Source0: http://dbus.freedesktop.org/releases/dbus/%{name}-%{version}.tar.gz
@@ -61,6 +61,9 @@ Patch1: 0001-name-test-Don-t-run-test-autolaunch-if-we-don-t-have.patch
Patch2: 0001-test-marshal-Ensure-we-use-suitably-aligned-buffers.patch
Patch3: avoid-undefined-7c00ed22d9b5c33f5b33221e906946b11a9bde3b.patch
Patch4: 0001-_dbus_babysitter_unref-avoid-infinite-loop-if-waitpi.patch
+Patch5: 0001-CVE-2014-3477-deliver-activation-errors-correctly-fi.patch
+Patch6: 0001-If-loader-contains-two-messages-with-fds-don-t-corru.patch
+Patch7: 0001-Handle-ETOOMANYREFS-when-sending-recursive-fds-SCM_R.patch
%description
D-BUS is a system for sending messages between applications. It is
@@ -114,6 +117,9 @@ in this separate package so server systems need not install X.
%patch2 -p1
%patch3 -p1
%patch4 -p1
+%patch5 -p1
+%patch6 -p1
+%patch7 -p1
%build
if test -f autogen.sh; then env NOCONFIGURE=1 ./autogen.sh; else autoreconf -v -f -i; fi
@@ -262,6 +268,13 @@ fi
%{_includedir}/*
%changelog
+* Wed Jul 2 2014 Colin Walters <walters at redhat.com> - 1:1.6.12-9
+- Backport patches from dbus-1.6
+- Fixes CVE-2014-3477 (fd.o#78979)
+- Fixes CVE-2014-3532 (fd.o#80163)
+- Fixes CVE-2014-3533 (fd.o#80469)
+- Resolves #1115636
+
* Thu Dec 26 2013 Dan Horák <dan[at]danny.cz> - 1:1.6.12-8
- valgrind is available only on selected arches
More information about the scm-commits
mailing list