Bug 32321 - Lots of nested variants crash the bus
Summary: Lots of nested variants crash the bus
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.4.x
Hardware: x86 (IA32) All
: medium critical
Assignee: Havoc Pennington
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-11 09:14 UTC by Remi Denis-Courmont
Modified: 2010-12-20 13:50 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:

Attachments
Proof of concept code (5.49 KB, text/plain)
2010-12-11 09:14 UTC, Remi Denis-Courmont 		Details
Add failing test case (4.21 KB, patch)
2010-12-12 18:38 UTC, Havoc Pennington 		Details | Splinter Review
Detect deep nesting during validation (3.74 KB, patch)
2010-12-12 18:39 UTC, Havoc Pennington 		Details | Splinter Review
squashed patch (8.81 KB, patch)
2010-12-18 14:59 UTC, Colin Walters 		Details | Splinter Review
Show Obsolete (2) View All

Description Remi Denis-Courmont 2010-12-11 09:14:19 UTC 
Created attachment 41018 [details]
Proof of concept code

Justification for critical severity: crashes

Sending a "valid" D-Bus message with a (really) a lot of nested variants triggers a segmentation fault and termination of the bus. This seems like a security concern in the case of the system bus.

Proof of concept code is attached.
Comment 1 Havoc Pennington 2010-12-12 18:38:51 UTC 
Created attachment 41049 [details] [review]
Add failing test case
Comment 2 Havoc Pennington 2010-12-12 18:39:15 UTC 
Created attachment 41050 [details] [review]
Detect deep nesting during validation
Comment 3 Havoc Pennington 2010-12-12 18:40:28 UTC 
I haven't tested these patches much (and still don't have working ssh to push them) but hope they are helpful. Someone might want to run the proof of concept exploit with these patches to see if the fix works.

There should also be a patch to the spec but I couldn't decide where to put the new text so I just left it as an exercise for the patch applier. ;-)
Comment 4 Colin Walters 2010-12-15 14:06:54 UTC 
I'd prefer one patch with the fix and additional unit test; having failing tests without the fix is bad for bisecting.

Also this will need to reference a CVE number; i'm getting one assigned now.
Comment 5 Havoc Pennington 2010-12-16 08:15:18 UTC 
yeah, feel free to squash
Comment 6 Colin Walters 2010-12-16 13:05:53 UTC 
This issue has been assigned CVE-2010-4352.
Comment 7 Colin Walters 2010-12-18 14:59:19 UTC 
Created attachment 41245 [details] [review]
squashed patch

Squashed patch, with update to specification.
Comment 8 Colin Walters 2010-12-18 15:00:25 UTC 
I can verify the patch fixes this against dbus-1.4 git master.
Comment 9 Colin Walters 2010-12-18 15:03:48 UTC 
Empirically, the maximum variant nesting depth on my Fedora 14 system does not exceed 2.  I can barely think of a rational situation in which it's larger than 5 or 7, much less 64.

While it's sort of lame to add a restriction, there's no reason for us to bend over backwards to support this either, so I think this patch is a reasonable fix.
Comment 10 Colin Walters 2010-12-18 15:05:20 UTC 
Will, can we get this patch queued for the Monday release?
Comment 11 Will Thompson 2010-12-20 04:03:30 UTC 
(In reply to comment #10)
> Will, can we get this patch queued for the Monday release?

Absolutely. Are there mailing lists—besides the D-Bus list—that I should announce it to?

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.