[Bug] Heap-Buffer-Overflow in `BgpOpenMessageLayer::getOptionalParameters()`

[Rulkallos]

Bug description

Summary

pocs.tar.gz
asan_reports.tar.gz

Multiple heap-buffer-overflow errors were detected in the function pcpp::BgpOpenMessageLayer::getOptionalParameters within BgpLayer.cpp. The overflows occur when parsing optional parameters in BGP OPEN messages, likely due to insufficient validation of buffer boundaries, especially when handling malformed or truncated packets.

Root Cause Analysis

Suspected Problematic Code

void BgpOpenMessageLayer::getOptionalParameters(std::vector<optional_parameter>& optionalParameters)
{
    bgp_open_message* msgHdr = getOpenMsgHeader();
    if (msgHdr == nullptr || msgHdr->optionalParameterLength == 0)
        return;

    size_t optionalParamsLen = (size_t)be16toh(msgHdr->optionalParameterLength);

    if (optionalParamsLen > getHeaderLen() - sizeof(bgp_open_message))
        optionalParamsLen = getHeaderLen() - sizeof(bgp_open_message);

    uint8_t* dataPtr = m_Data + sizeof(bgp_open_message);
    size_t byteCount = 0;
    while (byteCount < optionalParamsLen)
    {
        optional_parameter op;
        op.type = dataPtr[0];
        op.length = dataPtr[1];

        if (op.length > optionalParamsLen - byteCount)
        {
            PCPP_LOG_ERROR("Optional parameter length is out of bounds: " << (int)op.length);
            break;
        }

        if (op.length > 0)
            memcpy(op.value, dataPtr + 2 * sizeof(uint8_t), (op.length > 32 ? 32 : op.length));

        optionalParameters.push_back(op);
        size_t totalLen = 2 + (size_t)op.length;
        byteCount += totalLen;
        dataPtr += totalLen;
    }
}

Possible Explanation

• The code checks for optionalParamsLen > getHeaderLen() - sizeof(bgp_open_message) but does not fully ensure that each iteration's dataPtr access stays within the bounds of the buffer.
• In particular, the accesses to dataPtr[0], dataPtr[1], and especially memcpy(op.value, dataPtr + 2, ...) can read out of bounds if the buffer is malformed (i.e., not enough bytes left).
• When optionalParamsLen or the input buffer is too short, dataPtr + totalLen can easily move past the end of the buffer, especially if op.length is corrupted or maliciously large.

Suggested Fix

• Before reading each optional parameter, validate that at least 2 bytes are left in the buffer for type and length.
• After reading op.length, validate that (size_t)op.length bytes are available for value, i.e., byteCount + 2 + op.length <= optionalParamsLen and does not exceed buffer limits.
• If not enough bytes remain for a full parameter, abort parsing to avoid buffer overrun.

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

=================================================================
==529988==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x608000003280 at pc 0x5bc6a70a0f0d bp 0x7fff294b3310 sp 0x7fff294b3308
READ of size 1 at 0x608000003280 thread T0
    #0 0x5bc6a70a0f0c in pcpp::BgpOpenMessageLayer::getOptionalParameters(std::vector<pcpp::BgpOpenMessageLayer::optional_parameter, std::allocator<pcpp::BgpOpenMessageLayer::optional_parameter>>&) /src/PcapPlusPlus/Packet++/src/BgpLayer.cpp:210:36
    #1 0x5bc6a704fd68 in readParsedPacket(pcpp::Packet, pcpp::Layer*) /src/PcapPlusPlus/Tests/Fuzzers/ReadParsedPacket.h:431:22
    #2 0x5bc6a704bb50 in LLVMFuzzerTestOneInput /src/PcapPlusPlus/Tests/Fuzzers/FuzzTarget.cpp:66:5
    #3 0x5bc6a6f5ae90 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) crtstuff.c
    #4 0x5bc6a6f44d6f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) crtstuff.c
    #5 0x5bc6a6f4a826 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) crtstuff.c
    #6 0x5bc6a6f73cf2 in main (/out/FuzzTargetNg+0x1a7cf2) (BuildId: 97bfbeafd983e755)
    #7 0x74b421f7cd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #8 0x74b421f7ce3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #9 0x5bc6a6f3fc74 in _start (/out/FuzzTargetNg+0x173c74) (BuildId: 97bfbeafd983e755)

0x608000003280 is located 5 bytes after 91-byte region [0x608000003220,0x60800000327b)
allocated by thread T0 here:
    #0 0x5bc6a704850d in operator new[](unsigned long) (/out/FuzzTargetNg+0x27c50d) (BuildId: 97bfbeafd983e755)
    #1 0x5bc6a70634f4 in pcpp::PcapNgFileReaderDevice::getNextPacket(pcpp::RawPacket&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>&) /src/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:437:27
    #2 0x5bc6a70651ae in pcpp::PcapNgFileReaderDevice::getNextPacket(pcpp::RawPacket&) /src/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:462:10
    #3 0x5bc6a704bc36 in LLVMFuzzerTestOneInput /src/PcapPlusPlus/Tests/Fuzzers/FuzzTarget.cpp:71:19
    #4 0x5bc6a6f5ae90 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) crtstuff.c
    #5 0x5bc6a6f44d6f in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) crtstuff.c
    #6 0x5bc6a6f4a826 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) crtstuff.c
    #7 0x5bc6a6f73cf2 in main (/out/FuzzTargetNg+0x1a7cf2) (BuildId: 97bfbeafd983e755)
    #8 0x74b421f7cd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/PcapPlusPlus/Packet++/src/BgpLayer.cpp:210:36 in pcpp::BgpOpenMessageLayer::getOptionalParameters(std::vector<pcpp::BgpOpenMessageLayer::optional_parameter, std::allocator<pcpp::BgpOpenMessageLayer::optional_parameter>>&)
Shadow bytes around the buggy address:
  0x608000003000: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x608000003080: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x608000003100: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x608000003180: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fa
  0x608000003200: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 03
=>0x608000003280:[fa]fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00
  0x608000003300: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 fa
  0x608000003380: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x608000003400: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x608000003480: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd fd
  0x608000003500: fa fa fa fa fd fd fd fd fd fd fd fd fd fd fd 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
==529988==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