Heap-buffer-overflow(read) in TLVRecordReader::getNextTLVRecord() with two positions #1129

bladchan · 2023-04-27T02:50:01Z

Describe the bug
Bad .pcap files which can lead TLVRecordReader::getNextTLVRecord to heap-buffer-overflow (read) issues with two positions: one is in NflogTlv::getType(), the other is in NflogTlv::assign().

Pocs here:

heap_buffer_overflow_pocs.zip

To Reproduce

Build the whole library with ASAN;
Driver program (compile it with ASAN too):

// fuzz.cpp
#include<stdio.h>
#include <Packet.h>
#include <PcapFileDevice.h>

using namespace pcpp;

int
main(int argc, char* argv[])
{
  char *file = argv[1];
  // open a pcap file for reading
  pcpp::PcapFileReaderDevice reader(file);
  if (!reader.open())
  {
    printf("Error opening the pcap file\n");
    return 1;
  }

  // read the first (and only) packet from the file
  pcpp::RawPacket rawPacket;
  if (!reader.getNextPacket(rawPacket))
  {
    printf("Couldn't read the first packet in the file\n");
    return 1;
  }

  while (reader.getNextPacket(rawPacket)) {
      // parse the raw packet into a parsed packet
      pcpp::Packet parsedPacket(&rawPacket);
  }

  // close the file
  reader.close();

  return 0;
}

Run pocs:

$ ./fuzz ./poc_assign
$ ./fuzz ./poc_gettype

Expected behavior
The code snippet where the issue happened should avoid the out-bounds read operation.

Environment (please complete the following information):

System and Version : Ubuntu 20.04.1 + Clang 11
commit version: fb3a560

Additional context
For poc_assign, ASAN says:

$ ./fuzz poc_assign 
=================================================================
==3150137==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x606000004680 at pc 0x7f8fa8b80535 bp 0x7fffc291de50 sp 0x7fffc291de48
READ of size 1 at 0x606000004680 thread T0
    #0 0x7f8fa8b80534 in pcpp::NflogTlv::assign(unsigned char*) /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/NflogLayer.h:117:12
    #1 0x7f8fa8b80534 in pcpp::TLVRecordReader<pcpp::NflogTlv>::getNextTLVRecord(pcpp::NflogTlv&, unsigned char const*, unsigned long) const /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/TLVData.h:296:11
    #2 0x7f8fa8b80534 in pcpp::TLVRecordReader<pcpp::NflogTlv>::getTLVRecord(unsigned int, unsigned char*, unsigned long) const /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/TLVData.h:321:14
    #3 0x7f8fa8b80534 in pcpp::NflogLayer::getTlvByType(pcpp::NflogTlvType) const /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/NflogLayer.cpp:38:29
    #4 0x7f8fa8b80534 in pcpp::NflogLayer::parseNextLayer() /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/NflogLayer.cpp:50:21
    #5 0x7f8fa8b9a6b9 in pcpp::Packet::setRawPacket(pcpp::RawPacket*, bool, unsigned long, pcpp::OsiModelLayer) /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/Packet.cpp:81:13
    #6 0x4ca26b in main /home/ubuntu/test_program/PcapPlusPlus/build/fuzz.cpp:28:20
    #7 0x7f8fa829b082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #8 0x41d41d in _start (/home/ubuntu/test_program/PcapPlusPlus/build/fuzz+0x41d41d)

0x606000004680 is located 0 bytes to the right of 64-byte region [0x606000004640,0x606000004680)
allocated by thread T0 here:
    #0 0x4c723d in operator new[](unsigned long) (/home/ubuntu/test_program/PcapPlusPlus/build/fuzz+0x4c723d)
    #1 0x7f8fa8f31ee4 in pcpp::PcapFileReaderDevice::getNextPacket(pcpp::RawPacket&) /home/ubuntu/test_program/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:307:27
    #2 0x4ca1c2 in main /home/ubuntu/test_program/PcapPlusPlus/build/fuzz.cpp:26:17
    #3 0x7f8fa829b082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/NflogLayer.h:117:12 in pcpp::NflogTlv::assign(unsigned char*)
Shadow bytes around the buggy address:
  0x0c0c7fff8880: 00 00 00 00 00 00 00 fa fa fa fa fa 00 00 00 00
  0x0c0c7fff8890: 00 00 00 fa fa fa fa fa 00 00 00 00 00 00 00 fa
  0x0c0c7fff88a0: fa fa fa fa 00 00 00 00 00 00 00 fa fa fa fa fa
  0x0c0c7fff88b0: 00 00 00 00 00 00 00 fa fa fa fa fa fd fd fd fd
  0x0c0c7fff88c0: fd fd fd fd fa fa fa fa 00 00 00 00 00 00 00 00
=>0x0c0c7fff88d0:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff88e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff88f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8900: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8910: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff8920: 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
==3150137==ABORTING

For poc_gettype, ASAN says:

$ ./fuzz poc_gettype 
=================================================================
==3391523==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60f0000000eb at pc 0x7f052f7fb54f bp 0x7ffc89555ed0 sp 0x7ffc89555ec8
READ of size 2 at 0x60f0000000eb thread T0
    #0 0x7f052f7fb54e in pcpp::NflogTlv::getType() const /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/NflogLayer.h:134:45
    #1 0x7f052f7fb54e in pcpp::TLVRecordReader<pcpp::NflogTlv>::getTLVRecord(unsigned int, unsigned char*, unsigned long) const /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/TLVData.h:316:16
    #2 0x7f052f7fb54e in pcpp::NflogLayer::getTlvByType(pcpp::NflogTlvType) const /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/NflogLayer.cpp:38:29
    #3 0x7f052f7fb54e in pcpp::NflogLayer::parseNextLayer() /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/NflogLayer.cpp:50:21
    #4 0x7f052f8156b9 in pcpp::Packet::setRawPacket(pcpp::RawPacket*, bool, unsigned long, pcpp::OsiModelLayer) /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/Packet.cpp:81:13
    #5 0x4ca26b in main /home/ubuntu/test_program/PcapPlusPlus/build/fuzz.cpp:28:20
    #6 0x7f052ef16082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #7 0x41d41d in _start (/home/ubuntu/test_program/PcapPlusPlus/build/fuzz+0x41d41d)

0x60f0000000eb is located 0 bytes to the right of 171-byte region [0x60f000000040,0x60f0000000eb)
allocated by thread T0 here:
    #0 0x4c723d in operator new[](unsigned long) (/home/ubuntu/test_program/PcapPlusPlus/build/fuzz+0x4c723d)
    #1 0x7f052fbacee4 in pcpp::PcapFileReaderDevice::getNextPacket(pcpp::RawPacket&) /home/ubuntu/test_program/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:307:27
    #2 0x4ca1c2 in main /home/ubuntu/test_program/PcapPlusPlus/build/fuzz.cpp:26:17
    #3 0x7f052ef16082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/NflogLayer.h:134:45 in pcpp::NflogTlv::getType() const
Shadow bytes around the buggy address:
  0x0c1e7fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1e7fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1e7fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1e7fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c1e7fff8000: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
=>0x0c1e7fff8010: 00 00 00 00 00 00 00 00 00 00 00 00 00[03]fa fa
  0x0c1e7fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e7fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e7fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e7fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c1e7fff8060: 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
==3391523==ABORTING

The text was updated successfully, but these errors were encountered:

seladb · 2023-04-27T10:31:20Z

@bladchan thanks for reporting this issue!

@jafar75 this happens in NflogLayer which you worked on, do you think you can take a look?

bladchan · 2023-04-28T01:54:02Z

I found another bad .pcap file which lead to a heap-buffer-overflow (read) issue in NflogLayer with different position. Maybe they have same root cause. I report it here for future improving.

Poc:
poc_getTotalSize.zip

ASAN says:

$ ./fuzz ./poc_getTotalSize 
=================================================================
==1867956==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000014 at pc 0x7f1bb60c74b6 bp 0x7ffd652d8570 sp 0x7ffd652d8568
READ of size 2 at 0x602000000014 thread T0
    #0 0x7f1bb60c74b5 in pcpp::NflogTlv::getTotalSize() const /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/NflogLayer.h:104:48
    #1 0x7f1bb60c74b5 in pcpp::TLVRecordReader<pcpp::NflogTlv>::getFirstTLVRecord(unsigned char*, unsigned long) const /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/TLVData.h:266:34
    #2 0x7f1bb60c74b5 in pcpp::TLVRecordReader<pcpp::NflogTlv>::getTLVRecord(unsigned int, unsigned char*, unsigned long) const /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/TLVData.h:313:27
    #3 0x7f1bb60c74b5 in pcpp::NflogLayer::getTlvByType(pcpp::NflogTlvType) const /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/NflogLayer.cpp:38:29
    #4 0x7f1bb60c74b5 in pcpp::NflogLayer::parseNextLayer() /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/NflogLayer.cpp:50:21
    #5 0x7f1bb60e16b9 in pcpp::Packet::setRawPacket(pcpp::RawPacket*, bool, unsigned long, pcpp::OsiModelLayer) /home/ubuntu/test_program/PcapPlusPlus/Packet++/src/Packet.cpp:81:13
    #6 0x4ca26b in main /home/ubuntu/test_program/PcapPlusPlus/build/fuzz.cpp:28:20
    #7 0x7f1bb57e2082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
    #8 0x41d41d in _start (/home/ubuntu/test_program/PcapPlusPlus/build/fuzz+0x41d41d)

0x602000000015 is located 0 bytes to the right of 5-byte region [0x602000000010,0x602000000015)
allocated by thread T0 here:
    #0 0x4c723d in operator new[](unsigned long) (/home/ubuntu/test_program/PcapPlusPlus/build/fuzz+0x4c723d)
    #1 0x7f1bb6478ee4 in pcpp::PcapFileReaderDevice::getNextPacket(pcpp::RawPacket&) /home/ubuntu/test_program/PcapPlusPlus/Pcap++/src/PcapFileDevice.cpp:307:27
    #2 0x4ca1c2 in main /home/ubuntu/test_program/PcapPlusPlus/build/fuzz.cpp:26:17
    #3 0x7f1bb57e2082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/ubuntu/test_program/PcapPlusPlus/Packet++/header/NflogLayer.h:104:48 in pcpp::NflogTlv::getTotalSize() const
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa[05]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: 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
==1867956==ABORTING

jafar75 · 2023-04-28T23:46:27Z

@seladb sorry for response delay.
Unfortunately, I don't have enough time and concentration to investigate the problem in the next 7 days because of my daily job.

if it helps, I will check it after this interval
regards

seladb · 2023-04-29T08:53:24Z

@jafar75 thanks for your reply! There's no pressure, this issue can wait a week or two until you have some time to look

seladb · 2023-06-08T06:39:19Z

@jafar75 do you have time to work on it now?

jafar75 · 2023-06-09T11:57:05Z

@seladb sorry for delay, I will check it in the next 2 days

jafar75 · 2023-06-11T15:08:10Z

investigating the problem...

seladb · 2023-07-07T08:28:03Z

investigating the problem...

@jafar75 any progress on this?

jafar75 · 2023-07-07T21:06:38Z

@seladb very sorry for delay, after some investigation, I realized that pcap files attached to issue are corrupted maybe. when opening with wireshark, the message "The capture file appears to have been cut short in the middle of a packet."
I was trying to find a way to distinguish such pcap files and showing appropriate err message.

Unfortunately, I won't have enough time to work on issue.
If other friends have time, It will be excellent.

bladchan · 2023-07-08T01:50:18Z

@jafar75 The pcap files attached are generated by fuzzer, so they are corrupted with high probability:). I think you should debug it with debugger or maybe find something. Heap buffer overflow issues usually occur due to unchecked boundary conditions， maybe there're some unchecked bounds in TLVData.h/NflogLayer.h. Hope that helps.

bladchan · 2023-07-08T02:10:03Z

@jafar75 For example:

PcapPlusPlus/Packet++/header/NflogLayer.h

Lines 104 to 122 in e96b741

    
           		size_t getTotalSize() const { return m_Data->recordLen; } 
        
           		/** 
        
           		 * Assign a pointer to the TLV record raw data (byte array) 
        
           		 * @param[in] recordRawData A pointer to the TLV record raw data 
        
           		 */ 
        
           		void assign(uint8_t* recordRawData) 
        
           		{ 
        
           			if(recordRawData == nullptr) 
        
           				m_Data = nullptr; 
        
           			else 
        
           			{ 
        
           				// to pass from some unknown paddings after tlv with type NFULA_PREFIX 
        
           				while (*recordRawData == 0) 
        
           					recordRawData += 1; 
        
           				m_Data = (NflogTLVRawData*)recordRawData; 
        
           			} 
        
           		}

In line 119 m_Data = (NflogTLVRawData*)recordRawData, we assume that recordRawData's real allocated left is less than sizeof NflogTLVRawData(due to corrupted pcap file). Then, when you call getTotalSize in line 104, m_Data->recordLen may point to unallocated memory area, which lead to heap-buffer-overflow. Just for an example, maybe there're more issues in code. I have no idea how to fix these issues more elegantly.🤔

seladb · 2023-08-03T00:19:01Z

The fix by @sashashura was merged to master.

Thanks @bladchan for reporting this issue, and thanks @sashashura for providing the fix! 🙏 🙏

seladb added the fuzzing label Apr 27, 2023

clementperon mentioned this issue May 1, 2023

Heap-buffer-overflow (read) in TcpLayer::isDataValid at TcpLayer.h:500 #1130

Closed

sashashura mentioned this issue Jul 21, 2023

Fix Heap-buffer-overflow (read) in NflogTlv::getType #1163

Merged

seladb closed this as completed Aug 3, 2023

sashashura added a commit to sashashura/PcapPlusPlus that referenced this issue Aug 4, 2023

Fixes poc_assign from seladb#1129

2b02ad3

sashashura mentioned this issue Aug 4, 2023

Fixes poc_assign from #1129 #1175

Closed

sashashura added a commit to sashashura/PcapPlusPlus that referenced this issue Aug 4, 2023

Fixes poc_assign from seladb#1129

0a0ea75

sashashura added a commit to sashashura/PcapPlusPlus that referenced this issue Aug 7, 2023

Fixes poc_assign from seladb#1129

6a4fdaf

sashashura added a commit to sashashura/PcapPlusPlus that referenced this issue Aug 9, 2023

Fixes poc_assign from seladb#1129

79e98b9

sashashura mentioned this issue Aug 9, 2023

Fix Unknown read in pcpp::NflogLayer::parseNextLayer #1178

Merged

sashashura added a commit to sashashura/PcapPlusPlus that referenced this issue Aug 9, 2023

Fixes poc_assign from seladb#1129

1471520

seladb linked a pull request Aug 15, 2023 that will close this issue

Fix Unknown read in pcpp::NflogLayer::parseNextLayer #1178

Merged

seladb pushed a commit that referenced this issue Aug 24, 2023

Fixes poc_assign from #1129 (#1178)

6986e81

fxlb pushed a commit to fxlb/PcapPlusPlus that referenced this issue Oct 22, 2024

Fixes poc_assign from seladb#1129 (seladb#1178)

27d0622

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Heap-buffer-overflow(read) in TLVRecordReader::getNextTLVRecord() with two positions #1129

Heap-buffer-overflow(read) in TLVRecordReader::getNextTLVRecord() with two positions #1129

bladchan commented Apr 27, 2023 •

edited

Loading

seladb commented Apr 27, 2023

bladchan commented Apr 28, 2023

jafar75 commented Apr 28, 2023

seladb commented Apr 29, 2023

seladb commented Jun 8, 2023

jafar75 commented Jun 9, 2023

jafar75 commented Jun 11, 2023

seladb commented Jul 7, 2023

jafar75 commented Jul 7, 2023

bladchan commented Jul 8, 2023 •

edited

Loading

bladchan commented Jul 8, 2023 •

edited

Loading

seladb commented Aug 3, 2023

Heap-buffer-overflow(read) in TLVRecordReader::getNextTLVRecord() with two positions #1129

Heap-buffer-overflow(read) in TLVRecordReader::getNextTLVRecord() with two positions #1129

Comments

bladchan commented Apr 27, 2023 • edited Loading

seladb commented Apr 27, 2023

bladchan commented Apr 28, 2023

jafar75 commented Apr 28, 2023

seladb commented Apr 29, 2023

seladb commented Jun 8, 2023

jafar75 commented Jun 9, 2023

jafar75 commented Jun 11, 2023

seladb commented Jul 7, 2023

jafar75 commented Jul 7, 2023

bladchan commented Jul 8, 2023 • edited Loading

bladchan commented Jul 8, 2023 • edited Loading

seladb commented Aug 3, 2023

bladchan commented Apr 27, 2023 •

edited

Loading

bladchan commented Jul 8, 2023 •

edited

Loading

bladchan commented Jul 8, 2023 •

edited

Loading