Fix vuln OSV-2024-343

[aled-ua]

[Warning] This PR is generated by AI

Pull Request Description for Fixing Vulnerability - OSV-2024-343

---

1. PR Title: Fix for Heap Buffer Overflow in pcapplusplus - OSV-2024-343

---

2. PR Description:
   • Bug Type: Heap-buffer-overflow
   • Summary: A vulnerability was identified in the pcapplusplus project where a heap-buffer-overflow occurred due to an out-of-bounds memory access in the BgpUpdateMessageLayer class. This issue was detected in the function getPathAttributesLength() which is part of BgpLayer.cpp. The vulnerability allows memory access outside of allocated bounds, potentially leading to undefined behavior or program crashes.
   • Fix Summary: The patch introduces a bounds checking mechanism in the getPathAttributesLength() function to ensure that the memory access remains within the allocated buffer's limits. Specifically, the fix checks the calculated offset against the total buffer size (headerLen) and returns 0 if the access is invalid. This modification effectively prevents the heap-buffer-overflow by ensuring all accesses are safe and within bounds.
   • Security and Stability Improvement: The fix enhances the program's security by eliminating the potential for out-of-bounds memory access, thereby preventing crashes and undefined behavior. It also improves the overall stability of the pcapplusplus program by ensuring robust handling of invalid memory access scenarios.

---

3. Sanitizer Report Summary:
   The sanitizer detected a heap-buffer-overflow error in the function getPathAttributesLength() located in BgpLayer.cpp:546. The issue was caused by accessing heap memory beyond its allocated size, and the error was triggered during fuzz testing. The stack trace indicated that the vulnerability propagates through the setPathAttributes and clearPathAttributes functions, leading to an invalid memory access in the readParsedPacket function during fuzzing.

---

4. Full Sanitizer Report:

   ==31822==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x514000009d05 at pc 0x555635a9897d bp 0x7ffd6df7c600 sp 0x7ffd6df7c5f8
   READ of size 2 at 0x514000009d05 thread T0
       #0 0x555635a9897c in pcpp::BgpUpdateMessageLayer::getPathAttributesLength() const /root/Packet++/src/BgpLayer.cpp:546:8
       #1 0x555635a99800 in pcpp::BgpUpdateMessageLayer::setPathAttributes(std::vector<pcpp::BgpUpdateMessageLayer::path_attribute, std::allocator<pcpp::BgpUpdateMessageLayer::path_attribute>> const&) /root/Packet++/src/BgpLayer.cpp:638:37
       #2 0x555635a99e76 in pcpp::BgpUpdateMessageLayer::clearPathAttributes() /root/Packet++/src/BgpLayer.cpp:680:10
       #3 0x555635a54532 in readParsedPacket(pcpp::Packet, pcpp::Layer*) /root/Tests/Fuzzers/ReadParsedPacket.h:471:24
       #4 0x555635a4de82 in LLVMFuzzerTestOneInput /root/Tests/Fuzzers/FuzzTarget.cpp:66:5
       #5 0x5556359586d4 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/root/out/FuzzTarget+0xd66d4) (BuildId: 4504d0c118576c250bcce97cc74a518cc1645217)
       #6 0x555635941806 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/root/out/FuzzTarget+0xbf806) (BuildId: 4504d0c118576c250bcce97cc74a518cc1645217)
       #7 0x5556359472ba in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/root/out/FuzzTarget+0xc52ba) (BuildId: 4504d0c118576c250bcce97cc74a518cc1645217)
       #8 0x555635971a76 in main (/root/out/FuzzTarget+0xefa76) (BuildId: 4504d0c118576c250bcce97cc74a518cc1645217)
       #9 0x7f74ef4ad1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
       #10 0x7f74ef4ad28a in __libc_start_main csu/../csu/libc-start.c:360:3
       #11 0x55563593c3d4 in _start (/root/out/FuzzTarget+0xba3d4) (BuildId: 4504d0c118576c250bcce97cc74a518cc1645217)
   
   Address 0x514000009d05 is a wild pointer inside of access range of size 0x000000000002.
   SUMMARY: AddressSanitizer
   

---

5. Files Modified:
   The patch modifies the following file:

   • Packet++/src/BgpLayer.cpp

   --- a/Packet++/src/BgpLayer.cpp
   +++ b/Packet++/src/BgpLayer.cpp
   @@ -543,6 +543,11 @@
   		if (headerLen >= minLen)
   		{
   			size_t withdrawnRouteLen = getWithdrawnRoutesLength();
   +			// Ensure the memory access is within bounds
   +			if (sizeof(bgp_common_header) + sizeof(uint16_t) + withdrawnRouteLen + sizeof(uint16_t) > headerLen)
   +			{
   +				return 0;  // Invalid access, return 0
   +			}
   			uint16_t res =
   			    be16toh(*(uint16_t*)(m_Data + sizeof(bgp_common_header) + sizeof(uint16_t) + withdrawnRouteLen));
   			if ((size_t)res > headerLen - minLen - withdrawnRouteLen)

---

6. Patch Validation:
   The patch has been validated using the provided PoC and fuzzing tools. The heap-buffer-overflow issue reported in the sanitizer report has been resolved. Additionally, no new issues or regressions were introduced during the testing process.

---

7. Links:
   • Original Vulnerability Report
   • Proof of Concept (PoC)

---

Please review and merge the patch to ensure that the program is secure and stable. Thank you!

[codecov]

Codecov Report

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

Project coverage is 83.15%. Comparing base (f81ced2) to head (a1304e5).
Report is 14 commits behind head on dev.

Files with missing lines	Patch %	Lines
Packet++/src/BgpLayer.cpp	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1680      +/-   ##
==========================================
- Coverage   83.16%   83.15%   -0.01%     
==========================================
  Files         277      277              
  Lines       48193    48203      +10     
  Branches     9966     9948      -18     
==========================================
+ Hits        40081    40085       +4     
- Misses       7234     7268      +34     
+ Partials      878      850      -28     

Flag	Coverage Δ
alpine320	75.15% <0.00%> (-0.01%)	⬇️
fedora40	75.19% <0.00%> (-0.01%)	⬇️
macos-13	80.65% <0.00%> (-0.01%)	⬇️
macos-14	80.65% <0.00%> (-0.01%)	⬇️
macos-15	80.62% <0.00%> (-0.01%)	⬇️
mingw32	70.87% <0.00%> (-0.05%)	⬇️
mingw64	70.83% <0.00%> (-0.05%)	⬇️
npcap	85.30% <50.00%> (-0.02%)	⬇️
rhel94	75.02% <0.00%> (-0.01%)	⬇️
ubuntu2004	58.60% <0.00%> (-0.03%)	⬇️
ubuntu2004-zstd	58.72% <0.00%> (-0.03%)	⬇️
ubuntu2204	74.97% <0.00%> (-0.02%)	⬇️
ubuntu2204-icpx	61.43% <0.00%> (-0.01%)	⬇️
ubuntu2404	75.22% <0.00%> (+0.02%)	⬆️
unittest	83.15% <50.00%> (-0.01%)	⬇️
windows-2019	85.33% <50.00%> (-0.02%)	⬇️
windows-2022	85.36% <50.00%> (-0.02%)	⬇️
winpcap	85.32% <50.00%> (-0.01%)	⬇️
xdp	50.53% <0.00%> (-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.

[Dimi1010]

LGTM

[seladb]

Thank you @aled-ua for working on this fix, much appreciated! 🙏