PCH-02-008 — BinaryReader explicit bounds checks

Severity: Informational Target file: src/crypto/peachPayload.ts:940-969 (BinaryReader) Report: peach-code-review.md, Finding PCH-02-008

Current failure modes (verified by reading the class)

• u8() past end → undefined (then NaN arithmetic downstream)
• u16()/u32() past end → RangeError from DataView (engine-worded, not a Peach error code)
• read(n) past end → silently clamped short subarray
• lpString() → short string from clamped read

The decoded bytes normally sit inside a GCM-authenticated envelope, but the same decoder runs on imported .peach payloads where an attacker who knows the passphrase controls the plaintext (report text). The MV-4 declared-length and cumulative caps (peachPayload.ts:301–311) bound total work before this point; this fix makes the per-read failure mode uniform and self-evidently safe.

Error-code choice (deviation from the brief's sample text)

The brief's sample throws prose messages ("BinaryReader: unexpected end of buffer reading u8"). This file's decode layer throws ALL_CAPS error codes (PAYLOAD_TOO_SHORT, SECTION_LENGTH_MISMATCH, BAD_MAGIC, UNKNOWN_ENTRY_TYPE:*) that the import UI maps to user copy. A truncated buffer mid-entry IS a too-short payload, so the bounds check throws the existing PAYLOAD_TOO_SHORT code — the import UI handles it today with zero UI changes. (Distinct prose codes would surface as the generic error string instead.)

Exact replacement — BinaryReader class

// REPLACE the whole class (peachPayload.ts:940-969) with:

class BinaryReader {
  cursor: number;
  private view: DataView;
  constructor(private data: Uint8Array, start: number) {
    this.cursor = start;
    this.view = new DataView(data.buffer, data.byteOffset, data.byteLength);
  }
  // PCH-02-008: every read validates the cursor stays inside the buffer. The
  // payload normally arrives through a GCM-authenticated envelope, but the
  // same decoder runs on imported `.peach` files where the plaintext is
  // attacker-shaped (given the passphrase); a section truncated mid-entry
  // must fail with the same PAYLOAD_TOO_SHORT code the section parser uses,
  // never with undefined/NaN reads or engine RangeErrors.
  private ensure(n: number): void {
    if (this.cursor + n > this.data.length) throw new Error("PAYLOAD_TOO_SHORT");
  }
  u8(): number {
    this.ensure(1);
    return this.data[this.cursor++];
  }
  u16(): number {
    this.ensure(2);
    const v = this.view.getUint16(this.cursor, true);
    this.cursor += 2;
    return v;
  }
  u32(): number {
    this.ensure(4);
    const v = this.view.getUint32(this.cursor, true);
    this.cursor += 4;
    return v;
  }
  read(n: number): Uint8Array {
    this.ensure(n);
    const out = this.data.subarray(this.cursor, this.cursor + n);
    this.cursor += n;
    return out;
  }
  lpString(lenSize: 1 | 2): string {
    const len = lenSize === 1 ? this.u8() : this.u16();
    return new TextDecoder().decode(this.read(len));
  }
}

Notes:

• ensure uses plain number addition; the largest n is a u32 blob length (≤ 2³²−1), far inside float53 — no overflow hazard.
• view construction is unchanged: it is already bounded to the data subarray (byteOffset/byteLength), so a reader can never see sibling bytes of a shared ArrayBuffer.
• decodeEntry's cursor: reader.cursor return contract is unchanged.

Test seam — export the class

BinaryReader is module-private and only reachable end-to-end through a full encrypted envelope, which cannot express "truncated mid-entry" without also failing the section-length check first. Export it for direct regression coverage (same precedent as the initHash/hkdf2 exports added for the PCH-01-002 KATs):

// BEFORE (peachPayload.ts:940)
class BinaryReader {

// AFTER
// Exported for the PCH-02-008 bounds-check regression tests only — not part
// of the payload API surface.
export class BinaryReader {

Tests

peachPayload.binaryreader-bounds.test.ts (in this folder) — 8 tests: each read method past-end throws PAYLOAD_TOO_SHORT (u8/u16/u32/read/lpString, including a declared length-prefix that overruns), valid reads at exact boundaries still succeed, cursor advances correctly (regression), and a non-zero start offset is respected. Destination: src/crypto/peachPayload.binaryreader-bounds.test.ts.

Existing round-trip coverage (peachPayload.test.ts, peachPayload.bomb.test.ts) regression-guards the happy decode path end-to-end.

Save message (integration)

fix: PCH-02-008 BinaryReader explicit bounds checks on every read — uniform PAYLOAD_TOO_SHORT on truncation