From a9c9f79ecedef886f3a281fb867010c3ef4fdd95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 21 Jun 2020 16:15:23 +0200 Subject: [PATCH 1/3] Revert "bus-message: immediately reject messages with invalid type" This reverts commit a2dd991d0fde59dc0574bd4d0c1438f01dc0b8ff. Creation of such messages is evidently useful, and at least sdbus-c++ test suite depends on that. Fixes #16193. --- src/libsystemd/sd-bus/bus-message.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 97b732ae42..4005023d24 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -451,7 +451,7 @@ int bus_message_from_header( if (!IN_SET(h->version, 1, 2)) return -EBADMSG; - if (h->type <= _SD_BUS_MESSAGE_TYPE_INVALID || h->type >= _SD_BUS_MESSAGE_TYPE_MAX) + if (h->type == _SD_BUS_MESSAGE_TYPE_INVALID) return -EBADMSG; if (!IN_SET(h->endian, BUS_LITTLE_ENDIAN, BUS_BIG_ENDIAN)) @@ -589,7 +589,8 @@ _public_ int sd_bus_message_new( assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(bus->state != BUS_UNSET, -ENOTCONN); assert_return(m, -EINVAL); - assert_return(type > _SD_BUS_MESSAGE_TYPE_INVALID && type < _SD_BUS_MESSAGE_TYPE_MAX, -EINVAL); + /* Creation of messages with _SD_BUS_MESSAGE_TYPE_INVALID is allowed. */ + assert_return(type < _SD_BUS_MESSAGE_TYPE_MAX, -EINVAL); sd_bus_message *t = malloc0(ALIGN(sizeof(sd_bus_message)) + sizeof(struct bus_header)); if (!t) @@ -5497,9 +5498,6 @@ int bus_message_parse_fields(sd_bus_message *m) { if (m->reply_cookie == 0 || !m->error.name) return -EBADMSG; break; - - default: - assert_not_reached("Bad message type"); } /* Refuse non-local messages that claim they are local */ From b17af3e503a5861ae1437254a8a2a3600e768cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 22 Jun 2020 17:09:49 +0200 Subject: [PATCH 2/3] bus-message: avoid dereferencing a NULL pointer We'd try to map a zero-byte buffer from a NULL pointer, which is undefined behaviour. src/systemd/src/libsystemd/sd-bus/bus-message.c:3161:60: runtime error: applying zero offset to null pointer #0 0x7f6ff064e691 in find_part /work/build/../../src/systemd/src/libsystemd/sd-bus/bus-message.c:3161:60 #1 0x7f6ff0640788 in message_peek_body /work/build/../../src/systemd/src/libsystemd/sd-bus/bus-message.c:3283:16 #2 0x7f6ff064e8db in enter_struct_or_dict_entry /work/build/../../src/systemd/src/libsystemd/sd-bus/bus-message.c:3967:21 #3 0x7f6ff06444ac in bus_message_enter_struct /work/build/../../src/systemd/src/libsystemd/sd-bus/bus-message.c:4009:13 #4 0x7f6ff0641dde in sd_bus_message_enter_container /work/build/../../src/systemd/src/libsystemd/sd-bus/bus-message.c:4136:21 #5 0x7f6ff0619874 in sd_bus_message_dump /work/build/../../src/systemd/src/libsystemd/sd-bus/bus-dump.c:178:29 #6 0x4293d9 in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-bus-message.c:39:9 #7 0x441986 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:558:15 #8 0x44121e in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/libfuzzer/FuzzerLoop.cpp:470:3 #9 0x443164 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector >&) /src/libfuzzer/FuzzerLoop.cpp:770:7 #10 0x4434bc in fuzzer::Fuzzer::Loop(std::__1::vector >&) /src/libfuzzer/FuzzerLoop.cpp:799:3 #11 0x42d2bc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:846:6 #12 0x42978a in main /src/libfuzzer/FuzzerMain.cpp:19:10 #13 0x7f6fef13c82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f) #14 0x407808 in _start (out/fuzz-bus-message+0x407808) --- src/libsystemd/sd-bus/bus-message.c | 3 ++- .../fuzz-bus-message/zero-offset-to-null-pointer | Bin 0 -> 64 bytes 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 test/fuzz/fuzz-bus-message/zero-offset-to-null-pointer diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 4005023d24..8a8fbde781 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -3159,7 +3159,8 @@ static struct bus_body_part* find_part(sd_bus_message *m, size_t index, size_t s return NULL; if (p) - *p = (uint8_t*) part->data + index - begin; + *p = part->data ? (uint8_t*) part->data + index - begin + : NULL; /* Avoid dereferencing a NULL pointer. */ m->cached_rindex_part = part; m->cached_rindex_part_begin = begin; diff --git a/test/fuzz/fuzz-bus-message/zero-offset-to-null-pointer b/test/fuzz/fuzz-bus-message/zero-offset-to-null-pointer new file mode 100644 index 0000000000000000000000000000000000000000..c1380441ed1a85b8e37d11453effdb7f9f4fd4d1 GIT binary patch literal 64 zcmc~16k%ilf{d(u23;V{!I;jVuAxzpr&*d&S(sQ-TA`Uzkpu>bKoZIX^D{vD0P6P< AZU6uP literal 0 HcmV?d00001 From b98f393d88262dc7afc321ac48c228d3b67b0178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 22 Jun 2020 17:18:35 +0200 Subject: [PATCH 3/3] bus-message: add macro for calculation of offset from the page --- src/basic/memory-util.h | 1 + src/libsystemd/sd-bus/bus-message.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/basic/memory-util.h b/src/basic/memory-util.h index a6a2ccdbc2..4f596cffb7 100644 --- a/src/basic/memory-util.h +++ b/src/basic/memory-util.h @@ -13,6 +13,7 @@ size_t page_size(void) _pure_; #define PAGE_ALIGN(l) ALIGN_TO((l), page_size()) #define PAGE_ALIGN_DOWN(l) ((l) & ~(page_size() - 1)) +#define PAGE_OFFSET(l) ((l) & (page_size() - 1)) /* Normal memcpy requires src to be nonnull. We do nothing if n is 0. */ static inline void memcpy_safe(void *dst, const void *src, size_t n) { diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 8a8fbde781..55e35cd902 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -3022,7 +3022,7 @@ int bus_body_part_map(struct bus_body_part *part) { return 0; } - shift = part->memfd_offset - ((part->memfd_offset / page_size()) * page_size()); + shift = PAGE_OFFSET(part->memfd_offset); psz = PAGE_ALIGN(part->size + shift); if (part->memfd >= 0)