Skip to content

A per-field padding ceiling does not bound their sum: ~10,900x amplification survives the #554 fix #573

Description

@JarryShaw

A per-field ceiling bounds any single rjust() but not their sum, so a small crafted capture can still make Extractor retain orders of magnitude more memory than it read. Found while comparing the two candidate fixes for #554; filed as a follow-up rather than a blocker on either.

The residue

PR #569 bounds FieldBase.unpack's zero-padding at _MAX_ZERO_PAD_LENGTH = 0x40_000 (262,144 octets — libpcap's MAXIMUM_SNAPLEN, and this package's own Header.make default at pcapkit/protocols/misc/pcap/header.py:190). That is the right ceiling for one field. It says nothing about how many fields a single parse may pad.

UnknownSecrets.data at pcapkit/protocols/schema/misc/pcapng.py:1459 is callable-length via __length__, and accepts a declared 262,144 from 2 supplied octets — just under the ceiling, so the guard never fires.

Measured on #569's tree: 200 such blocks, about 6.4 KiB of input, retained 50.0 MiB RSS. That is a per-field amplification of roughly 10,900×, against a minimal DSB of ~24 wire octets. Extractor(store=True) is the default, so parsed results accumulate rather than being discarded — a 1 MiB crafted file implies on the order of 11 GiB retained.

For proportion

On main the same shape gives a ratio of roughly 178,000,000×, since a UInt32 maximum can be declared from one block. So #569 reduces the exposure by about 16,000× and is a large improvement — it simply does not close it. This is explicitly not an argument against that fix.

Suggested direction

A per-parse padding budget rather than a per-field one: track how much zero padding a single Extractor run has synthesised and refuse past a total, so the sum is bounded even when every individual field is under the ceiling. Where that budget lives is the design question — Extractor, the schema layer, or a context object — and it interacts with store=True's accumulation, which is worth deciding deliberately rather than by where it is easiest to add.

Honest limits on what was measured

The 50.0 MiB figure is a real RSS measurement at the schema level, invoking UnknownSecrets.unpack — the path SchemaField.unpack uses. It was not demonstrated end-to-end through Extractor on a real .pcapng file, so the 11 GiB extrapolation is arithmetic from the measured ratio rather than an observed number. A 64 MiB surrogate was used throughout and nothing was deliberately allocated at multi-gigabyte scale.

Separately, and caught by neither candidate fix: host_id_hi_selector at pcapkit/protocols/schema/internet/hip.py:162 returns BytesField(length=pkt['hi_len']) — a plain int read from a UInt16Field at hip.py:676 — which pads 65,535 octets from a 2-octet buffer. Bounded at 64 KiB so it is a note rather than a hole, with the same shape at mh.py:354,357 bounded near 255.

Activity

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions