Commit e93a775e authored by Simon McVittie's avatar Simon McVittie

validate_body_helper: Bounds-check before validating booleans

Running the "embedded tests" through valgrind revealed that before this
commit, 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). This is not believed to be exploitable
to leak interesting information.

Fixes: 62e46533 "hardcode dbus_bool_t to 32 bits"
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107332Signed-off-by: Simon McVittie's avatarSimon McVittie <smcv@collabora.com>
Reviewed-by: Thiago Macieira's avatarThiago Macieira <thiago@kde.org>
Reviewed-by: Philip Withnall's avatarPhilip Withnall <withnall@endlessm.com>
parent 5f02cfeb
......@@ -358,8 +358,13 @@ validate_body_helper (DBusTypeReader *reader,
if (current_type == DBUS_TYPE_BOOLEAN)
{
dbus_uint32_t v = _dbus_unpack_uint32 (byte_order,
p);
dbus_uint32_t v;
if (p + 4 > end)
return DBUS_INVALID_NOT_ENOUGH_DATA;
v = _dbus_unpack_uint32 (byte_order, p);
if (!(v == 0 || v == 1))
return DBUS_INVALID_BOOLEAN_NOT_ZERO_OR_ONE;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment