Bug 98157 - format string vulnerability processing ActivationFailure messages
Summary: format string vulnerability processing ActivationFailure messages
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: All Linux (All)
: high major
Assignee: Simon McVittie
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords: patch, security
Depends on:
Blocks:
 
Reported: 2016-10-07 20:05 UTC by Simon McVittie
Modified: 2016-10-10 14:26 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
dbus_activation_systemd_failure: do not use non-literal format string (1.92 KB, patch)
2016-10-07 20:08 UTC, Simon McVittie
Details | Splinter Review
bus_driver_handle_message: reject ActivationFailure if unprivileged (1.43 KB, patch)
2016-10-07 20:29 UTC, Simon McVittie
Details | Splinter Review
Ignore ActivationFailure if not using systemd activation (1.19 KB, patch)
2016-10-07 20:30 UTC, Simon McVittie
Details | Splinter Review
Disable deprecation warnings for stable branch (796 bytes, patch)
2016-10-07 20:40 UTC, Simon McVittie
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Simon McVittie 2016-10-07 20:05:04 UTC
While trying to fix all the compiler warnings (Bug #97357) I tried enabling -Wsuggest-attribute=format, and found this:

---

dbus-daemon listens for ActivationFailure signals coming from systemd. When running with --systemd-activation, these signals are used to report that systemd could not activate a D-Bus service for us; when not using --systemd-activation, they are still processed.

dbus-daemon passes the human-readable message contained in these signals to dbus_set_error() where a format string is expected. In principle this could lead to arbitrary memory overwrite via a format string attack in the message received from systemd, resulting in arbitrary code execution.
    
I believe this is not an exploitable security vulnerability on the system bus in practice: it can only be exploited by the owner of the org.freedesktop.systemd1 bus name, which is restricted to uid 0, so if systemd is attacker-controlled then the system is already doomed. Similarly, if a systemd system unit mentioned in the activation failure message has an attacker-controlled name, then the attacker likely already has sufficient access to execute arbitrary code as root in any case.
    
However, prior to dbus 1.8.16 and 1.9.10, due to a missing check for systemd's identity, unprivileged processes could forge activation failure messages which would have gone through this code path. We thought at the time that this was a denial of service vulnerability (CVE-2015-0245); this bug means that it was in fact potentially an arbitrary code execution vulnerability.
    
Bug found using -Wsuggest-attribute=format and -Wformat-security.
Comment 1 Simon McVittie 2016-10-07 20:08:58 UTC
Created attachment 127124 [details] [review]
dbus_activation_systemd_failure: do not use non-literal format string

In principle this could lead to arbitrary memory overwrite via
a format string attack in the message received from systemd,
resulting in arbitrary code execution.
    
This is not believed to be a practical security vulnerability on the
system bus in practice: it can only be exploited by the owner of the
org.freedesktop.systemd1 bus name, which is restricted to uid 0, so
if systemd is attacker-controlled then the system is already doomed.
Similarly, if a systemd system unit mentioned in the activation failure
message has an attacker-controlled name, then the attacker likely already
has sufficient access to execute arbitrary code as root in any case.
    
However, prior to dbus 1.8.16 and 1.9.10, due to a missing check for
systemd's identity, unprivileged processes could forge activation
failure messages which would have gone through this code path.
We thought at the time that this was a denial of service vulnerability
(CVE-2015-0245); this bug means that it was in fact potentially an
arbitrary code execution vulnerability.
    
Bug found using -Wsuggest-attribute=format and -Wformat-security.

---

Please comment if you do not agree with my security analysis.

I intend to do 1.8.x, 1.10.x and 1.11.x releases for this on Monday unless told otherwise. I'll do a follow-up public bug report for all the non-security-sensitive format string issues after that.
Comment 2 Simon McVittie 2016-10-07 20:29:47 UTC
Created attachment 127125 [details] [review]
bus_driver_handle_message: reject ActivationFailure if  unprivileged

Specifically, this will allow ActivationFailure messages from our
own uid or from root, but reject them otherwise, even if the bus
configuration for who can own org.freedesktop.systemd1 is entirely
wrong due to something like CVE-2014-8148.

---

This is just hardening, but I'd like to include it in all supported branches.
Comment 3 Simon McVittie 2016-10-07 20:30:17 UTC
Created attachment 127127 [details] [review]
Ignore ActivationFailure if not using systemd activation

This isn't security-related, just defensive programming: if
dbus-daemon wasn't run with --systemd-activation, then there is no
reason why systemd would legitimately send us this signal, and if it
does we should just ignore it.
Comment 4 Simon McVittie 2016-10-07 20:40:22 UTC
Created attachment 127128 [details] [review]
Disable deprecation warnings for stable branch

We're not going to replace deprecated functions here.

---

Or I can apply Attachment #126941 [details] (from Bug #97357) if preferred.

Reviews of my other patches on Bug #97357 are of course very welcome.
Comment 5 Simon McVittie 2016-10-07 20:41:50 UTC
(In reply to Simon McVittie from comment #4)
> Disable deprecation warnings for stable branch
> 
> We're not going to replace deprecated functions here.

(Further explanation: none of the branches actually compile on my system at the moment, because glibc 2.24 deprecated readdir_r.)
Comment 6 Colin Walters 2016-10-08 19:52:18 UTC
Comment on attachment 127124 [details] [review]
dbus_activation_systemd_failure: do not use non-literal format string

Review of attachment 127124 [details] [review]:
-----------------------------------------------------------------

Looks good to me.
Comment 7 Colin Walters 2016-10-08 19:53:49 UTC
Comment on attachment 127125 [details] [review]
bus_driver_handle_message: reject ActivationFailure if  unprivileged

Review of attachment 127125 [details] [review]:
-----------------------------------------------------------------

LGTM.
Comment 8 Colin Walters 2016-10-08 19:54:06 UTC
Comment on attachment 127127 [details] [review]
Ignore ActivationFailure if not using systemd activation

Review of attachment 127127 [details] [review]:
-----------------------------------------------------------------

OK.
Comment 9 Colin Walters 2016-10-08 19:55:09 UTC
Comment on attachment 127128 [details] [review]
Disable deprecation warnings for stable branch

Review of attachment 127128 [details] [review]:
-----------------------------------------------------------------

OK.
Comment 10 Simon McVittie 2016-10-09 22:43:00 UTC
Ralf (and other reviewers):

If you're looking at Bug #97357, please don't merge anything from there, or add more format-string warning stuff, until I have this released. I don't want to make the format string warnings obvious until the fix is public, in case the security impact turns out to be greater than I thought.

I have a branch adding and fixing more format-related warnings, which I'll send for review after this is released (this was the only one that looks security-significant). There's no point in someone else duplicating effort on that.
Comment 11 Simon McVittie 2016-10-10 14:26:43 UTC
Unembargoing and closing. Fixed in 1.8.22, 1.10.12, 1.11.6.

Also fixed on the dbus-1.6 branch post-1.6.30 in case any distributions are following that branch longer than we are willing to support it.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.