Comments on improving FuzzTargetNg

[CWResearcher]

Description

A potential fuzz blocker has been identified in the pcapplusplus fuzzer within the OSS-Fuzz project, due to a null-pointer-dereference issue. We kindly request a review of the following report for further details and assessment.

Log

root@55e0479c3c23:/out/pcapplusplus# ./FuzzTargetNg ./pcapplusplus--FuzzTargetNg--crash-d225aed676f1595210b1b424ea739971-2023-12-06-21\:13\:55
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2911733560
INFO: Loaded 1 modules   (37646 inline 8-bit counters): 37646 [0xae5ea8, 0xaef1b6),
INFO: Loaded 1 PC tables (37646 PCs): 37646 [0xaef1b8,0xb82298),
./FuzzTargetNg: Running 1 inputs 1 time(s) each.
Running: ./pcapplusplus--FuzzTargetNg--crash-d225aed676f1595210b1b424ea739971-2023-12-06-21:13:55
Read 0 packets successfully and 0 packets could not be read
OS is ''; Hardware is '''; CaptureApplication is ''; CaptureFileComment is ''
AddressSanitizer:DEADLYSIGNAL
=================================================================
==43==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000048 (pc 0x0000005f1a6f bp 0x7ffe6c63abf0 sp 0x7ffe6c63ab60 T0)
==43==The signal is caused by a READ memory access.
==43==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
    #0 0x5f1a6f in __append_interface_block_to_file_info /src/PcapPlusPlus/3rdParty/LightPcapNg/LightPcapNg/src/light_pcapng_ext.c:145:12
    #1 0x5f2de8 in light_get_next_packet /src/PcapPlusPlus/3rdParty/LightPcapNg/LightPcapNg/src/light_pcapng_ext.c:392:4
    #2 0x5c74fe in pcpp::PcapNgFileReaderDevice::getNextPacket(pcpp::RawPacket&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&) /src/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:371:7
    #3 0x5c8732 in pcpp::PcapNgFileReaderDevice::getNextPacket(pcpp::RawPacket&) /src/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:404:9
    #4 0x5c0d0c in pcpp::IFileReaderDevice::getNextPackets(pcpp::PointerVector<pcpp::RawPacket>&, int) /src/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:107:21
    #5 0x5aaf5a in LLVMFuzzerTestOneInput /src/PcapPlusPlus/Tests/Fuzzers/FuzzTarget.cpp:47:14
    #6 0x47bab3 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
    #7 0x467212 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
    #8 0x46cabc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
    #9 0x495ff2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #10 0x7fdd3cdae082 in __libc_start_main /build/glibc-BHL3KM/glibc-2.31/csu/../csu/libc-start.c:308:16
    #11 0x45d3dd in _start (/out/pcapplusplus/FuzzTargetNg+0x45d3dd)

DEDUP_TOKEN: __append_interface_block_to_file_info--light_get_next_packet--pcpp::PcapNgFileReaderDevice::getNextPacket(pcpp::RawPacket&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >&)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /src/PcapPlusPlus/3rdParty/LightPcapNg/LightPcapNg/src/light_pcapng_ext.c:145:12 in __append_interface_block_to_file_info
==43==ABORTING

Analyze

Based on the crash log information, it seems that the cause of the crash was due to a NULL being passed as an argument to a structure pointer.

Below is a capture confirming that a NULL value is actually being passed as a function argument.

[seladb]

Thanks for reporting this issue @CWResearcher ! Can I ask what is FuzzTargetNg?

Since you already analyzed the problem, would you consider introducing a fix?

[CWResearcher]

FuzzTargetNg is a fuzzer binary name. It is believed to be compiled from the following source code:

https://github.com/seladb/PcapPlusPlus/blob/master/Tests/Fuzzers/FuzzTarget.cpp

If you are maintaining this source code, I recommend adding validation logic for NULL values in structure pointers.

[seladb]

FuzzTargetNg is a fuzzer binary name. It is believed to be compiled from the following source code:

https://github.com/seladb/PcapPlusPlus/blob/master/Tests/Fuzzers/FuzzTarget.cpp

If you are maintaining this source code, I recommend adding validation logic for NULL values in structure pointers.

Got it thanks! @CWResearcher would you consider opening a PR with a fix?

[seladb]

@CWResearcher I looked again at the screenshots you shared. In order to debug it and provide a fix I need the input file that was given to the fuzzer. The crash seems to be related to the pcapng file parser. Do you have this input file?

[egecetin]

@CWResearcher Thanks for reporting the issue. Fixed with #1636. Just for cross-referencing @seladb, with #1348 we should also consider full check of LightPcapNg source for possible problems.