diff options
| author | srdusr <[email protected]> | 2026-05-29 22:56:00 +0200 |
|---|---|---|
| committer | srdusr <[email protected]> | 2026-05-29 22:56:00 +0200 |
| commit | 719b7f439c1c8c76d0573b34daa329061c9ad8a6 (patch) | |
| tree | d6a1ad5546332859a717bdadbfd72ff4041e18f3 /tests | |
| parent | e41dade9ac3bbd7ffbc1eec826801af1a38d8b9e (diff) | |
| download | packeteer-719b7f439c1c8c76d0573b34daa329061c9ad8a6.tar.gz packeteer-719b7f439c1c8c76d0573b34daa329061c9ad8a6.zip | |
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.
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_ipv6.cpp | 37 | ||||
| -rw-r--r-- | tests/test_net.cpp | 31 | ||||
| -rw-r--r-- | tests/test_summarize.cpp | 28 |
3 files changed, 96 insertions, 0 deletions
diff --git a/tests/test_ipv6.cpp b/tests/test_ipv6.cpp index 1cce85b..2d75e9a 100644 --- a/tests/test_ipv6.cpp +++ b/tests/test_ipv6.cpp @@ -129,6 +129,43 @@ TEST_CASE("walk_ipv6_extension_headers walks the fixed-size Fragment header") { CHECK(result.payload[0] == 0xCA); } +TEST_CASE("walk_ipv6_extension_headers reports the fragment id even for the first fragment") { + // Same shape as the test above (fragment offset 0 - the first + // fragment), but this time checking that the walk continues on to + // TCP *and* still surfaces the Identification field, which a + // caller needs to correlate this with the fragments that follow. + std::vector<unsigned char> payload = {static_cast<unsigned char>(kProtoTcp), 0x00, + 0x00, 0x00, 0x00, 0x00, 0x30, 0x39, // id = 0x3039 + 0xCA, 0xFE}; + auto result = walk_ipv6_extension_headers(kNextHeaderFragment, payload); + CHECK_FALSE(result.is_non_first_fragment); + REQUIRE(result.fragment_id.has_value()); + CHECK(*result.fragment_id == 0x3039); + CHECK(result.final_next_header == kProtoTcp); + REQUIRE(result.payload.size() == 2); // walk continued past the fragment header to real payload +} + +TEST_CASE("walk_ipv6_extension_headers stops at a non-first fragment rather than walking into " + "continuation data") { + // Fragment offset field (13 bits, packed into the top of bytes[2:3]) + // set to a nonzero value - 8 in units of 8 bytes, i.e. byte offset + // 64 into the original datagram. offset_res_m = 8 << 3 = 0x0040. + std::vector<unsigned char> payload = { + static_cast<unsigned char>(kProtoTcp), 0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x2A, + 0xDE, 0xAD, 0xBE, 0xEF, // pure continuation data - NOT a TCP header + }; + auto result = walk_ipv6_extension_headers(kNextHeaderFragment, payload); + CHECK(result.is_non_first_fragment); + REQUIRE(result.fragment_id.has_value()); + CHECK(*result.fragment_id == 0x2A); + // final_next_header still names TCP (that's what the reassembled + // datagram eventually is), but the payload past it is untouched + // continuation data - callers must not decode it as TCP. + CHECK(result.final_next_header == kProtoTcp); + REQUIRE(result.payload.size() == 4); + CHECK(result.payload[0] == 0xDE); +} + TEST_CASE("walk_ipv6_extension_headers applies AH's 4-byte-unit length formula") { // AH: next_header(1)=TCP, payload_len(1)=1 -> total len (1+2)*4=12 bytes. std::vector<unsigned char> payload(12, 0); diff --git a/tests/test_net.cpp b/tests/test_net.cpp index 2702758..64db07f 100644 --- a/tests/test_net.cpp +++ b/tests/test_net.cpp @@ -134,6 +134,37 @@ TEST_CASE("parse_ipv4 honors IHL > 5 (options present)") { CHECK(ip->payload.empty()); } +TEST_CASE("parse_ipv4 decodes an unfragmented packet with fragment_offset zero") { + std::vector<unsigned char> bytes(20, 0); + bytes[0] = 0x45; + auto ip = parse_ipv4(bytes); + REQUIRE(ip.has_value()); + CHECK_FALSE(ip->header.more_fragments); + CHECK(ip->header.fragment_offset == 0); +} + +TEST_CASE("parse_ipv4 decodes the identification field and More Fragments flag") { + std::vector<unsigned char> bytes(20, 0); + bytes[0] = 0x45; + bytes[4] = 0x12; bytes[5] = 0x34; // identification = 0x1234 + bytes[6] = 0x20; bytes[7] = 0x00; // flags: MF=1, fragment_offset=0 (first fragment) + auto ip = parse_ipv4(bytes); + REQUIRE(ip.has_value()); + CHECK(ip->header.identification == 0x1234); + CHECK(ip->header.more_fragments); + CHECK(ip->header.fragment_offset == 0); +} + +TEST_CASE("parse_ipv4 decodes a nonzero fragment_offset in 8-byte units") { + std::vector<unsigned char> bytes(20, 0); + bytes[0] = 0x45; + bytes[6] = 0x00; bytes[7] = 0x08; // fragment_offset = 8 (i.e. byte offset 64), MF=0 (last fragment) + auto ip = parse_ipv4(bytes); + REQUIRE(ip.has_value()); + CHECK_FALSE(ip->header.more_fragments); + CHECK(ip->header.fragment_offset == 8); +} + TEST_CASE("parse_tcp decodes header fields and flags") { std::vector<unsigned char> bytes(20, 0); bytes[0] = 0x00; bytes[1] = 0x50; // src port 80 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<unsigned char> fake_continuation_data = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01, 0x02, 0x03}; + + std::vector<unsigned char> 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<unsigned char> eth = { + 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x08, 0x00, + }; + + std::vector<unsigned char> 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<unsigned char> rtcp = {0x80, 0xC9, 0x00, 0x01, 0, 0, 0, 0}; // RR, len=1 -> 8 bytes |