Bug 32321 - Lots of nested variants crash the bus
Lots of nested variants crash the bus
Status: RESOLVED FIXED
Product: dbus
Classification: Unclassified
Component: core
1.4.x
x86 (IA32) All
: medium critical
Assigned To: Havoc Pennington
John (J5) Palmieri
:
Depends on:
Blocks:
  Show dependency treegraph
 
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

Note You need to log in before you can comment on or make changes to this bug.
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?