From f6554cf4a091c78afb55c6f4a3539439cc109e31 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 12 Jul 2018 19:11:05 +0100 Subject: [PATCH] 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" Signed-off-by: Simon McVittie --- dbus/dbus-marshal-validate.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c index 6a3bf46e..4d492f3f 100644 --- a/dbus/dbus-marshal-validate.c +++ b/dbus/dbus-marshal-validate.c @@ -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; } -- 2.18.0