From ac874b8fb13bf293986a3814149a820729b27a30 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 23 May 2023 18:09:23 +0200 Subject: [PATCH] sd-journal: avoid double-free If we fail to combine the new entry with a previous one, or update it in the hashmap, we might later on attempt a double-free: ================================================================= ==10==ERROR: AddressSanitizer: attempting double-free on 0x611000039fc0 in thread T0: SCARINESS: 42 (double-free) #0 0x4a0962 in __interceptor_free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3 #1 0x7f55e431d9f2 in _hashmap_clear /work/build/../../src/systemd/src/basic/hashmap.c:927:33 #2 0x7f55e431d4c8 in _hashmap_free /work/build/../../src/systemd/src/basic/hashmap.c:896:17 #3 0x4de1de in ordered_hashmap_free_free_free /work/build/../../src/systemd/src/basic/hashmap.h:120:24 #4 0x4de1de in ordered_hashmap_free_free_freep /work/build/../../src/systemd/src/basic/hashmap.h:434:1 #5 0x4de1de in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-catalog.c:26:1 #6 0x4de8b8 in NaloFuzzerTestOneInput (/build/fuzz-catalog+0x4de8b8) #7 0x4fd8c3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #8 0x4fd0aa in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:514:3 #9 0x4fe779 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #10 0x4ff445 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #11 0x4ee7af in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #12 0x4ef078 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #13 0x4deb35 in main (/build/fuzz-catalog+0x4deb35) #14 0x7f55e3a32082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) #15 0x41f7cd in _start (/build/fuzz-catalog+0x41f7cd) DEDUP_TOKEN: __interceptor_free--_hashmap_clear--_hashmap_free 0x611000039fc0 is located 0 bytes inside of 224-byte region [0x611000039fc0,0x61100003a0a0) freed by thread T0 here: #0 0x4a0962 in __interceptor_free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3 #1 0x7f55e451493d in freep /work/build/../../src/systemd/src/basic/alloc-util.h:107:22 #2 0x7f55e451493d in finish_item /work/build/../../src/systemd/src/libsystemd/sd-journal/catalog.c:187:1 #3 0x7f55e4513e56 in catalog_import_file /work/build/../../src/systemd/src/libsystemd/sd-journal/catalog.c:313:45 #4 0x4de1be in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-catalog.c:23:16 #5 0x4de8b8 in NaloFuzzerTestOneInput (/build/fuzz-catalog+0x4de8b8) #6 0x4fd8c3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #7 0x4fd0aa in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:514:3 #8 0x4fe779 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #9 0x4ff445 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #10 0x4ee7af in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #11 0x4ef078 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #12 0x4deb35 in main (/build/fuzz-catalog+0x4deb35) #13 0x7f55e3a32082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) DEDUP_TOKEN: __interceptor_free--freep--finish_item previously allocated by thread T0 here: #0 0x4a0c06 in __interceptor_malloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3 #1 0x4de539 in malloc (/build/fuzz-catalog+0x4de539) #2 0x7f55e42bf96b in memdup /work/build/../../src/systemd/src/basic/alloc-util.c:16:15 #3 0x7f55e451475d in finish_item /work/build/../../src/systemd/src/libsystemd/sd-journal/catalog.c:176:28 #4 0x7f55e4513e56 in catalog_import_file /work/build/../../src/systemd/src/libsystemd/sd-journal/catalog.c:313:45 #5 0x4de1be in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-catalog.c:23:16 #6 0x4de8b8 in NaloFuzzerTestOneInput (/build/fuzz-catalog+0x4de8b8) #7 0x4fd8c3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #8 0x4fd0aa in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:514:3 #9 0x4fe779 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:757:19 #10 0x4ff445 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:895:5 #11 0x4ee7af in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:912:6 #12 0x4ef078 in LLVMFuzzerRunDriver /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:925:10 #13 0x4deb35 in main (/build/fuzz-catalog+0x4deb35) #14 0x7f55e3a32082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee) DEDUP_TOKEN: __interceptor_malloc--malloc--memdup SUMMARY: AddressSanitizer: double-free /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3 in __interceptor_free Found by Nallocfuzz. --- src/libsystemd/sd-journal/catalog.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-journal/catalog.c b/src/libsystemd/sd-journal/catalog.c index 7527abf636c..78f7f226c89 100644 --- a/src/libsystemd/sd-journal/catalog.c +++ b/src/libsystemd/sd-journal/catalog.c @@ -145,7 +145,8 @@ static int finish_item( char *payload, size_t payload_size) { _cleanup_free_ CatalogItem *i = NULL; - _cleanup_free_ char *prev = NULL, *combined = NULL; + _cleanup_free_ char *combined = NULL; + char *prev; assert(h); assert(payload); @@ -171,6 +172,7 @@ static int finish_item( if (ordered_hashmap_update(h, i, combined) < 0) return log_oom(); combined = NULL; + free(prev); } else { /* A new item */ combined = memdup(payload, payload_size + 1);