1
0
mirror of https://github.com/systemd/systemd.git synced 2024-12-25 01:34:28 +03:00
Commit Graph

13 Commits

Author SHA1 Message Date
Zbigniew Jędrzejewski-Szmek
f0ae945ecc bus-message: validate signature in gvariant messages
We would accept a message with 40k signature and spend a lot of time iterating
over the nested arrays. Let's just reject it early, as we do for !gvariant
messages.
2019-04-11 14:01:38 +02:00
Zbigniew Jędrzejewski-Szmek
902000c198 bus-message: avoid wrap-around when using length read from message
We would read (-1), and then add 1 to it, call message_peek_body(..., 0, ...),
and when trying to make use of the data.

The fuzzer test case is just for one site, but they all look similar.

v2: fix two UINT8_MAX/UINT32_MAX mismatches founds by LGTM
2018-10-02 11:59:08 +02:00
Zbigniew Jędrzejewski-Szmek
d831fb6f2b bus-message: return -EBADMSG not -EINVAL on invalid !gvariant messages 2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
edde66ffc2 fuzz-bus-message: add two test cases that pass now
It seems that they got fixed by one of the patches. Let's add them
just in case.
2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
73777ddba5 bus-message: fix skipping of array fields in !gvariant messages
We copied part of the string into a buffer that was off by two.
If the element signature had length one, we'd copy 0 bytes and crash when
looking at the "first" byte. Otherwise, we would crash because strncpy would
not terminate the string.
2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
f88214cf9d bus-message: fix calculation of offsets table for arrays
This is similar to the grandparent commit 'fix calculation of offsets table',
except that now the change is for array elements. Same story as before: we need
to make sure that the offsets increase enough taking alignment into account.

While at it, rename 'p' to 'previous' to match similar code in other places.
2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
12603b84d2 bus-message: fix calculation of offsets table
The offsets specify the ends of variable length data. We would trust the
incoming data, putting the offsets specified in our message
into the offsets tables after doing some superficial verification.
But when actually reading the data we apply alignment, so we would take
the previous offset, align it, making it bigger then current offset, and
then we'd try to read data of negative length.

In the attached example, the message specifies the following offsets:
[1, 4]
but the alignment of those items is
[1, 8]
so we'd calculate the second item as starting at 8 and ending at 4.
2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
81b6e63029 bus-message: do not crash on message with a string of zero length
We'd calculate the "real" length of the string as 'item_size - 1', which does
not work out well when item_size == 0.
2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
69bd42ca07 bus-message: let's always use -EBADMSG when the message is bad
-EINVAL means the arguments were somehow wrong, so translate the code we get
internally into -EBADMSG when returning.
2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
ec6bda56cb bus-message: avoid an infinite loop on empty structures
The alternative would be to treat gvariant and !gvariant messages differently.
But this is a problem because we check signatures is variuos places before we
have an actual message, for example in sd_bus_add_object_vtable(). It seems
better to treat things consistent (i.e. follow the lowest common denominator)
and disallow empty structures everywhere.
2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
6d1e0f4fcb sd-bus: unify three code-paths which free struct bus_container
We didn't free one of the fields in two of the places.

$ valgrind --show-leak-kinds=all --leak-check=full \
  build/fuzz-bus-message \
  test/fuzz/fuzz-bus-message/leak-c09c0e2256d43bc5e2d02748c8d8760e7bc25d20
...
==14457== HEAP SUMMARY:
==14457==     in use at exit: 3 bytes in 1 blocks
==14457==   total heap usage: 509 allocs, 508 frees, 51,016 bytes allocated
==14457==
==14457== 3 bytes in 1 blocks are definitely lost in loss record 1 of 1
==14457==    at 0x4C2EBAB: malloc (vg_replace_malloc.c:299)
==14457==    by 0x53AFE79: strndup (in /usr/lib64/libc-2.27.so)
==14457==    by 0x4F52EB8: free_and_strndup (string-util.c:1039)
==14457==    by 0x4F8E1AB: sd_bus_message_peek_type (bus-message.c:4193)
==14457==    by 0x4F76CB5: bus_message_dump (bus-dump.c:144)
==14457==    by 0x108F12: LLVMFuzzerTestOneInput (fuzz-bus-message.c:24)
==14457==    by 0x1090F7: main (fuzz-main.c:34)
==14457==
==14457== LEAK SUMMARY:
==14457==    definitely lost: 3 bytes in 1 blocks
2018-10-02 11:53:20 +02:00
Zbigniew Jędrzejewski-Szmek
7f546026ab Introduce free_and_strndup and use it in bus-message.c
v2: fix error in free_and_strndup()

When the orignal and copied message were the same, but shorter than specified
length l, memory read past the end of the buffer would be performed. A test
case is included: a string that had an embedded NUL ("q\0") is used to replace
"q".

v3: Fix one more bug in free_and_strndup and add tests.

v4: Some style fixed based on review, one more use of free_and_replace, and
make the tests more comprehensive.
2018-10-02 11:42:45 +02:00
Zbigniew Jędrzejewski-Szmek
56b560c263 fuzz-bus-message: add fuzzer for message parsing
As with other fuzzers, SYSTEMD_FUZZ_OUTPUT=1 and SYSTEMD_LOG_LEVEL=debug can be
used for debugging.
2018-10-02 11:09:05 +02:00