[Bug] Heap-Buffer-Overflow in `RawPacket::removeData()`

[Rulkallos]

Bug description

Summary

pocs.tar.gz

AddressSanitizer detected a heap-buffer-overflow in the function pcpp::RawPacket::removeData() from RawPacket.cpp when handling buffer removal and memmove operations. This occurs during operations such as BgpUpdateMessageLayer::setWithdrawnRoutes() that modify the packet structure.

Root Cause Analysis

Suspected Problematic Code

bool RawPacket::removeData(int atIndex, size_t numOfBytesToRemove)
{
    if ((atIndex + (int)numOfBytesToRemove) > m_RawDataLen)
    {
        PCPP_LOG_ERROR("Remove section is out of raw packet bound");
        return false;
    }

    if ((atIndex + (int)numOfBytesToRemove) != m_RawDataLen)
        memmove((uint8_t*)m_RawData + atIndex,
                (uint8_t*)m_RawData + atIndex + numOfBytesToRemove,
                m_RawDataLen - (atIndex + numOfBytesToRemove));

    m_RawDataLen -= numOfBytesToRemove;
    m_FrameLength = m_RawDataLen;
    return true;
}

• The code checks that the remove region fits inside the current buffer, but when the removal occurs exactly at the end of the buffer, the memmove call might copy a length of zero, which is OK, but the buffer size may not be updated before the next access.
• If a logic error elsewhere causes this to be called with a removal region at or beyond the buffer end, an OOB write may occur.

Possible Explanation

• The bounds checking in removeData may not be sufficient, especially if upstream logic calculates atIndex or numOfBytesToRemove incorrectly.
• A removal region at the end of the buffer (atIndex + numOfBytesToRemove == m_RawDataLen) skips the memmove, but the underlying logic (e.g., in shortenLayer, or its callers) may have already calculated an out-of-bounds region.

Suggested Fix

• Ensure atIndex >= 0 and numOfBytesToRemove > 0.
• Make sure (atIndex + (int)numOfBytesToRemove) <= m_RawDataLen, and also check that atIndex < m_RawDataLen.
• Ensure all callers of removeData and related layer resizing functions calculate offsets and sizes correctly, especially when shrinking or modifying packet contents.

Platform

• OS: Ubuntu 22.04.5 LTS
• Clang version: Ubuntu clang version 16.0.6 (++20231112100510+7cbf1a259152-1~exp1~20231112100554.106)
• Version commit 0588d88eca769a02e660161f8965ef6152ada90e

Steps to Reproduce

export SRC=/src OUT=/out
mkdir $SRC $OUT

apt-get update && apt-get install -y cmake autoconf flex bison zip
export CC=clang CXX=clang++ CFLAGS='-fsanitize=address -g -O1' CXXFLAGS='-fsanitize=address -g -O1' LDFLAGS="-fuse-ld=lld" LIB_FUZZING_ENGINE='-fsanitize=fuzzer'

git clone --depth=1 https://github.com/seladb/PcapPlusPlus PcapPlusPlus
git clone --depth=1 https://github.com/the-tcpdump-group/tcpdump.git tcpdump
git clone --depth=1 https://github.com/the-tcpdump-group/libpcap.git libpcap

cd $SRC/PcapPlusPlus
$SRC/PcapPlusPlus/Tests/Fuzzers/ossfuzz.sh

$OUT/FuzzTargetNg poc

ASAN report

=================================================================
==530122==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x613000000368 at pc 0x5878794e71f7 bp 0x7ffd511492d0 sp 0x7ffd51148a98
WRITE of size 52 at 0x613000000368 thread T0
    #0 0x5878794e71f6 in __asan_memmove (/out/FuzzTargetNg+0x2411f6) (BuildId: 97bfbeafd983e755)
    #1 0x587879665308 in pcpp::RawPacket::removeData(int, unsigned long) /src/PcapPlusPlus/Packet++/src/RawPacket.cpp:170:4
    #2 0x587879634464 in pcpp::Packet::shortenLayer(pcpp::Layer*, int, unsigned long) /src/PcapPlusPlus/Packet++/src/Packet.cpp:646:21
    #3 0x5878796155e8 in pcpp::Layer::shortenLayer(int, unsigned long) /src/PcapPlusPlus/Packet++/src/Layer.cpp:114:20
    #4 0x58787957f645 in pcpp::BgpUpdateMessageLayer::setWithdrawnRoutes(std::vector<pcpp::BgpUpdateMessageLayer::prefix_and_ip, std::allocator<pcpp::BgpUpdateMessageLayer::prefix_and_ip>> const&) /src/PcapPlusPlus/Packet++/src/BgpLayer.cpp:581:15
    #5 0x58787952bb62 in readParsedPacket(pcpp::Packet, pcpp::Layer*) /src/PcapPlusPlus/Tests/Fuzzers/ReadParsedPacket.h:451:24
    #6 0x587879525b50 in LLVMFuzzerTestOneInput /src/PcapPlusPlus/Tests/Fuzzers/FuzzTarget.cpp:66:5
    #7 0x587879434e90 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) crtstuff.c
    #8 0x58787941ed6f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) crtstuff.c
    #9 0x587879424826 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) crtstuff.c
    #10 0x58787944dcf2 in main (/out/FuzzTargetNg+0x1a7cf2) (BuildId: 97bfbeafd983e755)
    #11 0x76379e82ad8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #12 0x76379e82ae3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #13 0x587879419c74 in _start (/out/FuzzTargetNg+0x173c74) (BuildId: 97bfbeafd983e755)

0x613000000368 is located 0 bytes after 360-byte region [0x613000000200,0x613000000368)
allocated by thread T0 here:
    #0 0x5878794e7c48 in calloc (/out/FuzzTargetNg+0x241c48) (BuildId: 97bfbeafd983e755)
    #1 0x5878795704bd in parse_by_block_type /src/PcapPlusPlus/3rdParty/LightPcapNg/LightPcapNg/src/light_pcapng.c:165:16
    #2 0x587879571244 in light_read_record /src/PcapPlusPlus/3rdParty/LightPcapNg/LightPcapNg/src/light_pcapng.c:377:4
    #3 0x58787956be79 in light_get_next_packet /src/PcapPlusPlus/3rdParty/LightPcapNg/LightPcapNg/src/light_pcapng_ext.c:397:3
    #4 0x58787953ca4b in pcpp::PcapNgFileReaderDevice::getNextPacket(pcpp::RawPacket&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&) /src/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:421:8
    #5 0x58787953f1ae in pcpp::PcapNgFileReaderDevice::getNextPacket(pcpp::RawPacket&) /src/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:462:10
    #6 0x5878795330e3 in pcpp::IFileReaderDevice::getNextPackets(pcpp::PointerVector<pcpp::RawPacket, std::default_delete<pcpp::RawPacket>>&, int) /src/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:135:22

SUMMARY: AddressSanitizer: heap-buffer-overflow (/out/FuzzTargetNg+0x2411f6) (BuildId: 97bfbeafd983e755) in __asan_memmove
Shadow bytes around the buggy address:
  0x613000000080: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x613000000100: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x613000000180: fd fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa
  0x613000000200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x613000000280: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x613000000300: 00 00 00 00 00 00 00 00 00 00 00 00 00[fa]fa fa
  0x613000000380: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x613000000400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x613000000480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x613000000500: 00 00 01 fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x613000000580: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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
==530122==ABORTING

PcapPlusPlus versions tested on

PcapPlusPlus master branch

Other PcapPlusPlus version (if applicable)

hash: 0588d88eca769a02e660161f8965ef6152ada90e

Operating systems tested on

Linux

Other operation systems (if applicable)

No response

Compiler version

clang version 16.0.6

Packet capture backend (if applicable)

No response

[tigercosmos]

Thanks @Rulkallos. Feel free to make a patch for this.

[Rulkallos]

Thanks @Rulkallos. Feel free to make a patch for this.

I attempted to investigate this issue a few months ago, but I found that fixing this pcap-related bug would require more specialized expertise than I currently have 😢. While I would be interested in addressing it if possible, I am presently occupied with my own commitments. If time permits in the future, I would be glad to revisit the issue.
Thank you for your understanding, and I apologize for having to set this aside for now.🙏

[seladb]

Thanks @Rulkallos. Feel free to make a patch for this.

I attempted to investigate this issue a few months ago, but I found that fixing this pcap-related bug would require more specialized expertise than I currently have 😢. While I would be interested in addressing it if possible, I am presently occupied with my own commitments. If time permits in the future, I would be glad to revisit the issue. Thank you for your understanding, and I apologize for having to set this aside for now.🙏

No worries at all @Rulkallos ! If you ever have time to work on it please feel free to reach out, and we can help you debug and fix it