write heap buffer overflow in dlt_buffer_write_block

[schrewe]

Hello,

I have fuzzed dlt-daemon to search for bugs using the Tool CI-Fuzz. I think I found a write heap buffer overflow in the function dlt_buffer_write_block. You can find the full crash report below. This is from a fuzz test I wrote to execute the function dlt_buffer_push with fuzzer generated inputs.

After having a look at the code it seems that there is no size check before writing to the ring buffer: There is no check if size<=buf->size in dlt_buffer_write_block.
As far as I can say this condition isn't checked elsewhere. Since buffer overflows can often be used to get arbitrary code execution this is a potential security issue. Using the dlt_daemon_client_send function a connected dlt user could write into the memory of the daemon and possibly gain arbitrary code execution.

I think this could be fixed by changing the if condition in dlt_common.c:2508 (in function dlt_buffer_push3) into a while loop.
If you need further information or discussion I'm happy to help as far as I can.

==14==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6260003ec810 at pc 0x000000495890 bp 0x7ffca5a15a50 sp 0x7ffca5a15218
WRITE of size 10006 at 0x6260003ec810 thread T0
#0 0x49588f in __asan_memcpy /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3
#1 0x7fe86a1c316b in dlt_buffer_write_block /home/user/.local/share/code-intelligence/projects/dlt-daemon-8b34ed1f/libfuzzer/address/src/shared/dlt_common.c:2281:13
#2 0x7fe86a1cf7b9 in dlt_buffer_push3 /home/user/.local/share/code-intelligence/projects/dlt-daemon-8b34ed1f/libfuzzer/address/src/shared/dlt_common.c:2530:9
#3 0x7fe86a1cd2ca in dlt_buffer_push /home/user/.local/share/code-intelligence/projects/dlt-daemon-8b34ed1f/libfuzzer/address/src/shared/dlt_common.c:2460:12
#4 0x4c5cda in LLVMFuzzerTestOneInput /home/user/.local/share/code-intelligence/projects/dlt-daemon-8b34ed1f/libfuzzer/address/.code-intelligence/fuzz_targets/fuzz_dlt_common_dlt_buffer_push.c:81:4
#5 0x4feb61 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:556:15
#6 0x4fe2a5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:470:3
#7 0x500bc7 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocatorfuzzer::SizedFile >&) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:765:7
#8 0x500f29 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocatorfuzzer::SizedFile >&) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:792:3
#9 0x4efbee in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:829:6
#10 0x518a22 in main /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerMain.cpp:19:10
#11 0x7fe869bb00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
#12 0x41de5d in _start (/projects/dlt-daemon-8b34ed1f/libfuzzer/address/fuzz_dlt_common_dlt_buffer_push+0x41de5d)

0x6260003ec810 is located 0 bytes to the right of 10000-byte region [0x6260003ea100,0x6260003ec810)
allocated by thread T0 here:
#0 0x49638d in malloc /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x7fe86a1c5f2e in dlt_buffer_increase_size /home/user/.local/share/code-intelligence/projects/dlt-daemon-8b34ed1f/libfuzzer/address/src/shared/dlt_common.c:2344:15
#2 0x7fe86a1cec2f in dlt_buffer_push3 /home/user/.local/share/code-intelligence/projects/dlt-daemon-8b34ed1f/libfuzzer/address/src/shared/dlt_common.c:2510:13
#3 0x7fe86a1cd2ca in dlt_buffer_push /home/user/.local/share/code-intelligence/projects/dlt-daemon-8b34ed1f/libfuzzer/address/src/shared/dlt_common.c:2460:12
#4 0x4c5cda in LLVMFuzzerTestOneInput /home/user/.local/share/code-intelligence/projects/dlt-daemon-8b34ed1f/libfuzzer/address/.code-intelligence/fuzz_targets/fuzz_dlt_common_dlt_buffer_push.c:81:4
#5 0x4feb61 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:556:15
#6 0x4fe2a5 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:470:3
#7 0x500bc7 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocatorfuzzer::SizedFile >&) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:765:7
#8 0x500f29 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocatorfuzzer::SizedFile >&) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:792:3
#9 0x4efbee in fuzzer::FuzzerDriver(int*, char***, int ()(unsigned char const, unsigned long)) /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:829:6
#10 0x518a22 in main /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/fuzzer/FuzzerMain.cpp:19:10
#11 0x7fe869bb00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

SUMMARY: AddressSanitizer: heap-buffer-overflow /llvmbuild/llvm-project-llvmorg-10.0.0/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 in __asan_memcpy
Shadow bytes around the buggy address:
0x0c4c800758b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c4c800758c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c4c800758d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c4c800758e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c4c800758f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4c80075900: 00 00[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c4c80075910: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c4c80075920: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c4c80075930: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c4c80075940: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c4c80075950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==14==ABORTING
MS: 0 ; base unit: 0000000000000000000000000000000000000000
artifact_prefix='./'; Test unit written to ./crash-0dc6c277a5b5996e2eb49350577026f1820bb1e7

[schrewe]

Here a more detailed description how an attack using this bug could look like:
A attacker who has control over a connected dlt user (a application that uses the daemon to store logs via the shared library Dlt Library, see https://github.com/GENIVI/dlt-daemon/blob/master/doc/dlt_design_specification.md) can send arbitrary log/trace messages to the Dlt Daemon.
The deamon processes the message. At some point the function dlt_daemon_client_send is called with the potentially malicious message as the "void* data1" parameter. Under the right circumstances dlt_buffer_push3 in dlt_daemon_client.c:279 will be executed. The ringbuffer created at dlt_daemon_common.c:245 using dlt_buffer_init_dynamic(&(daemon->client_ringbuffer), RingbufferMinSize, RingbufferMaxSize, RingbufferStepSize). In the tests the sizes dlt_buffer_init_dynamic(&dlt_buffer, 5000, 50000, 5000) are used, so lets assume the initial size of the buffer is 5000. Additionally assume that the ringbuffer is empty, so the check in dlt_daemon_client:275 is not triggered.
The program will execute dlt_buffer_push3 and reach dlt_common.c:2579
If the message is bigger than 5000, dlt_buffer_increase_size will increase the buffer size by RingbufferStepSize, so the new size is 10000. The case that the message might be bigger than 10000 is missed here, since the condition in line 2508 is not checked again after calling dlt_buffer_increase_size! Assume our message had a size of 22000. Line 2530 is reached and dlt_buffer_write_block is called. The if condition in dlt_common.c:2365 evaluates to false, since size=22000 and buf->size=10000. Since write=0 we call memcpy(buf->mem, data, 10000) in line 2372 writing the first 1000 bytes of the message to buf->mem. After this in line 2373 we execute memcpy(buf->mem, data + buf->size, size - buf->size) which is in our case memcpy(buf->mem, data + 10000, 22000 - 10000). This results is using memcpy to write 12000 bytes to a 10000 byte buffer, which is a write buffer overflow.
Since such write buffer overflows can often be used to execute arbitrary code the attacker could possibly (I'm not an expert on exploits) compromise the Dlt Daemon.

Since I am not an expert regarding the code of Dlt Daemon it is possible that I got something wrong somewhere. I would be happy if someone could have a look at it.

edit: Somehow the line numbers from my IDE didn't match the actual line numbers of the sources files in the repo. I corrected the line numbers and added links to the files in the git repo of v2.18.5. Now this should be understandable. Sorry for that.

[jeremiah]

Ideally a mitigation would be provided as well if you have one available. I'll bring this issue to the attention of the maintainers.

[schrewe]

Great, thank you! Unfortunately I am not aware of a mitigation.

[schrewe]

Hello,
I created a pull request which, as I think, should fix the bug.
@jeremiah are there any news yet?

[jeremiah]

Hello - we need to bring this issue to the attention of the maintainer.
@GeniviDLTmaintainer, @ManikandanChockalingam have you seen this issue?

[ssugiura]

@schrewe @jeremiah Sorry for my late response. We will take a look into the issue and proposed commit and proceed very soon. Thanks.

[gy741]

Hello, @schrewe

Can you provide a testcase?

[schrewe]

This vulnerability was assigned the CVE CVE-2020-36244

[carnil]

Is this issue fixed with af734fe and so this issue be closed?

[ghost]

do we have a unit test to track this fix? Thanks

[Stimfarmer]

Hello,
Is it possible to get the crashing seed mentioned here ? Thank you

artifact_prefix='./'; Test unit written to ./crash-0dc6c277a5b5996e2eb49350577026f1820bb1e7

Regards