Repository navigation
fix(protocols): a declared length running past the data is accepted and zero-filled on rebuild #1325
Description
Describe the bug
Nothing checks that an option or parameter's declared length fits the remaining data; the missing octets are read as zeros. Root cause not established (parse is not rejected, so the reader/OptionField path is suspected).
To Reproduce
- HIP ACK
01c10100 00000000after a 40-octet header (declared length 0x0100, 4 present): parses with a 64-entryupdate_id;from_datarebuilds 304 octets with Header Length0x25 - MH BRR
3b010000 00000000 010400000000 0103(PadN declaring 3, none present): parses withPadOption length=5; rebuilds 24 octets (16 in)
Expected behavior
A truncated record is rejected or kept as-is, not padded with invented octets. Compare #1170.
Found by the internet-v4 round-trip audit (#1202); reproduced on main at 11ae68ebf (repro check D15).
Found by the internet-v4 round-trip audit (#1202). The repro was re-checked by an independent verifier, and I re-ran it on main at 11ae68ebf.
Activity
JarryShaw commentedon Oct 7, 2026
Decision needed. The root cause is found, and the fix conflicts with behaviour that is pinned on purpose.
Cause.
OptionField.unpack(corekit/fields/collections.py, about:445) hands each option schema the whole option area, unbounded. A short read is then padded byFieldBase.unpack(field.py:586,ljust(length, b'\x00')).Schema.unpackonly warns (schema.py:1007).Probed on
13aa677bewith a last option that runs past the data:MSS/SACKand SCTP already raise.The strict fix exists on branch
fix/1325-declared-length-overrun(50dd50f14, no PR yet). An option that reads short raisesProtocolError. Inside a parent layer, the octets are kept as captured, as #1171 does: IPv6 keeps the bad HOPOPT as raw extension data and rebuilds byte-exactly. Fixtureinfois unchanged. It breaks:test_ipv4_unit.pyandtest_tcp_udp_unit.py,test_a_truncated_option_still_parses_its_declared_length(OptionField.unpack never returns for a well-formed HOPOPT header with an SMF_DPD option #431, The #431 truncated-option short read has no test, only its empty-tail case does #572). These assert the invented zeros.test_pcapng_unit.pythat pin the clamp, whose stated reason is that one refusal should not abort the whole extraction.Options:
strictflag toOptionField, set only for HIP, MH, HOPOPT, IPv6-Opts and SCTP. IPv4, TCP and PCAP-NG keep their current zero-fill or clamp.I recommend (1). Invented zeros and the clamp both break the every-round-trip ruling, and keeping the bytes raw satisfies the PCAP-NG concern without aborting. It does change what a truncated IPv4 or TCP option looks like in
info.JarryShaw commentedon Oct 8, 2026
take (1) then
JarryShaw commentedon Oct 8, 2026
Ruling recorded: option (1), strict everywhere. An option whose declared length runs past the data raises
ProtocolError. Inside a parent layer, the octets are kept as captured, as #1171 does, so nothing invents zeros.A worker is taking
fix/1325-declared-length-overrun(50dd50f14) onto currentmain. The PCAP-NG part waits for #1378, which edits the same files.fix(protocols)!: a declared length running past the data is accepted …
fix(protocols)!: a declared length running past the data is accepted …
fix(ipv4,tcp)!: a header length declared past the data zero-fills on …