Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [day] [month] [year] [list]
Date: Mon, 10 Nov 2014 16:22:05 +0000
From: Simon McVittie <simon.mcvittie@...labora.co.uk>
To: "dbus@...ts.freedesktop.org" <dbus@...ts.freedesktop.org>, 
 oss-security@...ts.openwall.com
Subject: CVE-2014-7824: D-Bus denial of service via incomplete fix for CVE-2014-3636

CVE: CVE-2014-7824
Tracked as: https://bugs.freedesktop.org/show_bug.cgi?id=85105
Impact: local denial of service
Access required: local
Versions believed to be vulnerable: dbus >= 1.3.0
Fixed in: dbus 1.6.x >= 1.6.26, 1.8.x >= 1.8.10, all versions >= 1.9.2
Credit: discovered by Simon McVittie at Collabora Ltd.

D-Bus <http://www.freedesktop.org/wiki/Software/dbus/> is an
asynchronous inter-process communication system, commonly used
for system services or within a desktop session on Linux and other
operating systems.

The patch issued by the D-Bus maintainers for CVE-2014-3636 was based on
incorrect reasoning, and does not fully prevent the attack described as
"CVE-2014-3636 part A", which is repeated below. Preventing that attack
requires raising the system dbus-daemon's RLIMIT_NOFILE (ulimit -n) to a
higher value. CVE-2014-7824 has been allocated for this vulnerability.

To avoid propagating that higher limit to activatable system services,
it is desirable to start the system dbus-daemon as root so it can store
its previous limit, raise its limit, drop root privileges (which its
default configuration will do automatically), and restore the previous
limit before launching activatable services. Some operating system
distributions, such as anything using the upstream-supplied systemd
units, start the system dbus-daemon as root already; others, such as
Debian 7, currently start the system dbus-daemon under its less
privileged uid and will need minor modifications to their init scripts.

This is fixed in dbus 1.6.26, 1.8.10 and 1.9.2, released today. The
patch used in 1.8.x and 1.9.x is attached; it applies to 1.6.x with
trivial adjustments. Older versions are no longer security-supported by
the D-Bus maintainers, but any distributions needing those versions are
invited to share backported security fixes in the appropriate upstream
branches (dbus-1.4, etc.).

Attack details (repeating CVE-2014-3636 part A):

By queuing up the maximum allowed number of fds, a malicious sender
could reach the system dbus-daemon's RLIMIT_NOFILE (ulimit -n, typically
1024 on Linux). This would act as a denial of service in two ways:

* new clients would be unable to connect to the dbus-daemon
* when receiving a subsequent message from a non-malicious client that
  contained a fd, dbus-daemon would receive the MSG_CTRUNC flag,
  indicating that the list of fds was truncated; kernel fd-passing APIs
  do not provide any way to recover from that, so dbus-daemon responds
  to MSG_CTRUNC by disconnecting the sender, causing denial of service
  to that sender

-- 
Simon McVittie, Collabora Ltd.
for the D-Bus maintainers


>From 4e466446d27f1a3991c22307a47a81c9e93e530d Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie@...labora.co.uk>
Date: Tue, 4 Nov 2014 14:41:54 +0000
Subject: [PATCH] CVE-2014-7824: set fd rlimit to 64k for the system
 dbus-daemon

This ensures that our rlimit is actually high enough to avoid the
denial of service described in CVE-2014-3636 part A.
CVE-2014-7824 has been allocated for this incomplete fix.

Restore the original rlimit for activated services, to avoid
them getting undesired higher limits.

(Thanks to Alban Crequy for various adjustments which have been
included in this commit.)

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=85105
Reviewed-by: Alban Crequy <alban.crequy@...labora.co.uk>
---
 bus/activation.c              |  28 +++++++-
 bus/bus.c                     |  50 ++++++++++++---
 bus/bus.h                     |   1 +
 dbus/dbus-sysdeps-util-unix.c | 145 +++++++++++++++++++++++++++++++++---------
 dbus/dbus-sysdeps-util-win.c  |  35 +++++++++-
 dbus/dbus-sysdeps.h           |  11 +++-
 6 files changed, 227 insertions(+), 43 deletions(-)

diff --git a/bus/activation.c b/bus/activation.c
index 149cca8..ecd19bb 100644
--- a/bus/activation.c
+++ b/bus/activation.c
@@ -1688,6 +1688,31 @@ out:
   return retval;
 }
 
+static void
+child_setup (void *user_data)
+{
+#ifdef DBUS_UNIX
+  BusActivation *activation = user_data;
+  DBusRLimit *initial_fd_limit;
+  DBusError error;
+
+  dbus_error_init (&error);
+  initial_fd_limit = bus_context_get_initial_fd_limit (activation->context);
+
+  if (initial_fd_limit != NULL &&
+      !_dbus_rlimit_restore_fd_limit (initial_fd_limit, &error))
+    {
+      /* unfortunately we don't actually know the service name here */
+      bus_context_log (activation->context,
+                       DBUS_SYSTEM_LOG_INFO,
+                       "Failed to reset fd limit before activating "
+                       "service: %s: %s",
+                       error.name, error.message);
+    }
+#endif
+}
+
+
 dbus_bool_t
 bus_activation_activate_service (BusActivation  *activation,
                                  DBusConnection *connection,
@@ -2121,7 +2146,8 @@ bus_activation_activate_service (BusActivation  *activation,
                                           service_name,
                                           argv,
                                           envp,
-                                          NULL, activation,
+                                          child_setup,
+                                          activation,
                                           &tmp_error))
     {
       _dbus_verbose ("Failed to spawn child\n");
diff --git a/bus/bus.c b/bus/bus.c
index 35d4075..47cc345 100644
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -64,6 +64,7 @@ struct BusContext
   BusPolicy *policy;
   BusMatchmaker *matchmaker;
   BusLimits limits;
+  DBusRLimit *initial_fd_limit;
   unsigned int fork : 1;
   unsigned int syslog : 1;
   unsigned int keep_umask : 1;
@@ -659,19 +660,38 @@ oom:
 static void
 raise_file_descriptor_limit (BusContext      *context)
 {
+#ifdef DBUS_UNIX
+  DBusError error = DBUS_ERROR_INIT;
 
-  /* I just picked this out of thin air; we need some extra
-   * descriptors for things like any internal pipes we create,
-   * inotify, connections to SELinux, etc.
-   */
-  unsigned int arbitrary_extra_fds = 32;
-  unsigned int limit;
+  /* we only do this once */
+  if (context->initial_fd_limit != NULL)
+    return;
 
-  limit = context->limits.max_completed_connections +
-    context->limits.max_incomplete_connections
-    + arbitrary_extra_fds;
+  context->initial_fd_limit = _dbus_rlimit_save_fd_limit (&error);
+
+  if (context->initial_fd_limit == NULL)
+    {
+      bus_context_log (context, DBUS_SYSTEM_LOG_INFO,
+                       "%s: %s", error.name, error.message);
+      dbus_error_free (&error);
+      return;
+    }
 
-  _dbus_request_file_descriptor_limit (limit);
+  /* We used to compute a suitable rlimit based on the configured number
+   * of connections, but that breaks down as soon as we allow fd-passing,
+   * because each connection is allowed to pass 64 fds to us, and if
+   * they all did, we'd hit kernel limits. We now hard-code 64k as a
+   * good limit, like systemd does: that's enough to avoid DoS from
+   * anything short of multiple uids conspiring against us.
+   */
+  if (!_dbus_rlimit_raise_fd_limit_if_privileged (65536, &error))
+    {
+      bus_context_log (context, DBUS_SYSTEM_LOG_INFO,
+                       "%s: %s", error.name, error.message);
+      dbus_error_free (&error);
+      return;
+    }
+#endif
 }
 
 static dbus_bool_t
@@ -1130,6 +1150,10 @@ bus_context_unref (BusContext *context)
 
           dbus_free (context->pidfile);
 	}
+
+      if (context->initial_fd_limit)
+        _dbus_rlimit_free (context->initial_fd_limit);
+
       dbus_free (context);
 
       dbus_server_free_data_slot (&server_data_slot);
@@ -1294,6 +1318,12 @@ bus_context_get_reply_timeout (BusContext *context)
   return context->limits.reply_timeout;
 }
 
+DBusRLimit *
+bus_context_get_initial_fd_limit (BusContext *context)
+{
+  return context->initial_fd_limit;
+}
+
 void
 bus_context_log (BusContext *context, DBusSystemLogSeverity severity, const char *msg, ...) _DBUS_GNUC_PRINTF (3, 4);
 
diff --git a/bus/bus.h b/bus/bus.h
index 7d0b369..dac6ea5 100644
--- a/bus/bus.h
+++ b/bus/bus.h
@@ -116,6 +116,7 @@ int               bus_context_get_max_services_per_connection    (BusContext
 int               bus_context_get_max_match_rules_per_connection (BusContext       *context);
 int               bus_context_get_max_replies_per_connection     (BusContext       *context);
 int               bus_context_get_reply_timeout                  (BusContext       *context);
+DBusRLimit *      bus_context_get_initial_fd_limit               (BusContext       *context);
 void              bus_context_log                                (BusContext       *context,
                                                                   DBusSystemLogSeverity severity,
                                                                   const char       *msg,
diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c
index 0d8a66c..199eb3d 100644
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -378,53 +378,140 @@ _dbus_change_to_daemon_user  (const char    *user,
 }
 #endif /* !HAVE_LIBAUDIT */
 
+#ifdef HAVE_SETRLIMIT
 
-/**
- * Attempt to ensure that the current process can open
- * at least @p limit file descriptors.
- *
- * If @p limit is lower than the current, it will not be
- * lowered.  No error is returned if the request can
- * not be satisfied.
- *
- * @param limit number of file descriptors
+/* We assume that if we have setrlimit, we also have getrlimit and
+ * struct rlimit.
  */
-void
-_dbus_request_file_descriptor_limit (unsigned int limit)
+
+struct DBusRLimit {
+    struct rlimit lim;
+};
+
+DBusRLimit *
+_dbus_rlimit_save_fd_limit (DBusError *error)
+{
+  DBusRLimit *self;
+
+  self = dbus_new0 (DBusRLimit, 1);
+
+  if (self == NULL)
+    {
+      _DBUS_SET_OOM (error);
+      return NULL;
+    }
+
+  if (getrlimit (RLIMIT_NOFILE, &self->lim) < 0)
+    {
+      dbus_set_error (error, _dbus_error_from_errno (errno),
+                      "Failed to get fd limit: %s", _dbus_strerror (errno));
+      dbus_free (self);
+      return NULL;
+    }
+
+  return self;
+}
+
+dbus_bool_t
+_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int  desired,
+                                           DBusError    *error)
 {
-#ifdef HAVE_SETRLIMIT
   struct rlimit lim;
-  struct rlimit target_lim;
 
   /* No point to doing this practically speaking
    * if we're not uid 0.  We expect the system
    * bus to use this before we change UID, and
-   * the session bus takes the Linux default
-   * of 1024 for both cur and max.
+   * the session bus takes the Linux default,
+   * currently 1024 for cur and 4096 for max.
    */
   if (getuid () != 0)
-    return;
+    {
+      /* not an error, we're probably the session bus */
+      return TRUE;
+    }
 
   if (getrlimit (RLIMIT_NOFILE, &lim) < 0)
-    return;
+    {
+      dbus_set_error (error, _dbus_error_from_errno (errno),
+                      "Failed to get fd limit: %s", _dbus_strerror (errno));
+      return FALSE;
+    }
 
-  if (lim.rlim_cur >= limit)
-    return;
+  if (lim.rlim_cur == RLIM_INFINITY || lim.rlim_cur >= desired)
+    {
+      /* not an error, everything is fine */
+      return TRUE;
+    }
 
   /* Ignore "maximum limit", assume we have the "superuser"
    * privileges.  On Linux this is CAP_SYS_RESOURCE.
    */
-  target_lim.rlim_cur = target_lim.rlim_max = limit;
-  /* Also ignore errors; if we fail, we will at least work
-   * up to whatever limit we had, which seems better than
-   * just outright aborting.
-   *
-   * However, in the future we should probably log this so OS builders
-   * have a chance to notice any misconfiguration like dbus-daemon
-   * being started without CAP_SYS_RESOURCE.
-   */
-  setrlimit (RLIMIT_NOFILE, &target_lim);
+  lim.rlim_cur = lim.rlim_max = desired;
+
+  if (setrlimit (RLIMIT_NOFILE, &lim) < 0)
+    {
+      dbus_set_error (error, _dbus_error_from_errno (errno),
+                      "Failed to set fd limit to %u: %s",
+                      desired, _dbus_strerror (errno));
+      return FALSE;
+    }
+
+  return TRUE;
+}
+
+dbus_bool_t
+_dbus_rlimit_restore_fd_limit (DBusRLimit *saved,
+                               DBusError  *error)
+{
+  if (setrlimit (RLIMIT_NOFILE, &saved->lim) < 0)
+    {
+      dbus_set_error (error, _dbus_error_from_errno (errno),
+                      "Failed to restore old fd limit: %s",
+                      _dbus_strerror (errno));
+      return FALSE;
+    }
+
+  return TRUE;
+}
+
+#else /* !HAVE_SETRLIMIT */
+
+static void
+fd_limit_not_supported (DBusError *error)
+{
+  dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED,
+                  "cannot change fd limit on this platform");
+}
+
+DBusRLimit *
+_dbus_rlimit_save_fd_limit (DBusError *error)
+{
+  fd_limit_not_supported (error);
+  return NULL;
+}
+
+dbus_bool_t
+_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int  desired,
+                                           DBusError    *error)
+{
+  fd_limit_not_supported (error);
+  return FALSE;
+}
+
+dbus_bool_t
+_dbus_rlimit_restore_fd_limit (DBusRLimit *saved,
+                               DBusError  *error)
+{
+  fd_limit_not_supported (error);
+  return FALSE;
+}
+
 #endif
+
+void
+_dbus_rlimit_free (DBusRLimit *lim)
+{
+  dbus_free (lim);
 }
 
 void
diff --git a/dbus/dbus-sysdeps-util-win.c b/dbus/dbus-sysdeps-util-win.c
index 4678b11..ec9afbb 100644
--- a/dbus/dbus-sysdeps-util-win.c
+++ b/dbus/dbus-sysdeps-util-win.c
@@ -258,9 +258,42 @@ _dbus_change_to_daemon_user  (const char    *user,
   return TRUE;
 }
 
+static void
+fd_limit_not_supported (DBusError *error)
+{
+  dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED,
+                  "cannot change fd limit on this platform");
+}
+
+DBusRLimit *
+_dbus_rlimit_save_fd_limit (DBusError *error)
+{
+  fd_limit_not_supported (error);
+  return NULL;
+}
+
+dbus_bool_t
+_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int  desired,
+                                           DBusError    *error)
+{
+  fd_limit_not_supported (error);
+  return FALSE;
+}
+
+dbus_bool_t
+_dbus_rlimit_restore_fd_limit (DBusRLimit *saved,
+                               DBusError  *error)
+{
+  fd_limit_not_supported (error);
+  return FALSE;
+}
+
 void
-_dbus_request_file_descriptor_limit (unsigned int limit)
+_dbus_rlimit_free (DBusRLimit *lim)
 {
+  /* _dbus_rlimit_save_fd_limit() cannot return non-NULL on Windows
+   * so there cannot be anything to free */
+  _dbus_assert (lim == NULL);
 }
 
 void
diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index 47ba2f4..5465128 100644
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -546,8 +546,6 @@ dbus_bool_t _dbus_change_to_daemon_user (const char *user,
 
 void _dbus_flush_caches (void);
 
-void _dbus_request_file_descriptor_limit (unsigned int limit);
-
 /*
  * replaces the term DBUS_PREFIX in configure_time_path by the
  * current dbus installation directory. On unix this function is a noop
@@ -566,6 +564,15 @@ _dbus_replace_install_prefix (const char *configure_time_path);
  */
 #define DBUS_DEFAULT_MESSAGE_UNIX_FDS 16
 
+typedef struct DBusRLimit DBusRLimit;
+
+DBusRLimit     *_dbus_rlimit_save_fd_limit                 (DBusError    *error);
+dbus_bool_t     _dbus_rlimit_raise_fd_limit_if_privileged  (unsigned int  desired,
+                                                            DBusError    *error);
+dbus_bool_t     _dbus_rlimit_restore_fd_limit              (DBusRLimit   *saved,
+                                                            DBusError    *error);
+void            _dbus_rlimit_free                          (DBusRLimit   *lim);
+
 /** @} */
 
 DBUS_END_DECLS
-- 
2.1.3


Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ