From 719b7f439c1c8c76d0573b34daa329061c9ad8a6 Mon Sep 17 00:00:00 2001 From: srdusr <99972264+srdusr@users.noreply.github.com> Date: Fri, 29 May 2026 22:56:00 +0200 Subject: Fix IPv4/IPv6 fragment continuation data being decoded as fake transport headers A real correctness bug, not just missing visibility: a non-first fragment's payload is pure continuation data with no TCP/UDP/ICMP header in it at all, but it was being handed to the transport parsers unconditionally, which could misread arbitrary payload bytes as port numbers, sequence numbers, etc. and print a plausible-looking but entirely fake decode. IPv4 gained identification/more_fragments/fragment_offset fields; a nonzero offset now stops summarize_packet before transport dispatch, reporting the fragment instead. IPv6 expresses fragmentation as an extension header instead, so the fix lives in walk_ipv6_extension_headers(): it already walked past Fragment headers, but never checked the offset before continuing on as if a transport header followed - the same bug, reached through a different path. Fixed with an ESP-style hard stop on a nonzero offset. Caught and fixed a second bug while writing the first fix, before it ever ran: the fragment's Identification field was a loop-local variable, discarded the moment the walk continued past a *first* fragment (offset zero) to keep decoding the real payload underneath -- every later return reported no fragment id even though one applied. Fixed by hoisting it to a variable that persists across iterations, the same category of mistake as an earlier IGMPv3 bug. Fuzzed afterward regardless (fuzz_ipv4, fuzz_ipv6, fuzz_summarize, ~16.8M combined runs) - clean. Live-verified with a real 4000-byte ping to the local gateway over the actual 1500-MTU interface: both directions fragmented into 3 pieces each, the first decoded normally with a fragmentation note, and the continuation fragments correctly showed only fragment metadata, no fake ICMP decode attempted. --- tests/test_summarize.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'tests/test_summarize.cpp') diff --git a/tests/test_summarize.cpp b/tests/test_summarize.cpp index f180f87..baf4ed4 100644 --- a/tests/test_summarize.cpp +++ b/tests/test_summarize.cpp @@ -186,6 +186,34 @@ TEST_CASE("summarize_packet decodes an ARP request end to end") { "ARP who-has 10.0.0.2 tell 10.0.0.1 (aa:bb:cc:dd:ee:ff)"); } +TEST_CASE("summarize_packet reports a non-first IPv4 fragment without decoding fake TCP/UDP") { + // Payload here is arbitrary bytes - if this were mistakenly + // handed to a transport parser it would produce a plausible- + // looking but entirely fake TCP/UDP line. The point of this test + // is that it must not. + std::vector fake_continuation_data = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01, 0x02, 0x03}; + + std::vector ip(20, 0); + ip[0] = 0x45; + ip[4] = 0x00; ip[5] = 0x7B; // identification = 123 + ip[6] = 0x00; ip[7] = 0x08; // fragment_offset = 8 (byte offset 64), MF=0 + ip[9] = packeteer::net::kProtoTcp; + ip[12] = 10; ip[13] = 0; ip[14] = 0; ip[15] = 1; + ip[16] = 10; ip[17] = 0; ip[18] = 0; ip[19] = 2; + + std::vector eth = { + 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x08, 0x00, + }; + + std::vector frame = eth; + frame.insert(frame.end(), ip.begin(), ip.end()); + frame.insert(frame.end(), fake_continuation_data.begin(), fake_continuation_data.end()); + + auto line = packeteer::summarize_packet(frame, DLT_EN10MB); + CHECK(line.find("fragment id=123 offset=64") != std::string::npos); + CHECK(line.find("TCP") == std::string::npos); // must not have decoded the fake continuation data +} + TEST_CASE("summarize_packet falls back to the RTCP heuristic on an unmatched UDP port") { std::vector rtcp = {0x80, 0xC9, 0x00, 0x01, 0, 0, 0, 0}; // RR, len=1 -> 8 bytes -- cgit v1.2.3