[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#776480: lintian: [dbus] add the check that found CVE-2014-8148 and CVE-2014-8156

On Wed, 28 Jan 2015 at 14:14:56 +0000, Simon McVittie wrote:
> Patches also available from:
> ssh://git.debian.org/git/users/smcv/lintian.git dbus

Oops, I pointed to a git repository but forgot to attach them.
Here they are.

    S

>From 090485494408f4c72b8768d95160e8950c3cc7e3 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Mon, 5 Jan 2015 16:21:27 +0000
Subject: [PATCH 1/5] Transcode checks/dbus.pm to UTF-8

The UTF-8 turned into Latin-1 on its way through the BTS, or something.
---
 checks/dbus.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/checks/dbus.pm b/checks/dbus.pm
index 364c980..23ea0cd 100644
--- a/checks/dbus.pm
+++ b/checks/dbus.pm
@@ -1,7 +1,7 @@
 # dbus -- lintian check script, vaguely based on apache2 -*- perl -*-
 #
-# Copyright � 2012 Arno T�-# Copyright � 2014 Collabora Ltd.
+# Copyright © 2012 Arno Töll
+# Copyright © 2014 Collabora Ltd.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
-- 
2.1.4


>From b5bb0b11edb3ab2c7fc58c6e73b64cecb4d7c8aa Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Mon, 5 Jan 2015 16:49:51 +0000
Subject: [PATCH 2/5] dbus: capture <policy> for each <allow> or <deny> rule

Seeing a rule that says <allow send_interface="x.y.z"/>
doesn't tell you whether it is
<policy user="root"><allow send_interface="x.y.z"/> (usually an
anti-pattern, but sometimes necessary for the "agent" pattern as seen
in BlueZ) or <policy context="default"><allow send_interface="x.y.z"/>
(which should ring alarm bells).

To solve this, capture the enclosing <policy> for each <allow> or
<deny> rule.

This also means our output for the at_console check can indicate
precisely which rules apply to console users.
---
 checks/dbus.pm                                     | 31 +++++++++++++++-------
 .../debian/etc/dbus-1/system.d/at-console.conf     |  2 ++
 t/tests/dbus-policy/tags                           |  9 ++++---
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/checks/dbus.pm b/checks/dbus.pm
index 23ea0cd..2aecae8 100644
--- a/checks/dbus.pm
+++ b/checks/dbus.pm
@@ -70,20 +70,33 @@ sub _check_policy {
     # note that we are parsing the entire file as one big string,
     # so that we catch <policy\nat_console="true"\n> or whatever.
 
-    if ($xml =~ m{<policy[^>]+at_console=(["'])true\1.*?</policy>}s) {
-        tag('dbus-policy-at-console', $file);
-    }
-
     my @rules;
-    while ($xml =~ m{(<(?:allow|deny)[^>]+send_\w+=[^>]+>)}sg) {
-        push(@rules, $1);
+    # a small rubbish state machine: we want to match a <policy> containing
+    # any <allow> or <deny> rule that is about sending
+    my $policy = '';
+    while ($xml =~ m{(<policy[^>]*>)|(</policy\s*>)|(<(?:allow|deny)[^>]*>)}sg)
+    {
+        if (defined $1) {
+            $policy = $1;
+        } elsif (defined $2) {
+            $policy = '';
+        } else {
+            push(@rules, $policy.$3);
+        }
     }
     foreach my $rule (@rules) {
-        if ($rule !~ m{send_destination=}) {
-            # normalize whitespace a bit
-            $rule =~ s{\s+}{ }g;
+        # normalize whitespace a bit so we can report it sensibly:
+        # typically it will now look like
+        # <policy context="default"><allow send_destination="com.example.Foo"/>
+        $rule =~ s{\s+}{ }g;
+
+        if ($rule =~ m{send_} && $rule !~ m{send_destination=}) {
             tag('dbus-policy-without-send-destination', $file, $rule);
         }
+
+        if ($rule =~ m{at_console=['"]true}) {
+            tag('dbus-policy-at-console', $file, $rule);
+        }
     }
 
     return;
diff --git a/t/tests/dbus-policy/debian/etc/dbus-1/system.d/at-console.conf b/t/tests/dbus-policy/debian/etc/dbus-1/system.d/at-console.conf
index 06d96c8..8c47adb 100644
--- a/t/tests/dbus-policy/debian/etc/dbus-1/system.d/at-console.conf
+++ b/t/tests/dbus-policy/debian/etc/dbus-1/system.d/at-console.conf
@@ -4,10 +4,12 @@
   <!-- this is OK, at least for now -->
   <policy group="bluetooth">
     <allow send_destination="com.example.Service"/>
+    <allow send_destination="com.example.Other"/>
   </policy>
 
   <!-- this is deprecated -->
   <policy at_console="true">
     <allow send_destination="com.example.Service"/>
+    <allow send_destination="com.example.Other"/>
   </policy>
 </busconfig>
diff --git a/t/tests/dbus-policy/tags b/t/tests/dbus-policy/tags
index 0705661..f9028e6 100644
--- a/t/tests/dbus-policy/tags
+++ b/t/tests/dbus-policy/tags
@@ -1,4 +1,5 @@
-X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf
-X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <allow send_interface="org.freedesktop.DBus.ObjectManager"/>
-X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
-X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <allow send_path="/com/example/Here"/>
+X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <policy at_console="true"><allow send_destination="com.example.Other"/>
+X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <policy at_console="true"><allow send_destination="com.example.Service"/>
+X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_interface="org.freedesktop.DBus.ObjectManager"/>
+X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
+X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_path="/com/example/Here"/>
-- 
2.1.4


>From dccd58d69ca39f97e6a256869c1aca3184f2dfe6 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Wed, 28 Jan 2015 13:36:28 +0000
Subject: [PATCH 3/5] dbus: do not emit dbus-policy-without-send-destination
 for rules limited to root

root is all-powerful anyway, so this is probably OK - and as a
practical matter, this cuts down a lot of hits for this Lintian test
which are not actually security vulnerabilities in practice.
---
 checks/dbus.desc                                      | 19 ++++++++++++++++---
 checks/dbus.pm                                        | 11 ++++++++++-
 .../debian/etc/dbus-1/system.d/send-destination.conf  |  6 ++++++
 t/tests/dbus-policy/tags                              |  1 +
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/checks/dbus.desc b/checks/dbus.desc
index 42b9665..19befb5 100644
--- a/checks/dbus.desc
+++ b/checks/dbus.desc
@@ -33,10 +33,10 @@ Experimental: yes
 
 Tag: dbus-policy-without-send-destination
 Severity: normal
-Certainty: possible
+Certainty: certain
 Info: The package contains D-Bus policy configuration that uses
- one of the <tt>send_*</tt> conditions but does not specify a
- <tt>send_destination</tt>.
+ one of the <tt>send_*</tt> conditions, but does not specify a
+ <tt>send_destination</tt>, and is not specific to root.
  .
  Rules of the form
  .
@@ -52,6 +52,19 @@ Info: The package contains D-Bus policy configuration that uses
  .
  are redundant with the system bus' default-deny policy, and have
  unintended effects on other services.
+ .
+ This check ignores rules of the form
+ .
+   &lt;policy user="root"&gt;
+     &lt;allow ... /&gt;
+   &lt;/policy&gt;
+ .
+ which are commonly used for the "agent" pattern seen in services like
+ BlueZ and NetworkManager: a root-privileged daemon calls out to
+ one or more per-user user interface agent processes with no specific
+ name, so <tt>send_destination</tt> is not easily applicable.
+ However, such rules should still be made as specific as possible to
+ avoid undesired side-effects.
 Ref: https://bugs.freedesktop.org/show_bug.cgi?id=18961,http://lists.freedesktop.org/archives/dbus/2008-February/009401.html
 Experimental: yes
 
diff --git a/checks/dbus.pm b/checks/dbus.pm
index 2aecae8..18c976b 100644
--- a/checks/dbus.pm
+++ b/checks/dbus.pm
@@ -91,7 +91,16 @@ sub _check_policy {
         $rule =~ s{\s+}{ }g;
 
         if ($rule =~ m{send_} && $rule !~ m{send_destination=}) {
-            tag('dbus-policy-without-send-destination', $file, $rule);
+            # It is about sending but does not specify a send-destination.
+            # This could be bad.
+
+            if ($rule =~ m{[^>]*user=['"]root['"].*<allow}) {
+                # skip it: it's probably the "agent" pattern (as seen in
+                # e.g. BlueZ), and cannot normally be a security flaw
+                # because root can do anything anyway
+            } else {
+                tag('dbus-policy-without-send-destination', $file, $rule);
+            }
         }
 
         if ($rule =~ m{at_console=['"]true}) {
diff --git a/t/tests/dbus-policy/debian/etc/dbus-1/system.d/send-destination.conf b/t/tests/dbus-policy/debian/etc/dbus-1/system.d/send-destination.conf
index 6bb7150..ae052ff 100644
--- a/t/tests/dbus-policy/debian/etc/dbus-1/system.d/send-destination.conf
+++ b/t/tests/dbus-policy/debian/etc/dbus-1/system.d/send-destination.conf
@@ -1,5 +1,11 @@
 <!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd";>
 <busconfig>
+  <policy user="root">
+    <allow send_interface="org.freedesktop.DBus.ObjectManager"/>
+  </policy>
+  <policy user="nobody">
+    <allow send_interface="org.freedesktop.DBus.ObjectManager"/>
+  </policy>
   <policy context="default">
     <allow send_interface="org.freedesktop.DBus.ObjectManager"/>
     <allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
diff --git a/t/tests/dbus-policy/tags b/t/tests/dbus-policy/tags
index f9028e6..2d1d32e 100644
--- a/t/tests/dbus-policy/tags
+++ b/t/tests/dbus-policy/tags
@@ -3,3 +3,4 @@ X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <poli
 X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_interface="org.freedesktop.DBus.ObjectManager"/>
 X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
 X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_path="/com/example/Here"/>
+X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy user="nobody"><allow send_interface="org.freedesktop.DBus.ObjectManager"/>
-- 
2.1.4


>From 8241b2fd3d8383b5e609374dbf56b646e9754a21 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Wed, 28 Jan 2015 13:44:18 +0000
Subject: [PATCH 4/5] dbus: add a new tag for excessively broad match rules

This check has already found CVE-2014-8148 and the recently
unembargoed CVE-2014-8156, which I believe are the only ones
in current Debian. Hopefully having this as a Lintian check
will protect us from this class of vulnerability.
---
 checks/dbus.desc         | 31 +++++++++++++++++++++++++++++++
 checks/dbus.pm           | 18 ++++++++++++++++++
 t/tests/dbus-policy/tags |  2 ++
 3 files changed, 51 insertions(+)

diff --git a/checks/dbus.desc b/checks/dbus.desc
index 19befb5..c08e3ee 100644
--- a/checks/dbus.desc
+++ b/checks/dbus.desc
@@ -68,6 +68,37 @@ Info: The package contains D-Bus policy configuration that uses
 Ref: https://bugs.freedesktop.org/show_bug.cgi?id=18961,http://lists.freedesktop.org/archives/dbus/2008-February/009401.html
 Experimental: yes
 
+Tag: dbus-policy-excessively-broad
+Severity: serious
+Certainty: possible
+Info: The package contains D-Bus policy configuration that
+ matches broad classes of messages. This will cause strange side-effects,
+ is almost certainly unintended, and is a probable security flaw.
+ .
+ For instance,
+ .
+   &lt;policy user="daemon"&gt;
+     &lt;allow send_type="method_call"/&gt;
+     &lt;allow send_destination="com.example.Bees"/&gt;
+   &lt;/policy&gt;
+ .
+ in any system bus policy file would allow the <tt>daemon</tt> user to send
+ any method call to any service, including method calls which are meant to
+ be restricted to root-only for security, such as
+ <tt>org.freedesktop.systemd1.Manager.StartTransientUnit</tt>. (In addition,
+ it allows that user to send any message to the <tt>com.example.Bees</tt>
+ service.)
+ .
+ The intended policy for that particular example was probably more like
+ .
+   &lt;policy user="daemon"&gt;
+     &lt;allow send_type="method_call" send_destination="com.example.Bees"/&gt;
+   &lt;/policy&gt;
+ .
+ which correctly allows method calls to that particular service only.
+Ref: http://www.openwall.com/lists/oss-security/2015/01/27/25
+Experimental: yes
+
 Tag: dbus-session-service-wrong-name
 Severity: wishlist
 Certainty: certain
diff --git a/checks/dbus.pm b/checks/dbus.pm
index 18c976b..e56d753 100644
--- a/checks/dbus.pm
+++ b/checks/dbus.pm
@@ -61,6 +61,8 @@ sub run {
     return;
 }
 
+my $PROPERTIES = 'org.freedesktop.DBus.Properties';
+
 sub _check_policy {
     my ($file) = @_;
 
@@ -100,6 +102,22 @@ sub _check_policy {
                 # because root can do anything anyway
             } else {
                 tag('dbus-policy-without-send-destination', $file, $rule);
+
+                if (   $rule =~ m{send_interface=}
+                    && $rule !~ m{send_interface=['"]\Q${PROPERTIES}\E['"]}) {
+                    # That's undesirable, because it opens up communication
+                    # with arbitrary services and can undo DoS mitigation
+                    # efforts; but at least it's specific to an interface
+                    # other than o.fd.DBus.Properties, so all that should
+                    # happen is that the service sends back an error message.
+                    #
+                    # Properties doesn't count as an effective limitation,
+                    # because it's a sort of meta-interface.
+                } elsif ($rule =~ m{<allow}) {
+                    # Looks like CVE-2014-8148 or similar. This is really bad;
+                    # emit an additional tag.
+                    tag('dbus-policy-excessively-broad', $file, $rule);
+                }
             }
         }
 
diff --git a/t/tests/dbus-policy/tags b/t/tests/dbus-policy/tags
index 2d1d32e..8b40f8a 100644
--- a/t/tests/dbus-policy/tags
+++ b/t/tests/dbus-policy/tags
@@ -1,5 +1,7 @@
 X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <policy at_console="true"><allow send_destination="com.example.Other"/>
 X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <policy at_console="true"><allow send_destination="com.example.Service"/>
+X: dbus-policy: dbus-policy-excessively-broad etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
+X: dbus-policy: dbus-policy-excessively-broad etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_path="/com/example/Here"/>
 X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_interface="org.freedesktop.DBus.ObjectManager"/>
 X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
 X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_path="/com/example/Here"/>
-- 
2.1.4


>From 8104d50ee6dfcae73062ba89140882462aeff901 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Wed, 28 Jan 2015 13:53:43 +0000
Subject: [PATCH 5/5] Mark D-Bus checks as non-experimental

They've found two unrelated CVEs, I think the experiment was a success :-)
---
 checks/dbus.desc           |  5 -----
 t/tests/dbus-policy/tags   | 16 ++++++++--------
 t/tests/dbus-services/tags |  4 ++--
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/checks/dbus.desc b/checks/dbus.desc
index c08e3ee..2066f3d 100644
--- a/checks/dbus.desc
+++ b/checks/dbus.desc
@@ -29,7 +29,6 @@ Info: The package contains D-Bus policy configuration that uses the
  including distinguishing between users who are "at the console" and
  those who are not.
 Ref: https://bugs.freedesktop.org/show_bug.cgi?id=39611
-Experimental: yes
 
 Tag: dbus-policy-without-send-destination
 Severity: normal
@@ -66,7 +65,6 @@ Info: The package contains D-Bus policy configuration that uses
  However, such rules should still be made as specific as possible to
  avoid undesired side-effects.
 Ref: https://bugs.freedesktop.org/show_bug.cgi?id=18961,http://lists.freedesktop.org/archives/dbus/2008-February/009401.html
-Experimental: yes
 
 Tag: dbus-policy-excessively-broad
 Severity: serious
@@ -97,7 +95,6 @@ Info: The package contains D-Bus policy configuration that
  .
  which correctly allows method calls to that particular service only.
 Ref: http://www.openwall.com/lists/oss-security/2015/01/27/25
-Experimental: yes
 
 Tag: dbus-session-service-wrong-name
 Severity: wishlist
@@ -113,7 +110,6 @@ Info: The package contains a D-Bus session service whose filename
  name is <tt>com.example.MyService1</tt>, and it should be
  service-activatable, you should achieve that by packaging
  <tt>/usr/share/dbus-1/services/com.example.MyService1.service</tt>.
-Experimental: yes
 
 Tag: dbus-system-service-wrong-name
 Severity: serious
@@ -128,4 +124,3 @@ Info: The package contains a D-Bus system service whose filename
  <tt>com.example.MyService1</tt>, and it should be service-activatable,
  you must provide
  <tt>/usr/share/dbus-1/system-services/com.example.MyService1.service</tt>.
-Experimental: yes
diff --git a/t/tests/dbus-policy/tags b/t/tests/dbus-policy/tags
index 8b40f8a..1f65e49 100644
--- a/t/tests/dbus-policy/tags
+++ b/t/tests/dbus-policy/tags
@@ -1,8 +1,8 @@
-X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <policy at_console="true"><allow send_destination="com.example.Other"/>
-X: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <policy at_console="true"><allow send_destination="com.example.Service"/>
-X: dbus-policy: dbus-policy-excessively-broad etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
-X: dbus-policy: dbus-policy-excessively-broad etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_path="/com/example/Here"/>
-X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_interface="org.freedesktop.DBus.ObjectManager"/>
-X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
-X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_path="/com/example/Here"/>
-X: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy user="nobody"><allow send_interface="org.freedesktop.DBus.ObjectManager"/>
+E: dbus-policy: dbus-policy-excessively-broad etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
+E: dbus-policy: dbus-policy-excessively-broad etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_path="/com/example/Here"/>
+W: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <policy at_console="true"><allow send_destination="com.example.Other"/>
+W: dbus-policy: dbus-policy-at-console etc/dbus-1/system.d/at-console.conf <policy at_console="true"><allow send_destination="com.example.Service"/>
+W: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_interface="org.freedesktop.DBus.ObjectManager"/>
+W: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_member="AreYouReallySureThisMethodIsAlwaysOK"/>
+W: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy context="default"><allow send_path="/com/example/Here"/>
+W: dbus-policy: dbus-policy-without-send-destination etc/dbus-1/system.d/send-destination.conf <policy user="nobody"><allow send_interface="org.freedesktop.DBus.ObjectManager"/>
diff --git a/t/tests/dbus-services/tags b/t/tests/dbus-services/tags
index 217b153..9802a46 100644
--- a/t/tests/dbus-services/tags
+++ b/t/tests/dbus-services/tags
@@ -1,2 +1,2 @@
-X: dbus-services: dbus-session-service-wrong-name org.gtk.vfs.Daemon.service usr/share/dbus-1/services/gvfs-daemon.service
-X: dbus-services: dbus-system-service-wrong-name com.example.SystemDaemon1.service usr/share/dbus-1/system-services/this-name-cannot-work.service
+E: dbus-services: dbus-system-service-wrong-name com.example.SystemDaemon1.service usr/share/dbus-1/system-services/this-name-cannot-work.service
+I: dbus-services: dbus-session-service-wrong-name org.gtk.vfs.Daemon.service usr/share/dbus-1/services/gvfs-daemon.service
-- 
2.1.4
Reply to: