Created attachment 140779 [details] [review] validate_body_helper: Bounds-check before validating booleans Running the "embedded tests" through valgrind revealed that without the attached patch, we would have been willing to read up to 3 bytes off the end of a message if the message is truncated part way through a boolean. Any practical allocator will round up allocations to the next 32-bit (or larger) boundary, so in practice this will not leave the memory buffer (and in particular did not crash during unit testing), but it could read uninitialized contents. On little-endian CPUs, an attacker might be able to use this to learn whether up to 3 bytes of uninitialized memory in the dbus-daemon were all-zero (their crafted message would be relayed) or not (their connection would be disconnected for sending an invalid message). On big-endian CPUs, an attacker might be able to use this to learn whether up to 3 bytes were all-zeroes (relayed to a cooperating peer), 0-2 bytes of all-zeroes followed by 0x01 (relayed to a cooperating peer), or something else (disconnected). --- I don't think this is practically exploitable, so I'll probably handle this as an ordinary bug rather than as an embargoed security vulnerability; but I wanted to give other D-Bus maintainers a chance to object and point out ways it could be exploited if they disagree with me, so for now it's private.
This bug was introduced in early 2005 (62e46533 "hardcode dbus_bool_t to 32 bits") so all supported dbus versions are affected.
I'll also need reviews on Bug #107349 and Bug #107350 before I can actually release this fix.
Comment on attachment 140779 [details] [review] validate_body_helper: Bounds-check before validating booleans Review of attachment 140779 [details] [review]: ----------------------------------------------------------------- Looks good to me. Ship it.
(In reply to Thiago Macieira from comment #3) > Looks good to me. Ship it. Do you agree with my assessment that this is probably not a practical security vulnerability, or do you think we need to do the embargo/CVE thing?
Comment on attachment 140779 [details] [review] validate_body_helper: Bounds-check before validating booleans Review of attachment 140779 [details] [review]: ----------------------------------------------------------------- r+ I can’t think of an easy way to exploit this, unless there’s a way for the attacker to guarantee that the heap allocation for their crafted message is over the top of some previously-freed memory which they care about getting some bytes from. Maybe we should explicitly zero the allocation of an incoming message before doing anything about parsing that message, as paranoia? That would only guard against bugs of this class found in future releases though.
Making this public since we have concluded that it isn't a security flaw.
Fixed in git for 1.12.0, 1.13.6 and 1.10.28, thanks
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.