PcapPlusPlus Heap buffer overflow (read) in DnsResource::getDataLength() at DnsResource.cpp:277:24

There is a buffer overflow at the specified location:

==4123050==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x607000005a93 at pc 0x000000681f70 bp 0x7ffcb25feaa0 sp 0x7ffcb25fea98
READ of size 2 at 0x607000005a93 thread T0
    #0 0x681f6f in pcpp::DnsResource::getDataLength() const /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/src/DnsResource.cpp:277:24
    #1 0x5ed78f in pcpp::DnsResource::getSize() const /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/./header/DnsResource.h:230:100
    #2 0x5dd552 in pcpp::DnsLayer::parseResources() /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/src/DnsLayer.cpp:198:35
    #3 0x5dbaff in pcpp::DnsLayer::init(unsigned long, bool) /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/src/DnsLayer.cpp:86:3
    #4 0x5db7ef in pcpp::DnsLayer::DnsLayer(unsigned char*, unsigned long, pcpp::Layer*, pcpp::Packet*) /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/src/DnsLayer.cpp:22:2
    #5 0x57138d in pcpp::UdpLayer::parseNextLayer() /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/src/UdpLayer.cpp:117:21
    #6 0x54a40b in pcpp::Packet::setRawPacket(pcpp::RawPacket*, bool, unsigned long, pcpp::OsiModelLayer) /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/src/Packet.cpp:80:13
    #7 0x54d82b in pcpp::Packet::Packet(pcpp::RawPacket*, bool, unsigned long, pcpp::OsiModelLayer) /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/src/Packet.cpp:126:2
    #8 0x4f33c3 in main /home/vivin/Projects/phd/resources/archives/pcapplusplus/readpcap.cpp:30:20
    #9 0x7fd6a51a90b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #10 0x4441bd in _start (/home/vivin/Projects/phd/workspace/smartdsf/pcapplusplus/binaries/vanilla-asan/readpcap+0x4441bd)

0x607000005a93 is located 8 bytes to the right of 75-byte region [0x607000005a40,0x607000005a8b)
allocated by thread T0 here:
    #0 0x4f05ed in operator new[](unsigned long) (/home/vivin/Projects/phd/workspace/smartdsf/pcapplusplus/binaries/vanilla-asan/readpcap+0x4f05ed)
    #1 0x4f942d in pcpp::PcapFileReaderDevice::getNextPacket(pcpp::RawPacket&) /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Pcap++/src/PcapFileDevice.cpp:306:27
    #2 0x4f3328 in main /home/vivin/Projects/phd/resources/archives/pcapplusplus/readpcap.cpp:28:17
    #3 0x7fd6a51a90b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/vivin/Projects/phd/subjects/pcapplusplus/PcapPlusPlus-master/Packet++/src/DnsResource.cpp:277:24 in pcpp::DnsResource::getDataLength() const
Shadow bytes around the buggy address:
  0x0c0e7fff8b00: fa fa 00 00 00 00 00 00 00 00 00 fa fa fa fa fa
  0x0c0e7fff8b10: fd fd fd fd fd fd fd fd fd fd fa fa fa fa fd fd
  0x0c0e7fff8b20: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
  0x0c0e7fff8b30: fd fd fd fd fd fd fa fa fa fa fd fd fd fd fd fd
  0x0c0e7fff8b40: fd fd fd fd fa fa fa fa 00 00 00 00 00 00 00 00
=>0x0c0e7fff8b50: 00 03[fa]fa fa fa 00 00 00 00 00 00 00 00 00 00
  0x0c0e7fff8b60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0e7fff8b70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0e7fff8b80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0e7fff8b90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0e7fff8ba0: 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
==4123050==ABORTING
Aborted

I have attached the input that triggers this vulnerability.

id:000030,sig:06,src:000033+001376,op:splice,rep:32.pcap.zip

Mar 30 '22 16:03 vivin

Thanks @vivin for reporting this issue! Would you consider opening PRs with fixes to this and the other 3 issues you found?

Mar 31 '22 05:03 seladb

@seladb I can try -- but my C++ skills are so-so, my knowledge of network packets nonexistent, and I am also busy with some research (I discovered these bugs as part of that) :D.

Apr 02 '22 08:04 vivin

@vivin feel free to give it a shot! Given that you managed to find these issues using a fuzzer I'm sure you'll be able to fix them 😃

Apr 06 '22 06:04 seladb

I was fuzzing PcapPlusPlus yesterday and found the same bug in version #22269e8. By debugging it, I found that DnsResource::getDataLength() in Packet++/DnsResource.cpp didn't check the boundary of m_DnsLayer->m_Data. Therefore, It is possible to trigger an out-of-bounds read bug.

size_t DnsResource::getDataLength() const
{
        // may out-of-bounds read here
	uint16_t dataLength = *(uint16_t*)(getRawData() + m_NameLength + 2*sizeof(uint16_t) + sizeof(uint32_t)); 
	return be16toh(dataLength);
}

It seems that adding a boundary check before reading m_Data can fix this issue. Moreover, I have test testcases created by me and the testcase on this page and there is no more buffer overflow detected.

😃 I will PR this patch later.

Sep 06 '22 02:09 bladchan

Thank you @bladchan for your contribution to this project, much appreciated! 🙏 ❤️

Your PR was merged to master, I'm closing this issue now

Sep 07 '22 07:09 seladb