Blob Blame History Raw
From 8c7176019fbc2e8fee41d93ce82ac2603fe57d67 Mon Sep 17 00:00:00 2001
From: Alban Crequy <alban.crequy@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@kde.org>
[altered commit message to explain DoS significance -smcv]
Reviewed-by: Simon McVittie <simon.mcvittie@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