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.
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.
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.
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.
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.
(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 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 on attachment 127125 [details] [review] bus_driver_handle_message: reject ActivationFailure if unprivileged Review of attachment 127125 [details] [review]: ----------------------------------------------------------------- LGTM.
Comment on attachment 127127 [details] [review] Ignore ActivationFailure if not using systemd activation Review of attachment 127127 [details] [review]: ----------------------------------------------------------------- OK.
Comment on attachment 127128 [details] [review] Disable deprecation warnings for stable branch Review of attachment 127128 [details] [review]: ----------------------------------------------------------------- OK.
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.
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.