validate_body_helper: Bounds-check before validating booleans
authorSimon McVittie <smcv@collabora.com>
Thu, 12 Jul 2018 18:11:05 +0000 (19:11 +0100)
committerSimon McVittie <smcv@collabora.com>
Thu, 2 Aug 2018 18:21:35 +0000 (19:21 +0100)
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=107332
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Thiago Macieira <thiago@kde.org>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit e93a775e68daeda5c95984452aee6327e31c17dd)

dbus/dbus-marshal-validate.c

index 6a3bf46..4d492f3 100644 (file)
@@ -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;
             }