FieldBase.unpack pads a short read with rjust regardless of byte order, silently corrupting little-endian values

[JarryShaw]

FieldBase.unpack zero-pads a short read with rjust unconditionally. That is correct for a big-endian field and wrong for a little-endian one, where the padding lands in the low-order bytes and silently inflates the value.

Measured

pcapkit/corekit/fields/field.py:244:

value = struct.unpack(self.template, buffer[:length].rjust(length, b'\x00'))[0]

On origin/main (13a75dfcd), CPython 3.14.7, a UInt32Field given one octet where four are declared:

little  short read of 0x78 -> 2013265920   (expected 120)
big     short read of 0x78 -> 120          (expected 120)

0x78 becomes 0x78000000. No exception and no warning — the field returns a plausible integer that is wrong by seven orders of magnitude.

Why it matters

The short-read accommodation itself is deliberate (#431): a snapshot-truncated capture should still parse. The padding side, however, must follow the field's byte order — a truncated little-endian field has lost its high bytes, so the zeros belong on the right, not the left.

Classic PCAP files are little-endian on little-endian hosts, which is the common case, so this affects ordinary truncated captures rather than a crafted corner.

Reported effects beyond the wrong value

The #594 worker reports this as the confirmed root cause of the previously-noted unhandled MemoryError at pcapkit/protocols/protocol.py:1016 on dhcp_little_endian.pcapng truncated to 161 octets — an inflated length derived from a corrupted little-endian read is then used as an allocation size. That MemoryError had been observed twice before and attributed only to "the #431/#571 failure class"; this explains the mechanism. I have verified the value corruption above by direct execution but have not myself reproduced the MemoryError chain, so that link is reported rather than confirmed here.

It also reports ljust as the fix, verified safe against all 19 example captures and the relevant test files. Also not independently verified by me.

Notes

• Distinct 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, which is about the amount of padding synthesised. This is about which side it goes on, and it corrupts values rather than exhausting memory. The 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 worker deliberately did not ship it, because bundling it under a Fixes #594 PR would have misrepresented what that PR closed — the right call.
• Distinct from pre_process sizes a value with floor division disguised as math.ceil, so every octet boundary raises OverflowError #599 and ILNP nonce builders size with floor division: math.ceil(bit_length() // 8) at hopopt.py:1889 and ipv6_opts.py:1892 #601, which are floor-division sizing errors.
• A fix wants a test per byte order at more than one width, since a big-endian field must keep behaving exactly as it does now.

[JarryShaw]

Strengthening this issue: rjust is wrong for both byte orders, not only little-endian. The title and body understate it.

A cross-review checked big-endian with a non-degenerate value — 0x01020304 with 3 of 4 octets captured — and I have confirmed the arithmetic:

little-endian, 1 of 4 octets of 120 (0x78):
  rjust -> 00000078 -> 2013265920      wrong
  ljust -> 78000000 -> 120             correct

big-endian, 3 of 4 octets of 0x01020304:
  rjust -> 00010203 -> 0x10203         wrong, silently scaled DOWN by 256x
  ljust -> 01020300 -> 0x1020300       correct, missing trailing LSB assumed zero

So the defect is not "padding on the wrong side for little-endian". It is that rjust assumes the missing octets are the most significant ones, when a short read always loses the trailing octets — which is true regardless of byte order. For big-endian the error scales the value down; for little-endian it scales it up. ljust is value-preserving in both cases, because the octets that were not read are exactly the ones at the end of the buffer.

That also makes the fix simpler than this issue implied: it is not byte-order-conditional. ljust unconditionally is correct, and a test needs both orders at more than one width purely to pin that.

One consequence worth stating for whoever fixes it: a big-endian short read currently produces a value that is smaller than the truth, which is far more likely to pass a sanity check than the inflated little-endian case. So the little-endian symptom is the loud one and the big-endian symptom is the dangerous one.

Verified by execution on origin/main (13a75dfcd). The MemoryError chain at pcapkit/protocols/protocol.py:1016 remains reported-but-not-reproduced by me.

[JarryShaw]

Correcting this issue's framing on the basis of #621's work, and adding the one thing that changes its severity.

The MemoryError is conditional on available address space, and on an ordinary host the real defect is worse than a crash. I recorded the MemoryError at pcapkit/protocols/protocol.py:1016 as the consequence, relayed from the #594 investigation. #621 established what actually happens:

• Read out of the failing frame's own locals, a one-octet read of a little-endian 32-bit PCAP-NG block length yields 0x78000000 (1.88 GiB) at truncation 161, and 0x84000000 (2.06 GiB) at 641 and 1389 — each passed straight to self._file.read().
• Under a 1 GiB RLIMIT_AS all three raise MemoryError at exactly protocol.py:1016.
• Under 2 GiB, truncation 161 gives ValueError and only 641/1389 raise — consistent with those sizes rather than contradicting them.
• At 4 GiB the allocation succeeds. The parse then fails the same way it now fails directly.

So on an unconstrained host this is not a crash at all: it is a roughly 1.2 million-fold allocation amplification from a 1772-octet file. That belongs to the family of #554, #573 and #594 rather than being merely an exception-hygiene problem, and it is a more serious reading of #604 than the one I filed.

The fix is ljust unconditionally, and #621 confirmed that at four widths across both byte orders, reading the padding side back out of inspect.getsource(FieldBase.unpack) so the measurement names the code it ran against:

LE, 1 of 4 of 120            rjust 2013265920        ljust 120                  truth 120
BE, 3 of 4 of 0x01020304     rjust 0x10203           ljust 0x1020300            truth 0x1020300
BE, 1 of 2 of 0x0102         rjust 0x1               ljust 0x100                truth 0x100
BE, 5 of 8 of 0x0102…08      rjust 0x102030405       ljust 0x102030405000000    truth 0x102030405000000

Full reads are byte-identical at every width and both orders.

Two further corrections to the record. The line moved: this issue cites field.py:244, and on current main it is pcapkit/corekit/fields/field.py:506, because #593's budget work landed above it. And coverage cannot see this fix — it stays flat at 84%, because line 506 was already executed; what rose is 195 → 328 subtests, since the byte-order × width × truncation axis is what a statement counter is blind to.

Truncated captures still parse, which was the standing constraint: 20 captures swept both ways against the same base with fixtures regenerated for each, every full parse identical at 1594 frames, and 231 of 232 outcomes byte-identical. The single difference is many_interfaces.pcapng at 5222 octets moving between two ValueErrors — already failing before, not a crash either side.

A sibling defect in the same family is now filed separately as #622: SeekableReader.truncate at pcapkit/corekit/io.py:328 pads on the wrong side too, but io.BufferedReader's own buffering rules out the same one-character fix.