fix(pcapng): bound an option's payload to the area its block declares (#594)

[JarryShaw]

Fixes #594

The band, measured

#593 bounds the 32-bit padding band. The residual #594 records is a different
shape: a 16-bit declared length repeated across many blocks. An option's
length is a 16-bit wire field, so a four-octet option header can declare 65,535
octets of payload, and nothing bounded that against the option area the block
frames.

Reproduced on 0c7f2b7c9 with a crafted capture built to the issue's own
description — 2,000 Enhanced Packet Blocks in 80,048 octets, each carrying one
option that declares 65,535 octets against none present:

	input	frames	options	synthesised octets	amplification	maxrss delta
before	80,048	2,000	2,000	131,070,000 (125.00 MiB)	1637.393x	226,568 KB
after	80,048	2,000	2,000	0	0.000x	97,736 KB

The before row matches the issue's figures to three decimal places
(1637.393x, 125.00 MiB). Every frame and every option still parses after the
fix; only the synthesised padding is gone.

Where #593's budget lives, and why this band escaped it

The mechanism is in pcapkit/corekit/fields/field.py: a contextvars.ContextVar
ledger (_zero_pad_ledger, :168-171) charged inside FieldBase.unpack
(:391-528), against _MAX_ZERO_PAD_LENGTH = 0x40_000 (:64),
_MAX_ZERO_PAD_SHORTFALL = 0x10_000 (:104) and
_ZERO_PAD_BUDGET_RATIO = 0x10 (:125).

The band escapes it at field.py:494:

if padding > _MAX_ZERO_PAD_SHORTFALL:

A shortfall of 65,536 octets or fewer is never charged to the ledger at all,
so repeating it accumulates without limit. That is deliberate, not an oversight:
65,536 is the full span of a 16-bit wire length, and
tests/corekit/test_fields_field.py::test_a_shortfall_within_a_16_bit_length_is_never_refused
(:377) pins it, because a capture cut short by its snapshot length must still
parse. #571 was declined for proposing exactly the rejection that would break
that.

So the existing mechanism genuinely cannot cover this case at its own layer:
at FieldBase.unpack there is nothing to distinguish a crafted shortfall from a
legitimate one. Lowering the threshold reintroduces the history dependence #593
removed. _zero_pad_budget() (field.py:174-199) exists for scoping but has
zero production call sites — five call sites, all in
tests/corekit/test_fields_field.py — and scoping alone would only shrink the
ratio, not bound it.

What this change does, and where the bound comes from

bounded_option() clamps an option or record payload to the octets its area has
left at that field, and emits a SchemaWarning when it does. It is applied
to all 15 variable-width option and record payloads in the PCAP-NG schema.

bounded_area() then clamps a packet block's option area to the octets the
block itself holds, less the trailing Block Total Length. This closes a hole the
cross-review found: BlockType.post_process checks Block Total Length only
against its own trailing copy, never against the file, so a block declaring
1,000,000 octets while holding 36 sized its area at 999,964 and an option
declaring 65,535 was under that and never clamped — 1,820x, with no warning.
It is a no-op on well-formed blocks, where the octets left of the block are
exactly the area plus the trailing length's four. See the verdict comment below
for the measurement and for why the - 4 is load-bearing rather than cosmetic.

The bound has to come from this layer, and the information is already here. What
separates the crafted case from the legitimate one is not the shortfall's size —
both sit inside a 16-bit length — but whether the option is inconsistent with
the framing the block itself declares. Block Total Length is authoritative and
cross-checked against its own trailing copy (BlockType.post_process), so the
option area is length less the fixed fields, captured_len, and
captured_len's padding. An option declaring more payload than that area has
left is malformed however complete the file behind it is. A snapshot-truncated
capture says so through captured_len instead and leaves its options whole, so
it never trips this.

On the captured_len correction in #594: it stands and this change relies on
it. captured_len is not inert telemetry; it drives three formulas inside
EnhancedPacketBlock — packet_data at :1022, padding_data at :1024, and
the options area at :1031 on 0c7f2b7c9 (re-verified; now :1100, :1102
and :1109 after the insertion above them). This diff does not touch any of
the three. The area expression at :1031/:1109 is precisely what supplies
the bound, reaching the payload through OptionField.unpack's
schema.unpack(file, length, packet)
(pcapkit/corekit/fields/collections.py:448) and prepare's
packet['__length__'] = length (pcapkit/utilities/decorators.py:274).

Clamp, not raise, and not truncate silently

Three choices were on the table for the band. The change clamps and warns:

• Raising breaks the OptionField.unpack never returns for a well-formed HOPOPT header with an SMF_DPD option #431/corekit: reject short dynamic field buffers #571 bar. A PCAP-NG block read has no catch point
  above FieldBase.unpack, so one refusal aborts the whole extraction rather
  than one block — a truncated capture would stop reporting at the cut instead
  of reporting the frames before it.
• Silently truncating would change bytes with no signal, which the issue
  rules out. Hence the SchemaWarning, which names the declared length, the
  octets remaining, and what was read.
• Clamping is history-independent: it reads only this block's own declared
  framing, never a running total, so byte-identical input answers identically
  regardless of what preceded it. That is the property fix(corekit): bound the total zero padding a parse may synthesise (#573) #593's own notes record a
  naive threshold destroying.

Block-level payload fields (UnknownBlock.body, CustomBlock.data,
SystemdJournalExportBlock.entry) are deliberately left alone: those are the
32-bit band #593 already budgets, and clamping them would change block-level
truncation behaviour, which is its own review. The five non-packet option
areas — Section Header, Interface Description, Name Resolution, Interface
Statistics, Decryption Secrets — are also left unclamped, since each computes its
span with a different offset and the area-equals-remainder-less-four equality has
to be re-established per block rather than assumed. They keep the framing
assumption; #678 is the general fix.

House precedent for the shape: pcapkit/protocols/schema/transport/sctp.py:886
is length=lambda pkt: max(pkt['__length__'], 0), and its bounded() does the
same __length__ clamp for SCTP list fields.

The remaining < 0 guard is there for the unpack path, where __length__ can
already be past zero because Schema.unpack warns and carries on when an earlier
field over-consumed. Without it, a ten-octet area gives the field a '-2s'
template and unpack raises struct.error. It is not needed for the pack path:
the cross-review established that both BytesField.pre_process
(pcapkit/corekit/fields/strings.py:91-93) and StringField.pre_process
(:148-150) already repair a negative width to len(value), so an earlier
version of this paragraph had the justification wrong.

Which inputs discriminate a correct fix from a plausible wrong one

This is the part the repo has been bitten on four times, so each test names the
wrong rule it rules out.

input	correct	plausible wrong rule it kills
area 8, declares 4, 4 present	reads 4	clamping to the area including the 4-octet option header reads 0
area 8, declares 8, 4 present	reads 4	clamping to the area (8) rather than the remainder at that field (4) leaves 4 zeros synthesised — the sharpest one
area 8, declares 5, 4 present	reads 4	leaving a one-octet shortfall padded, or refusing the option outright
area 12, two options, 2nd declares 65,535	2nd reads 0	clamping to the area as declared rather than as the earlier options left it hands the tail option 12 octets
area 65,540, declares 65,535, 65,535 present	reads 65,535	any constant ceiling on option payloads — a full 16-bit option is legitimate when the block reserves room, and nothing here is short
a ten-octet area, so __length__ reaches -2 at the payload	reads 0, parses	dropping the remaining < 0 guard: the field's template becomes '-2s' and unpack raises struct.error
a block declaring 1,000,000 octets while holding 36	reads 0	bounding the payload but not the area — the area inherits the lie and 65,535 octets are synthesised anyway
pack an 8-octet option with __length__ absent	8 octets survive	nothing — see the correction below; kept as a no-regression check, not a discriminator

The first and fifth rows are the ones that pass both before and after: they
exist to catch a fix that is too aggressive, which is the failure mode a
single-crafted-input test cannot see.

The bound is asserted as a property, not as one example

test_the_payload_never_exceeds_the_area_for_any_declared_length sweeps every
combination of 1-3 options against declared lengths
(0, 1, 4, 5, 8, 12, 0x100, 0x1000, 0xFFFF) — 27 subtests — and asserts the
octets a block's options report holding never exceed the area, and never exceed
the block's own declared length. That is the invariant that bounds the
amplification, since the block's declared length is what the reader advances the
file by (pcapkit/protocols/misc/pcapng.py:968,977).

test_the_amplification_does_not_grow_with_the_block_count then asserts the
ratio is flat in the block count (1, 8, 64, 512 blocks), which is what
distinguishes a bound from a smaller constant: #594's vector was linear in the
block count and therefore unbounded in the input size.

Nine of the 27 sweep subtests fail without the fix, at declared lengths 256,
4,096 and 65,535.

No false positives on real captures

Every PCAP-NG fixture, committed and generated, parsed with warnings captured:

dhcp.pcapng                  frames=4  options=0   clamp_warnings=0
dhcp_big_endian.pcapng       frames=4  options=0   clamp_warnings=0
dhcp_little_endian.pcapng    frames=4  options=8   clamp_warnings=0
many_interfaces.pcapng       frames=64 options=0   clamp_warnings=0
profile.pcapng               frames=40 options=320 clamp_warnings=0   (1,030 payload octets)
test.pcapng                  frames=5  options=10  clamp_warnings=0   (59 payload octets)
TOTAL CLAMP WARNINGS ACROSS ALL PCAP-NG FIXTURES: 0

Separately, 101 EOF-truncation levels of dhcp.pcapng (every 4 octets down to
-400) were swept before and after: byte-identical results, including the
failures.

Failing then passing, exit codes read from files

The 15 new tests, run against the pristine HEAD copy of the schema file and
then against the modified one (same selection, same command):

=== BEFORE (origin/main copy of pcapkit/protocols/schema/misc/pcapng.py) ===
39 failed, 8 passed, 30 subtests passed
exit code: 1
=== AFTER (modified pcapkit/protocols/schema/misc/pcapng.py) ===
15 passed, 62 subtests passed
exit code: 0

Nine of the 15 test functions fail without the fix:

test_an_option_declaring_more_than_its_area_reads_only_the_area
test_an_option_over_declaring_by_four_octets_is_clamped_to_the_remainder
test_an_option_over_declaring_by_one_octet_loses_only_that_octet
test_a_tail_option_gets_only_what_the_options_before_it_left
test_every_clamped_payload_is_bounded_and_none_was_missed
test_the_amplification_does_not_grow_with_the_block_count
test_the_clamp_warns_rather_than_changing_bytes_in_silence
test_the_payload_never_exceeds_the_area_for_any_declared_length
test_a_block_declaring_more_room_than_it_holds_bounds_by_what_it_holds

test_every_clamped_payload_is_bounded_and_none_was_missed fails all 15
subtests without the fix — one per clamped site — which is what shows each of
the 15 was genuinely unbounded rather than incidentally safe.

Regression runs

tests/protocols/misc/test_pcapng_unit.py
tests/protocols/test_pcapng_regression.py
tests/foundation/engines/test_pcapng_engine.py
tests/integration/test_pcapng_end_to_end.py
  -> plus tests/protocols/test_option_roundtrip_unit.py and tests/corekit/test_fields_field.py
  -> 117 passed, 1 skipped, 742 subtests passed   exit code: 0

And the contract tests, before and after, same command:

tests/protocols/test_option_roundtrip_unit.py
tests/protocols/transport/test_tcp_udp_unit.py
tests/protocols/internet/test_ipv4_unit.py
tests/corekit/test_fields_field.py
  BEFORE -> 85 passed, 528 subtests passed   exit code: 0
  AFTER  -> 85 passed, 528 subtests passed   exit code: 0

EXPECTED_FAILURES: no entry moved. It was read by importing the module
(** unpacking makes it ungreppable) — 45 entries, 35 PCAP-NG-related. The
round-trip module enforces both halves (an unrecorded failure fails, and a
recorded gap that starts passing fails with "delete its EXPECTED_FAILURES
entry", test_option_roundtrip_unit.py:676-717), so the byte-identical
85-passed/528-subtest result before and after is the evidence that none moved.
No entry was deleted.

pcapkit.__file__ for every measurement:
/local/home/jarryx/GitHub/PyPCAPKit/.claude/worktrees/agent-a8c3daad0b5a55dd4/pcapkit/__init__.py
— asserted before any other import, with __editable__ finders stripped from
sys.meta_path and the worktree at sys.path[0]. Crafted inputs were capped and
parsed in a subprocess under resource.setrlimit with RLIMIT_AS.

Coverage and lint

	statements	branches	cover
before	489	58	100%
after	510	64	100%

Coverage does not go backwards: it stays at 100% while adding 9 statements and 2
branches, all executed -- 21 statements and 6 branches, with the area bound. PCAP-NG
subtests 189 to 251. pylint and mypy on the
changed file, diffed before against after: zero new findings, zero resolved
(364 pre-existing pylint findings and 1 pre-existing unused-ignore identical in
both runs).

CI is pending, not green — the Actions queue is backed up, and any single
passing check on this PR is pyup.io/safety-ci, a StatusContext rather than an
Actions job. Everything above is local.

Labels, and the breaking argument

Applied: fix, test. breaking deliberately not applied, and here
is the argument so it can be overridden.

• No capture that previously parsed now raises. Nothing in this change raises
  at all; the clamp is not a rejection. Every fixture, every truncation level and
  every crafted input that parsed before still parses, with the same frame and
  option counts.
• The behaviour that changes is the value of a payload on input the PCAP-NG
  framing declares malformed — an option claiming more payload than its own
  block's declared area has left. That payload used to be zero-padded to the
  declared length and is now the octets actually in the area.
• The honest counter-argument: a consumer that reads option.data from a
  malformed capture and depends on its length matching option.length sees a
  shorter value. option.length itself is unchanged, so the two can now
  disagree, and the new SchemaWarning is the signal that they do. If that
  counts as a public-attribute change in the sense Frame.len and Frame.cap_len are populated from opposite wire fields by the PCAP and PCAP-NG readers #618 was labelled breaking
  for, breaking should be added — it is additive, and I would not argue
  against it.

Interaction with #646

#646 (every PCAP-NG packet block loses its payload octets) is neither
improved nor worsened by this change. Its defect is packet being computed
correctly in PCAPNG.unpack (pcapkit/protocols/misc/pcapng.py:902-909) and
then overwritten at pcapkit/protocols/protocol.py:647, because
ProtocolBase.packet treats PCAPNG.length — the Block Total Length — as a
pre-payload header length. That is the packet_data payload path, sized from
captured_len at :1022. This change touches only option and record
payloads, never packet_data, never captured_len, and nothing in
protocol.py (which #640 owns). #646 does not mention padding, and its own fix
lands in files this diff does not open.

Found and deliberately not fixed

• PCAP-NG can barely parse an EOF-truncated file today. Swept at 1-octet
  granularity, 501 truncation levels of dhcp.pcapng give 497 uncaught
  ValueError: read length must be non-negative or -1 from
  pcapkit/protocols/schema/schema.py:857, 2 struct.error: bad char in struct format, and 2 that parse. (An earlier revision of this body said all levels
  raised the same ValueError; the cross-review caught that over-generalisation.)
  The ValueError arises
  because pcapng_block_selector passes
  SchemaField(length=packet['__length__'])
  (pcapkit/protocols/schema/misc/pcapng.py:228) and __length__ can be
  negative when a block's declared length has run past the file. sctp.py already
  uses max(pkt['__length__'], 0) for the same hazard. Identical before and after
  this change. It is a block-level robustness gap, changes frame counts on
  truncated captures, and wants its own review and its own breaking argument —
  filing separately rather than folding it in here.
• A block that over-declares its own total length (Block Total Length past
  what the file holds) would bypass an area-derived bound in principle. In
  practice it hits the ValueError above on the current tree, before any padding
  is synthesised, so it is not a usable route — measured byte-identical before
  and after. Closing the gap properly depends on the item above.

[JarryShaw]

The truncation gap in Found and deliberately not fixed is now filed as #678, with the 101-level measurement and the max(packet['__length__'], 0) candidate fix. Measured byte-identical before and after this PR at every truncation level, so this change neither causes nor cures it.

[JarryShaw]

Adversarial probes against the bound, before and after

The PR body reports one vector. Here are five more, each an attempt to get past the bound, measured
end to end through Extractor(store=True) on the pristine origin/main schema and then on this
branch. Ratio is synthesised option octets over input octets.

probe	input	before	after
crafted/500 — the #594 vector, 1 option/block declaring 65,535	20,048	1634.452x	0.000x
many-options/200x8 — 8 over-declaring options per area	13,648	960.360x	0.410x
many-options/100x64 — 64 over-declaring options per area	29,248	224.067x	0.862x
opt_comment/500 — StringField payload, not BytesField	20,048	1634.452x	0.000x
huge-captured-len/200 — captured_len = 0xFFFFFF, driving the area expression negative	8,048	struct.error: bad char in struct format	identical

Worst ratio achieved after the fix across every probe: 0.862x. That is the bound behaving as
claimed — payload never exceeds the area, the area never exceeds the block's declared length, and the
reader advances the file by that length.

many-options/100x64 is the most informative row: the area is 64 option headers = 256 octets, the
first option is clamped to the 252 octets the area has left after its header, and the remaining 63
get zero. 252 octets from a 292-octet block is 0.863x — exactly what the bound predicts, and it
confirms that packing many options into one area does not multiply the allowance.

opt_comment matters because it is a StringField rather than a BytesField, which is a different
pcapkit.corekit.fields type with its own post_process. Both are covered.

A correction to my own measurement, since it bears on trusting the rest

My first run of this comparison was wrong and I am reporting it rather than quietly re-running: the
script took its baseline from git show HEAD:..., and once the fix was committed HEAD is the
fixed tree, so the "before" column was the fixed file and every row read 0.000x in both columns. Fixed
by taking the baseline from git show origin/main:... with an added assertion that the extracted copy
does not contain bounded_option. The numbers above are from the corrected run.

The earlier evidence in the PR body is unaffected — those runs all predate the commit, when HEAD
was 0c7f2b7c9 — and each carries its own internal proof that it really used the unfixed file: the
test comparison returned exit 1 with 38 failures, and the coverage baseline reported 489 statements
against the fixed file's 498.

Two probes that were inconclusive, not clean

nrb-records/200 and idb-if_name/500 both returned frames=0 — my hand-built Name Resolution and
Interface Description blocks were malformed enough not to parse, so those two probes prove nothing
either way and I am not claiming them as passes. The record and non-EPB option paths are covered
instead by test_every_clamped_payload_is_bounded_and_none_was_missed, which exercises all 15 clamped
sites directly at the schema level — including UnknownRecord, IPv4Record, IPv6Record,
NS_DNSNameOption and IF_NameOption — and fails all 15 subtests without the fix.

A third robustness gap, pre-existing and left alone

huge-captured-len raises struct.error: bad char in struct format, byte-identical before and
after. A captured_len past the block makes
length - 32 - captured_len - (4 - captured_len % 4) % 4 negative, the negative reaches a
_TextField template as f'{-N}s', and struct rejects it. Like #678 this is a negative computed
length escaping as a non-library exception, one layer above the option area, so it is out of scope
here — noted on #678 rather than filed separately, since it shares that root shape.

Python 3.10

vermin over both changed files reports minimum required 3.9, with every finding at a
pre-existing line (:66, :479, :702 in the schema; :1, :1288 in the tests) and none in the
new code. Clean for the 3.10 floor.

CI is still QUEUED across the whole matrix — nothing here rests on it.

[JarryShaw]

Completeness: every variable-length field in the file, accounted for

"All 15 option and record payloads" is a claim about coverage, so here is the mechanical check rather
than my word for it. Every length=lambda pkt: in pcapkit/protocols/schema/misc/pcapng.py that is
not wrapped in bounded_option, classified — line numbers on this branch:

Option and record area spans (7). :639 SHB, :962 IDB, :1109 EPB, :1297 NRB records,
:1456 ISB, :1685 DSB, :1806 PacketBlock. These are OptionField/ListField spans derived
from Block Total Length — they are the bound's source, not payloads, and clamping them would be
circular.

PayloadField (3). :1100 and :1799 sized from captured_len, :1138 from
min(snaplen, original_len). PayloadField takes the special-cased branch in Schema.unpack
(pcapkit/protocols/schema/schema.py:822-831) — a bare data.read(...) with no ljust, no declared
width and no ledger — so a shortfall here allocates nothing and there is no padding to bound. This is
the same finding #594's investigation recorded: the "54-octet frame declaring 65,535" comparator pads
zero octets because its shortfall is absorbed here.

Block-level BytesField (3). :450 UnknownBlock.body, :1480
SystemdJournalExportBlock.entry, :1721 CustomBlock.data, each pkt['length'] - N off a 32-bit
Block Total Length. Deliberately left: that is the 32-bit band FieldBase.unpack's ledger already
budgets, and clamping them changes block-level truncation behaviour, which wants its own review.

Already __length__-bounded (3). :1537 UnknownSecrets.data, :1548 TLSKeyLog.data, :1587
WireGuardKeyLog.data — all three are already length=lambda pkt: pkt['__length__'], which is the
same bound this change applies, reached through dsb_secrets_selector. Nothing to add.

Fixed-length (6). :775 and :788 (interface, 6 and 8 octets), :1627/:1642 (key, 16) and
:1631/:1650 (padding, 2) in the ZigBee secrets. A fixed width cannot be driven from the wire.

PaddingField throughout. Schema.unpack's padding branch (schema.py:836-844) is also a plain
data.read(length) with no padding synthesis, so (4 - pkt['length'] % 4) % 4 computed from an
unclamped declared length reads at most 3 octets and allocates nothing. Left unclamped on purpose: it
is also what a well-formed option needs on the pack path.

So the set of option and record payload fields is exactly the 15 that are wrapped, and
test_every_clamped_payload_is_bounded_and_none_was_missed has one subtest per site — all 15 of which
fail without the fix. If a payload is added later without the bound, it shows up there as a subtest
nobody wrote.

Two citations in the PR body, re-verified directly rather than taken from notes:
pcapkit/protocols/schema/transport/sctp.py:886 is length=lambda pkt: max(pkt['__length__'], 0),
and the reader's advance is seek_cur = _seek_set + block.length at
pcapkit/protocols/misc/pcapng.py:968 with the seek at :977.

[JarryShaw]

GOOD TO GO

Cross-review by a subagent on sonnet; this PR was authored on opus. Deliberately a different
model, briefed to falsify rather than bless. Its verdict line is above, verbatim.

It ran independently — its own clone, its own crafted captures built from #594's description rather
than from my harness, and it implemented six wrong rules to see whether my tests catch them. It also
found and corrected a bug in its own harness mid-review and said so. Two of its findings were real
and I have acted on both, so the diff has changed since it reviewed: df67d72d2 replaces
158badcbc, rebased onto c37d80c85.

---

Finding 1 — the bound rested on an unvalidated wire field. DISPUTED, and now fixed.

The reviewer's most valuable output, and it was right. My bound was:

payload ≤ option area ≤ Block Total Length ≤ octets the reader advances by

The third link is unvalidated. BlockType.post_process compares length against its own trailing
copy and never against the file. So a block declaring 1,000,000 octets while holding 36 sizes its
option area at 999,964, an option declaring 65,535 is comfortably under that, the payload clamp never
fires, and 65,535 octets are synthesised anyway. The reviewer measured 1,820x, with no
SchemaWarning at all, at exactly the API surface my own tests use. I reproduced it before changing
anything.

My test suite could not have caught this: block_body/epb_body always compute honest lengths, so
declared and real never diverged in any input I wrote. That was the gap, and the reviewer named it
precisely.

Fixed rather than documented, with a new bounded_area() applied to both packet blocks' option
areas. It clamps the area to the octets the block was actually handed, so the payload is bounded by
real octets whatever the block declared. It is a no-op on well-formed blocks by construction: at the
option field the only field still to come is the trailing Block Total Length, so the octets left of
the block are exactly the area plus four.

That + 4 is not academic, and finding it is why this was worth fixing rather than noting. My first
attempt clamped to __length__ whole, and the over-declared block then read the trailing Block
Total Length itself as option payload — four octets of payload on a block holding none, caught by
the new test asserting zero. Subtracting the trailing field's width makes the equality exact.

Two new tests: one pinning the over-declared block at zero payload, and
test_the_area_bound_does_not_touch_a_well_formed_block, which asserts the no-op across
captured_len 1..16 so the equality is checked at every alignment rather than assumed.

The reviewer also noted this is currently masked end-to-end by #678's crash, and that it would become
live once #678 is fixed. That is exactly right, and it is why the area bound belongs here rather than
waiting.

Finding 2 — one row of my discriminator table was false. Corrected, with a real replacement.

The reviewer implemented all six wrong rules. Five were caught:

wrong rule	result
clamp to the whole area (remaining + 4)	19 failed — caught
raise instead of clamp	46 failed — caught
constant ceiling (4096)	39 failed — caught
off-by-four +4	19 failed — caught
off-by-four -4	53 failed — caught
no remaining < 0 guard	12 passed, 46 subtests — NOT caught

So my claim that "pack an 8-octet option with __length__ absent" kills the missing-guard rule was
wrong, and the reviewer's explanation is correct: I verified directly that both
BytesField.pre_process (pcapkit/corekit/fields/strings.py:91-93) and StringField.pre_process
(:148-150) repair a negative width with self._length = len(value). The pack path cannot see the
guard at all.

But the guard is load-bearing — on the unpack path, where pre_process never runs and
__length__ can already be past zero, because Schema.unpack warns and carries on when an earlier
field over-consumed. I constructed the input the reviewer's mutation needed:

A ten-octet option area. The first option takes eight, leaving two — not a multiple of four — so
the loop runs again with two octets of area; the next option's type field takes both, its length field
short-reads to zero, and __length__ lands at -2 before the payload callback is consulted. Without
the guard the field's struct template becomes '-2s':

=== WITH the guard (as shipped) ===
ten-octet-area/negative-remaining   area=10   options=[(2, 2), (0, 0)]
eight-octet-area/control            area=8    options=[(2, 2)]
=== WITHOUT the guard ===
ten-octet-area/negative-remaining   area=10   error: bad char in struct format
eight-octet-area/control            area=8    options=[(2, 2)]

The control parses either way, so the input isolates the guard rather than the clamp. Added as
test_a_negative_remainder_is_left_alone_rather_than_clamped_to. Being precise about what it
proves: it passes on origin/main too, because with no clamp at all the payload is sized from
pkt['length'] = 0. It discriminates the guard within the fix, demonstrated by the mutation above,
not fix-versus-no-fix — and the pack-path test's docstring now says it is a no-regression check rather
than a discriminator, crediting this review.

The seventh rule, min(nominal, len(buffer)), the reviewer could not implement — the callback receives
only pkt, not the buffer — and correctly observed that declared-remaining and actually-available
coincide in every input I wrote. Finding 1 is where that distinction lived, and it is now closed for
the packet blocks.

Finding 3 — my truncation claim was over-general. Corrected.

I wrote that all truncation levels raise the same ValueError. The reviewer swept 376 levels and found
three distinct exception types and five successes. I re-swept at 1-octet granularity over 501
levels and confirm the shape, with my own counts:

497  ValueError: read length must be non-negative or -1
  2  error: bad char in struct format
  2  ok
BYTE-IDENTICAL across every level

Our counts differ because the ranges differ — the reviewer cut deeper, into the Section Header Block,
which is where their third type (ProtocolError: unknown byteorder magic: 0x0) comes from. Both agree
on what matters: byte-identical before and after, so the no-regression conclusion stands and only
my framing was imprecise. #678 has been updated.

Verified without dispute

• Headline numbers: independently reproduced from A 16-bit padding shortfall band is still unbudgeted after #593: a crafted capture amplifies 1,637x, indistinguishable from a truncated one at the field layer #594's description, not from my harness — input
  80,048 (matches), 1637.393x before to three decimals, 0.000x after, maxrss delta 229,796 KB
  against my 226,568 KB.
• Failing-then-passing: exact match to my claim — 38 failed, 6 passed, 14 subtests, exit 1
  before; 12 passed, 46 subtests, exit 0 after; 8 of 12 functions failing. Now 9 of 15 and 62
  subtests, with the four extra tests added since.
• EXPECTED_FAILURES: imported, 45 entries, and the defining file is byte-identical between base
  and PR — so no entry could have moved. A stronger argument than mine.
• Regressions: 79 passed, 1 skipped, 593 subtests on its five-file selection. Mine post-rebase:
  117 passed, 1 skipped, 742 subtests, exit 0.
• Zero clamp warnings across all six fixtures, frame counts (4, 4, 4, 64, 40, 5) matching exactly.
• Python 3.10: no 3.11+ constructs. My vermin run agrees — minimum required 3.9, every finding at
  a pre-existing line.
• breaking: no counterexample found. No input that parsed before now raises; no well-formed or
  legitimately-truncated input whose output changes. It correctly left the
  len(option.data) == option.length judgement to the maintainer rather than claiming to settle it.
• captured_len: lines 1022/1024/1031 on 0c7f2b7c9 confirmed exactly, the captured_length
  repeat confirmed, and the diff confirmed never to touch them — only line-number drift.

What it could not resolve, stated rather than glossed

Whether Finding 1's bypass can be made to produce a silent, non-crashing amplification through some
other entry point — a direct schema-level embedding, or any path after #678 is fixed. It showed the
gap is real at the schema layer and masked at the Extractor layer, and did not exhaustively search
for a path avoiding both. The area bound now closes it for the two packet blocks; the five non-packet
option areas still carry the framing assumption, which is stated in bounded_area's docstring and
left to #678.

It removed its worktree and scratch clone, confirmed via git worktree list, and never touched my
worktree or the shared checkout.

Two line-number citations of mine, corrected while I was at it

Schema.unpack's PayloadField branch is pcapkit/protocols/schema/schema.py:822-833 and its
PaddingField branch is :835-845; an earlier comment said 822-831 and 836-844. The
round-trip enforcement I cited as test_option_roundtrip_unit.py:676-717 is :675-681 (no stale
entries) plus :700-718 (both halves, including "If the defect is fixed, delete its
EXPECTED_FAILURES entry"). The blocks named were the right ones; the ranges were off by one at the
edges.

---

Re-verified after the amend and the rebase onto c37d80c85: 117 tests, 1 skipped, 742 subtests,
exit 0 read from a file. Coverage of the changed file 510 statements / 64 branches / 100% against
origin/main's 489 / 58 / 100% — still no misses. pylint and mypy, diffed against origin/main:
zero new findings. CI remains QUEUED; nothing above rests on it.