Skip to content

Every EOF-truncated PCAP-NG file raises an uncaught ValueError: pcapng_block_selector passes a negative __length__ to SchemaField #678

Description

@JarryShaw

Found while measuring #594's amplification band, and deliberately left out of #676 because it is a
block-level defect rather than an option-level one, and fixing it changes frame counts on truncated
captures.

Every EOF-truncated PCAP-NG file raises an uncaught ValueError

Not "loses a frame" — raises out of Extractor, so the whole extraction is lost. Measured on
0c7f2b7c9 against the committed examples/captures/dhcp.pcapng, truncated at every 4-octet
boundary from -4 to -400. All 101 levels fail, identically:

dhcp.pcapng/whole  input=1508  frames=4   ok
dhcp.pcapng/-4     input=1504  ValueError: read length must be non-negative or -1
dhcp.pcapng/-8     input=1500  ValueError: read length must be non-negative or -1
...
dhcp.pcapng/-400   input=1108  ValueError: read length must be non-negative or -1

ValueError is not one of pcapkit.utilities.exceptions, so a caller cannot distinguish it from a
bug in its own code, and it defeats the #431/#571 standing constraint that a legitimately
truncated capture must still parse — for PCAP-NG that constraint is currently vacuous at the file
level
.

Where it comes from

  File "pcapkit/foundation/extraction.py", line 700, in record_frames
    self._exeng.read_frame()
  File "pcapkit/foundation/engines/pcapng.py", line 189, in read_frame
    block = P_PCAPNG(ext._ifile, num=ext._frnum+1, ...)
  File "pcapkit/protocols/protocol.py", line 644, in __init__
    self.__post_init__(file, length, **kwargs)
  File "pcapkit/protocols/misc/pcapng.py", line 1088, in __post_init__
    self._info = self.unpack(length, _read=_read, _seek_set=_seek_set, **kwargs)
  File "pcapkit/protocols/misc/pcapng.py", line 900, in unpack
    self.__header__ = cast('Schema_PCAPNG', self.__schema__.unpack(self._file, length, packet))
  File "pcapkit/utilities/decorators.py", line 280, in unpack
    schema = func(cls, data, length, packet)
  File "pcapkit/protocols/schema/schema.py", line 857, in unpack
    byte = data.read(length)
ValueError: read length must be non-negative or -1

The negative length originates at pcapkit/protocols/schema/misc/pcapng.py:228:

def pcapng_block_selector(packet: 'dict[str, Any]') -> 'Field':
    ...
    return SchemaField(length=packet['__length__'], schema=schema)

packet['__length__'] is seeded by @prepare from what is left in the file
(pcapkit/utilities/decorators.py:274) and then decremented by four for PCAPNG.type. A previous
block whose declared Block Total Length ran past the real end of the file leaves the reader
positioned such that fewer than four octets remain, so __length__ goes negative and
data.read(negative) raises. @prepare only turns a remainder of exactly zero into the quiet
StreamEOFError that the frame loop is built to catch (decorators.py:264-270); one, two or three
octets fall through.

The fix that is probably right

max(packet['__length__'], 0), which is the idiom pcapkit/protocols/schema/transport/sctp.py
already uses for the same hazard, and which Schema.unpack itself already anticipates —
schema.py:898-900 warns rather than raises when __length__ has gone negative, so the negative
value is expected to be survivable at that layer and only this one call site treats it as fatal.

Worth deciding alongside it: whether a short tail should surface as the quiet StreamEOFError the
frame loop already handles (giving the frames before the cut, like the legacy PCAP reader) or as a
ProtocolError naming the truncation. The first matches the #431 bar; the second is louder. Either
way it should be an in-library exception rather than a bare ValueError.

Why it is not in #676

#676 bounds an option's payload to the option area its block declares. This is the block's own
span against the file, one layer up. Measured byte-identical before and after #676 at all 101
truncation levels, including the failures — #676 neither causes nor cures it. Fixing it changes frame
counts on truncated captures, which wants its own review and its own breaking argument.

Related

Activity

  1. added
    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)
    on Sep 22, 2026
  2. JarryShaw commented on Sep 22, 2026

    @JarryShaw
    OwnerAuthor

    A second manifestation of the same root shape, found while probing #676's bound: a captured_len past the block makes EnhancedPacketBlock's option-area expression length - 32 - captured_len - (4 - captured_len % 4) % 4 negative, the negative reaches a _TextField template as f'{-N}s', and struct raises struct.error: bad char in struct format — again not one of pcapkit.utilities.exceptions.

    Measured on 200 Enhanced Packet Blocks with captured_len = 0xFFFFFF in 8,048 octets, byte-identical before and after #676. Same fix family: clamp the computed length at zero where it is consumed, rather than letting a negative reach struct or read().

  3. JarryShaw commented on Sep 22, 2026

    @JarryShaw
    OwnerAuthor

    Correction to the sweep in the body, from the cross-review of #676. I wrote that every truncation level raises the same ValueError; it does not.

    Re-swept dhcp.pcapng at 1-octet granularity over 501 levels:

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

    The reviewer swept 376 levels independently and found a third type, ProtocolError: unknown byteorder magic: 0x0, from cuts reaching into the Section Header Block. So the picture is: overwhelmingly the ValueError, with struct.error and a byteorder-magic ProtocolError at particular depths, and a small number of levels that do parse.

    That strengthens rather than weakens the report — three different exception types out of one root cause, two of them not from pcapkit.utilities.exceptions — but the original wording was over-general and worth correcting before it becomes the record.

  4. JarryShaw commented on Sep 23, 2026

    @JarryShaw
    OwnerAuthor

    Completing the census in the correction above, since the re-sweep stopped at 501 levels and two of the
    three exception types only appear deeper. Swept all 1,509 octet boundaries of dhcp.pcapng at
    1-octet granularity on f0999858e (post-#676, post-#683), classifying by whether the exception is one
    of pcapkit.utilities.exceptions:

    1479  ValueError: read length must be non-negative or -1        (foreign)
      10  struct.error: bad char in struct format                   (foreign)
       8  ProtocolError: unknown byteorder magic: 0x0 and friends   (in-library)
       4  FormatError: unknown file format                          (in-library)
       2  ProtocolError: PCAP-NG: [if_tsresol] invalid length       (in-library)
       6  parsed
    

    So 1,489 of 1,509 levels raise from outside the library — the two types this issue names, and the
    reviewer's byteorder-magic ProtocolError was already in-library. The struct.error depths are 372,
    373, 720, 721 and six more; at 372 it arrives by a route the comment above does not name — not the
    crafted captured_len, but __option_padding__ reaching -32 on EnhancedPacketBlock.padding_opts,
    because OptionField.unpack subtracts each parsed option's real size from the area without checking it
    fits. Same root shape, third consumption point.

    One correction to the "Where it comes from" section: the negative does not originate at
    pcapng_block_selector, it only surfaces there. PCAPNG.read's seek_cur = _seek_set + block.length
    seeks past the real end of the file when the last block declares more than the file holds — legal
    and silent — so the next block read has prepare derive the remainder as end - tell, which is
    already negative before PCAPNG.type is read at all. That is why the failure is the whole extraction
    rather than the one truncated block, and why max(packet['__length__'], 0) at the selector alone is not
    sufficient: with the length clamped to 0 the block schema fabricates a block from zero padding, and
    UnknownBlock.body's pkt['length'] - 12 then goes negative instead.

    Fix in #699: clamp the seek at the octets _read_fileng actually returned, report a tail under the
    12-octet block minimum as the quiet StreamEOFError the frame loop already catches, and floor every
    computed length in the schema at zero. 1,495 of the 1,509 levels parse, and none of the 14 that do not
    raises from outside the library.

  5. 9 remaining items

  6. added this to the 1.5 milestone on Oct 6, 2026
  7. moved this to Done in PyPCAPKiton Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions