docs(security): document the ByteStorage decompression bound (LAB-2504)

SECURITY.md

@@ -49,7 +49,7 @@ This crate protects against:
 - **Data tampering**: GCM authentication tags (when encryption enabled); xxHash3 detects accidental corruption only
 - **Data disclosure**: AES-256-GCM encryption (when enabled)
 - **Key compromise isolation**: HKDF domain separation per tenant
-- **Decompression bombs**: Size limits + ratio validation
+- **Decompression bombs**: Size limits + ratio validation (see [Decompression limits](#decompression-limits))
 - **Memory disclosure**: `zeroize` on drop for key material
 
 This crate does **not** protect against:
@@ -59,6 +59,128 @@ This crate does **not** protect against:
 - Denial of service via resource exhaustion (partial protection only)
 - Attacks requiring physical access
 
+### Decompression limits
+
+`StorageEnvelope::extract` bounds LZ4 decompression **before** calling
+`lz4_flex::decompress`, so a forged envelope cannot expand without limit:
+
+| Limit | Value | Enforced on |
+|:------|:------|:------------|
+| `MAX_COMPRESSED_SIZE` | 512 MiB | `compressed_data.len()` |
+| `MAX_UNCOMPRESSED_SIZE` | 512 MiB | declared `original_size` |
+| `MAX_COMPRESSION_RATIO` | 1000:1 | `original_size` vs `compressed_data.len()` |
+
+The same `MAX_COMPRESSED_SIZE` constant also bounds the *serialized* envelope
+before MessagePack deserialization — but that check lives in
+`ByteStorage::retrieve` and `ByteStorage::validate`, not on `StorageEnvelope`.
+`StorageEnvelope` is public with public fields, so a caller who deserializes it
+directly gets no such bound and must impose one.
+
+The ratio product is computed in `u64` via `checked_mul` (overflow is treated
+as a bomb — though the 512 MiB compressed-size check above already puts the
+product near 2^39, so that branch is belt-and-braces rather than a live
+defense), and zero-length compressed data is rejected outright regardless of
+what `original_size` claims — the check is unconditional, so an envelope
+declaring `original_size == 0` is rejected on the same branch rather than
+decompressing to an empty result. `lz4_flex` returns `OutputTooSmall` rather
+than growing past the allocation, so the decompressed output is bounded by
+`min(512 MiB, 1000 × compressed_data.len())` regardless of what the envelope
+claims — a property of `lz4_flex`'s default `safe-decode` path, which this
+crate must not opt out of (`default-features = false` moves bounds enforcement
+into the separate `checked-decode` feature and swaps the fixed-length buffer
+for `with_capacity` + `set_len`),
+and `extract` re-checks the produced length afterwards — a decompressor's
+size argument sizes a buffer, it never asserts the decoded length.
+
+- **`original_size` does not act as a bound.** It is attacker-controlled on any
+  backend an attacker can write to. It *does* size the allocation, but only
+  within the absolute and ratio limits already checked above — so a forged
+  envelope can still make a reader allocate up to `1000 × compressed_data.len()`
+  before the LZ4 stream is validated. That allocation amplification is the
+  sizing question in LAB-2505, not a bypass of the bound. Note that
+  `ByteStorage::validate()` reaches the same allocation — it calls `extract()`
+  and discards the result — so despite its name and its "validate envelope
+  without extracting data" doc comment it is *not* a cheap structural
+  pre-screen for untrusted envelopes.
+- **xxHash3-64 is not a control here.** It is unkeyed, so anyone who can
+  forge an envelope recomputes it. It detects accidental corruption, not
+  forgery. Authentication comes from AES-256-GCM, and only for secure caches.
+
+**The ceiling is server-class.** 512 MiB assumes a host that can absorb a
+512 MiB allocation. It does *not* prevent an out-of-memory kill in a
+constrained runtime — a Cloudflare Workers isolate has ~128 MiB, so a payload
+well inside these limits can still exhaust it, and on `wasm32` the allocation
+is an eager `memory.grow` that needs no valid LZ4 stream behind it. On `wasm32`
+that is worse than a spike: linear memory never shrinks, so a single large
+extract permanently raises the isolate's floor for every subsequent request it
+serves. Deployments on constrained runtimes must bound payload size at the
+caller. Making these constants environment-aware or configurable is tracked in
+LAB-2505.
+
+These properties are exercised by the `compression_bomb` fuzz target
+(`fuzz/fuzz_targets/compression_bomb.rs`) — but read its assertions before
+crediting them:
+
+- It asserts `extract` never panics. That one is unconditional and real.
+- It asserts the output never exceeds 512 MiB. Vacuous in this target: with
+  `compressed_size` a `u16`, compressed input caps at 64 KiB, so the ratio
+  bound already holds output under ~62.5 MiB. The assertion cannot fire.
+- It asserts rejections are one of `DecompressionBomb`, `InputTooLarge`, or
+  `DecompressionFailed` — but **only** for inputs that already violate the
+  declared-size or ratio limit. Both variant checks sit behind guards. Every
+  other rejection is unconstrained, and `extract`'s two remaining failure
+  variants, `ChecksumMismatch` and `SizeValidationFailed`, are never asserted
+  against at all.
+
+On which bounds it reaches: `compressed_size` is a `u16`, so the 512 MiB
+`MAX_COMPRESSED_SIZE` boundary is never approached. `original_size` is a `u32`,
+which does range past 512 MiB, so the target does reach `MAX_UNCOMPRESSED_SIZE`'s
+*rejection* branch. It exercises the **ratio** bound and that rejection path —
+not the compressed-size limit, and not the output bound.
+
+Be precise about how much CI coverage that buys: the `quick-fuzz` matrix in
+`.github/workflows/security.yml` runs on pull requests and on pushes to `main`
+(not on feature-branch pushes), and `fuzz/.gitignore` excludes `corpus/*/`, so
+on a fresh checkout the corpus is empty. libFuzzer seeds an empty corpus with a
+single newline input and executes it before the run-limit check, so `-runs=0`
+is not literally zero executions — but it performs no mutation, so the job
+generates no inputs of its own. At PR time it is therefore **single-seed smoke
+coverage**: it will catch a build break or a panic on that one newline input,
+and nothing else. It produces no boundary coverage; the merge-time coverage for
+the boundary cases is the unit tests below.
+
+Input coverage comes from the weekly deep-fuzz run — though that job does not
+persist its corpus either (it uploads `fuzz/artifacts/` only, and the cache key
+covers `fuzz/target/`), so each week restarts cold from the same seed and
+coverage does not accumulate — and from the unit tests in `src/byte_storage.rs`,
+which call `extract` directly. Those unit tests are the only thing in this repo
+that executes the bound at merge time.
+
+**The Kani proofs are weaker than they look, and are not a merge-time gate.**
+The `kani` job runs only on `schedule` and `workflow_dispatch`, never on a pull
+request. More importantly, none of the four size/ratio harnesses can fail on a
+wrong predicate — though they get there by two different routes:
+
+- `verify_input_size_limits` and `verify_compressed_size_limits` are literal
+  self-comparisons: `let exceeds_limit = size > MAX_UNCOMPRESSED_SIZE; let
+  should_reject = size > MAX_UNCOMPRESSED_SIZE; assert_eq!(exceeds_limit,
+  should_reject);`. That holds for any predicate, and would still pass with the
+  comparison inverted or the constant wrong.
+- `verify_decompression_bomb_protection` compares the `checked_mul` result
+  against the same product computed directly — equal by construction, and its
+  `kani::assume(compressed_size <= 1000)` makes the multiplication infallible,
+  so the overflow branch it exists to check is unreachable and stubbed
+  `assert!(true)`.
+- `verify_compression_ratio_calculation_safety` derives `is_bomb = original_size
+  > max_allowed` and then asserts that same comparison in both branches of
+  `if original_size <= max_allowed`, which restates its own definition.
+
+What Kani does buy is its default check set — no panic, no arithmetic overflow —
+over the harness bodies. What it does not buy is any evidence that the predicates
+are the *right* ones, and it never executes `StorageEnvelope::extract`, so it
+cannot catch a divergence between the modelled predicate and the shipped one.
+Treat these as smoke checks, not as verification of the bound.
+
 ### Dependencies
 
 Security-critical dependencies are audited via `cargo-deny`: