Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework processing of ASN.1record length (unsized types, strict checks) #1616

Merged
merged 3 commits into from
Nov 8, 2024

Conversation

1ndahous3
Copy link
Contributor

@1ndahous3 1ndahous3 commented Oct 20, 2024

fix checks of ASN.1record length (got segfault on invalid data, added in fuzzing regression testcase).

File summary:
~~~~~~~~~~~~~
   File name: /home/user/crash-invalid-asn-record-len
   File size: 305 bytes
   Link layer type: Ethernet

==2300384== Invalid read of size 1
==2300384==    at 0x1C53F3: pcpp::Asn1Record::decodeTagAndCreateRecord(unsigned char const*, unsigned long, int&) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x1C57AC: pcpp::Asn1Record::decodeInternal(unsigned char const*, unsigned long, bool) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x1C58E1: pcpp::Asn1ConstructedRecord::decodeValue(unsigned char*, bool) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x1C4FC7: pcpp::Asn1Record::decodeValueIfNeeded() (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x1A0B77: pcpp::LdapLayer::parseLdapMessage(unsigned char*, unsigned long, pcpp::Layer*, pcpp::Packet*) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x169CBB: pcpp::TcpLayer::parseNextLayer() (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x15368E: pcpp::Packet::setRawPacket(pcpp::RawPacket*, bool, unsigned int, pcpp::OsiModelLayer) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x13C4E1: printPcapPackets(pcpp::IFileReaderDevice*, std::ostream*, int) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x12AC6F: main (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==  Address 0xfffffffcfcd0cbed is not stack'd, malloc'd or (recently) free'd
==2300384== 
==2300384== 
==2300384== Process terminating with default action of signal 11 (SIGSEGV)
==2300384==  Access not within mapped region at address 0xFFFFFFFCFCD0CBED
==2300384==    at 0x1C53F3: pcpp::Asn1Record::decodeTagAndCreateRecord(unsigned char const*, unsigned long, int&) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x1C57AC: pcpp::Asn1Record::decodeInternal(unsigned char const*, unsigned long, bool) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x1C58E1: pcpp::Asn1ConstructedRecord::decodeValue(unsigned char*, bool) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x1C4FC7: pcpp::Asn1Record::decodeValueIfNeeded() (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x1A0B77: pcpp::LdapLayer::parseLdapMessage(unsigned char*, unsigned long, pcpp::Layer*, pcpp::Packet*) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x169CBB: pcpp::TcpLayer::parseNextLayer() (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x15368E: pcpp::Packet::setRawPacket(pcpp::RawPacket*, bool, unsigned int, pcpp::OsiModelLayer) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x13C4E1: printPcapPackets(pcpp::IFileReaderDevice*, std::ostream*, int) (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)
==2300384==    by 0x12AC6F: main (in /home/user/git/PcapPlusPlus/build/examples_bin/PcapPrinter)

@1ndahous3 1ndahous3 changed the base branch from master to dev October 20, 2024 11:32
@seladb
Copy link
Owner

seladb commented Oct 23, 2024

@1ndahous3 can you please fix the clang-format issues?

Copy link

codecov bot commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.13%. Comparing base (d3adb80) to head (f79106a).
Report is 9 commits behind head on dev.

Files with missing lines Patch % Lines
Packet++/src/Asn1Codec.cpp 94.73% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1616      +/-   ##
==========================================
- Coverage   83.15%   83.13%   -0.02%     
==========================================
  Files         276      276              
  Lines       47296    47318      +22     
  Branches     9710     9536     -174     
==========================================
+ Hits        39328    39339      +11     
- Misses       7081     7108      +27     
+ Partials      887      871      -16     
Flag Coverage Δ
fedora40 75.23% <73.33%> (-0.04%) ⬇️
macos-13 80.67% <84.21%> (-0.02%) ⬇️
macos-14 80.67% <84.21%> (-0.02%) ⬇️
macos-15 80.64% <84.21%> (-0.02%) ⬇️
mingw32 70.79% <73.33%> (-0.06%) ⬇️
mingw64 70.76% <73.33%> (-0.04%) ⬇️
npcap 85.18% <92.85%> (-0.03%) ⬇️
rhel94 75.09% <73.33%> (-0.03%) ⬇️
ubuntu2004 58.66% <73.33%> (-0.02%) ⬇️
ubuntu2004-zstd 58.80% <73.33%> (+<0.01%) ⬆️
ubuntu2204 74.98% <73.33%> (-0.03%) ⬇️
ubuntu2204-icpx 61.51% <84.21%> (+0.03%) ⬆️
ubuntu2404 75.28% <73.33%> (-0.02%) ⬇️
unittest 83.13% <94.73%> (-0.02%) ⬇️
windows-2019 85.22% <92.85%> (-0.02%) ⬇️
windows-2022 85.24% <92.85%> (-0.02%) ⬇️
winpcap 85.21% <92.85%> (-0.01%) ⬇️
xdp 50.21% <73.33%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@seladb seladb changed the title rework processing of ASN.1record length (unsized types, strict checks) Rework processing of ASN.1record length (unsized types, strict checks) Nov 8, 2024
@seladb seladb merged commit 0fc73bf into seladb:dev Nov 8, 2024
41 checks passed
@seladb
Copy link
Owner

seladb commented Nov 8, 2024

Thanks for working on this @1ndahous3 , much appreciated! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants