diff --git a/docs/reviews/consumer-findings-ledger.md b/docs/reviews/consumer-findings-ledger.md index dca8b61..1a96c71 100644 --- a/docs/reviews/consumer-findings-ledger.md +++ b/docs/reviews/consumer-findings-ledger.md @@ -11,6 +11,40 @@ Format: date | found-in (alkhttp context) | severity | status. ## Open +### CF-002 — demux `TooLarge` skip allocates the full peer-declared length up front (up to ~4 GiB from an 8-byte header) (2026-08-30) + +- **Found in:** alkhttp Review 001, finding WS-12 (cross-crate; + `docs/reviews/001-initial-implementation-review.md`, Part B, + verified against tree `4a825d3`). Filed to this ledger 2026-08-30. + `src/channels/adapter.rs:144` (`run_demux_loop_for_client`) — when a + parsed chunk header carries `length > MAX_CHUNK_LEN`, the + `ChunkError::TooLarge` arm allocates + `let mut discard = vec![0u8; length as usize];` **before** reading: + the buffer is sized from the peer's untrusted 8-byte header, and + `length` is a `u32` (`0xFFFFFFFF` ≈ 4 GiB). +- **Impact:** memory-shape DoS, not a correctness bug (the resync/skip + itself is correct — alkcall's own resync test covers it). Any + authenticated peer can send header `[ch][len = 0xFFFFFFFF]` and then + dribble; the allocation is pinned for as long as the payload takes to + arrive (a dribbling peer can hold it indefinitely). Via the alkhttp WS + path this is trivially reachable by any authenticated browser; K + concurrent connections × 4 GiB headers → OOM before the dribble even + matters. The *normal* payload arm (`:159`, `vec![0u8; + header.length as usize]`) is safe — `parse_header` rejects + `length > MAX_CHUNK_LEN`, so it is bounded at 16 MiB; only the + `TooLarge` skip arm is unbounded. +- **Suggested direction:** stream-skip with a bounded fixed buffer + (e.g. a 64 KiB stack/fixed buffer, `read_exact` loop consuming + `length` bytes) instead of the pre-sized allocation. Optionally bound + cumulative skipped bytes per connection before tearing it down, so a + peer cannot loop `TooLarge` headers to burn bandwidth/CPU forever. +- **alkhttp side:** nothing to change there — this is demux-internal. + alkhttp's own read-side hardening (review-001-ws-pump-consolidation: + explicit inbound `max_message_size`/`max_frame_size`, idle-read + timeout) bounds the WS ingress, but a header that survives those caps + still hits this arm, so the fix belongs on the alkcall side. +- **Status:** open — no alkcall change made. + ### CF-001 — `call_single_stream` write-failure maps to non-retryable `INTERNAL` (2026-08-29) - **Found in:** alkhttp `from_wss` drop-monitor race tests (WS-02/CON-02,