# 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

```typescript
// 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):

```typescript
// 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`