Repository navigation
Unbounded allocation in FieldBase.unpack: a wire-declared length drives rjust() with no bound #554
Description
Activity
Fixed, in two stages, and ready to close.
4c0bcf9b7(fix(corekit): bound FieldBase.unpack's rjust() padding to a sane ceiling (#554) #569) added the per-field ceiling:_MAX_ZERO_PAD_LENGTH = 0x40_000inpcapkit/corekit/fields/field.py, bounding what a single wire-declared length can driverjust()to synthesise.13fd1860c(fix(corekit): bound the total zero padding a parse may synthesise (#573) #593) added the cumulative bound the per-field ceiling could not provide, since a packet holds many fields and their sum was unbounded:_MAX_ZERO_PAD_SHORTFALL = 0x10_000plus a budgeted band above it. Measured, the 200-block scenario went from 10,922x amplification to 54.6x.
Both constants are present in
mainatfa6d18e31.This issue stayed open after the first fix only because #569's body referenced it as a bare
#554rather thanFixes #554, so GitHub never closed it. Every pull request in the batch since has carried the keyword explicitly and closed its issue within a second of merging.One residual is deliberately left open as #594, and it is not part of this issue's scope: a 16-bit shortfall band is padded unconditionally, so a crafted capture still amplifies 1,637x. It stayed open because a legitimately truncated offload frame amplifies by the same ratio — 1637.375x crafted versus 1637.393x legitimate — so nothing inside
pcapkit/corekit/fieldscan separate them. Distinguishing them needs the frame's ownincl_len/orig_len, which lives at the PCAP-NG layer. #594 records the measurement and what a real fix would require.
pcapkit/corekit/fields/field.py:240-244:lengthderives from the field's configured length, which for several schemas is a callable resolved against the packet under parse — so it can be attacker-influenced by a length written on the wire.rjust(length, b'\x00')then allocateslengthbytes regardless of how much data actually arrived.The consequence is that a small crafted input can force a very large allocation. A roughly 40-octet PCAP-NG Decryption Secrets Block is enough to drive a multi-gigabyte
rjust, because the block's own declared length feeds the field length while the file supplies almost no bytes.This is the only memory-safety-shaped finding from a sweep of deferred work across this release's 68 merged PRs, and it was never raised in review.
Why it is worth a real fix rather than a cap
pcapkitparses untrusted capture files by design — that is the whole use case — so "the input is hostile" is the normal condition rather than an edge case. A plain upper bound would stop the allocation but silently truncate legitimate large fields; better is to refuse when the declared length exceeds the bytes actually available, which is a genuine format error and already has an in-library exception for it.Note only in-library exceptions from
pcapkit.utilities.exceptionsshould be used, andFieldValueErroris the existing precedent for an impossible field value.Coverage
A test that feeds a short buffer declaring a huge length and asserts a bounded in-library exception rather than an allocation. It must not actually attempt the allocation when run against the fixed code, and should be proven to fail — by raising
MemoryErroror hanging under a timeout — against the current code.