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 /PLAN.md | |
| 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 'PLAN.md')
| -rw-r--r-- | PLAN.md | 42 |
1 files changed, 42 insertions, 0 deletions
@@ -685,3 +685,45 @@ None currently open. real intermediate routers - this machine's own gateway, then real ISP infrastructure several hops out - all correctly showing "[this machine -> 8.8.8.8 proto=1]", matching traceroute's own hop output. +- IPv4/IPv6 fragmentation: a real correctness fix, not just added + visibility. Before this, a non-first fragment's payload - pure + continuation data, no TCP/UDP/ICMP header present at all - was + 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 on + Ipv4Header; a nonzero fragment_offset now stops summarize_packet + before it ever reaches transport dispatch, reporting the fragment + itself instead. IPv6 expresses fragmentation as a Fragment + extension header instead of header fields, so the fix lives in + walk_ipv6_extension_headers(): it already walked past Fragment + headers, but never checked the offset before continuing on to + decode whatever followed as if it were a transport header -- + exactly the same bug, just reached through the extension-header + path instead of a header field. Fixed by having the walk stop + immediately (like the existing ESP hard-stop) whenever the offset is + nonzero, and reporting that stop via two new + Ipv6ExtensionWalkResult fields. + Caught and fixed a real bug in this fix while writing it, before it + ever ran: the first version computed a fragment's Identification + field as a loop-local variable, so it was silently discarded the + moment the walk continued past a *first* fragment's header (offset + zero) to keep decoding the real transport protocol underneath -- + every return path after that point reported no fragment id at all, + even though one genuinely applied. Fixed by hoisting it to a + variable that persists across loop iterations, the same category of + mistake (and the same fix) as this session's earlier IGMPv3 bug: + state that needs to survive past the specific branch that computed + it. Fuzzed afterward regardless (fuzz_ipv4, fuzz_ipv6, + fuzz_summarize, ~16.8M combined runs) - clean, no crashes. + Live-verified with genuine IPv4 fragmentation: a 4000-byte ping to + the real local gateway over the actual 1500-MTU wlp1s0 interface, + captured on the wire. Both directions fragmented into exactly 3 + pieces each; the first fragment decoded normally (ICMP Echo + Request/Reply, correct id/seq) with a "(fragmented, ...)" note, and + the two continuation fragments correctly showed only fragment + metadata - no fake ICMP decode attempted on their payload. IPv6 + fragmentation is unit-tested (including the exact byte pattern that + triggered the fragment-id bug above) but not live-verified: this + machine has no real global IPv6 connectivity to generate genuine + IPv6-fragmented traffic against, only link-local addresses. |