PCH-02-008-binaryreader-bounds.patch.md

1# PCH-02-008 — BinaryReader explicit bounds checks23**Severity:** Informational4**Target file:** `src/crypto/peachPayload.ts:940-969` (`BinaryReader`)5**Report:** peach-code-review.md, Finding PCH-02-00867## Current failure modes (verified by reading the class)89- `u8()` past end → `undefined` (then `NaN` arithmetic downstream)10- `u16()`/`u32()` past end → `RangeError` from DataView (engine-worded, not a11  Peach error code)12- `read(n)` past end → silently clamped short `subarray`13- `lpString()` → short string from clamped `read`1415The decoded bytes normally sit inside a GCM-authenticated envelope, but the16same decoder runs on imported `.peach` payloads where an attacker who knows17the passphrase controls the plaintext (report text). The MV-4 declared-length18and cumulative caps (peachPayload.ts:301–311) bound total work *before* this19point; this fix makes the per-read failure mode uniform and self-evidently20safe.2122## Error-code choice (deviation from the brief's sample text)2324The brief's sample throws prose messages ("BinaryReader: unexpected end of25buffer reading u8"). This file's decode layer throws ALL_CAPS error codes26(`PAYLOAD_TOO_SHORT`, `SECTION_LENGTH_MISMATCH`, `BAD_MAGIC`,27`UNKNOWN_ENTRY_TYPE:*`) that the import UI maps to user copy. A truncated28buffer mid-entry IS a too-short payload, so the bounds check throws the29existing **`PAYLOAD_TOO_SHORT`** code — the import UI handles it today with30zero UI changes. (Distinct prose codes would surface as the generic error31string instead.)3233## Exact replacement — BinaryReader class3435```typescript36// REPLACE the whole class (peachPayload.ts:940-969) with:3738class BinaryReader {39  cursor: number;40  private view: DataView;41  constructor(private data: Uint8Array, start: number) {42    this.cursor = start;43    this.view = new DataView(data.buffer, data.byteOffset, data.byteLength);44  }45  // PCH-02-008: every read validates the cursor stays inside the buffer. The46  // payload normally arrives through a GCM-authenticated envelope, but the47  // same decoder runs on imported `.peach` files where the plaintext is48  // attacker-shaped (given the passphrase); a section truncated mid-entry49  // must fail with the same PAYLOAD_TOO_SHORT code the section parser uses,50  // never with undefined/NaN reads or engine RangeErrors.51  private ensure(n: number): void {52    if (this.cursor + n > this.data.length) throw new Error("PAYLOAD_TOO_SHORT");53  }54  u8(): number {55    this.ensure(1);56    return this.data[this.cursor++];57  }58  u16(): number {59    this.ensure(2);60    const v = this.view.getUint16(this.cursor, true);61    this.cursor += 2;62    return v;63  }64  u32(): number {65    this.ensure(4);66    const v = this.view.getUint32(this.cursor, true);67    this.cursor += 4;68    return v;69  }70  read(n: number): Uint8Array {71    this.ensure(n);72    const out = this.data.subarray(this.cursor, this.cursor + n);73    this.cursor += n;74    return out;75  }76  lpString(lenSize: 1 | 2): string {77    const len = lenSize === 1 ? this.u8() : this.u16();78    return new TextDecoder().decode(this.read(len));79  }80}81```8283Notes:84- `ensure` uses plain number addition; the largest `n` is a `u32` blob length85  (≤ 2³²−1), far inside float53 — no overflow hazard.86- `view` construction is unchanged: it is already bounded to the87  `data` subarray (`byteOffset`/`byteLength`), so a reader can never see88  sibling bytes of a shared ArrayBuffer.89- `decodeEntry`'s `cursor: reader.cursor` return contract is unchanged.9091## Test seam — export the class9293`BinaryReader` is module-private and only reachable end-to-end through a full94encrypted envelope, which cannot express "truncated mid-entry" without also95failing the section-length check first. Export it for direct regression96coverage (same precedent as the `initHash`/`hkdf2` exports added for the97PCH-01-002 KATs):9899```typescript100// BEFORE (peachPayload.ts:940)101class BinaryReader {102103// AFTER104// Exported for the PCH-02-008 bounds-check regression tests only — not part105// of the payload API surface.106export class BinaryReader {107```108109## Tests110111`peachPayload.binaryreader-bounds.test.ts` (in this folder) — 8 tests: each112read method past-end throws `PAYLOAD_TOO_SHORT` (u8/u16/u32/read/lpString,113including a declared length-prefix that overruns), valid reads at exact114boundaries still succeed, cursor advances correctly (regression), and a115non-zero `start` offset is respected.116Destination: `src/crypto/peachPayload.binaryreader-bounds.test.ts`.117118Existing round-trip coverage (`peachPayload.test.ts`, `peachPayload.bomb.test.ts`)119regression-guards the happy decode path end-to-end.120121## Save message (integration)122123`fix: PCH-02-008 BinaryReader explicit bounds checks on every read — uniform PAYLOAD_TOO_SHORT on truncation`