Conversation
VoteStaple::verify inflated attacker-supplied compressed staple bytes with miniz_oxide::decompress_to_vec_zlib, which has no output-size limit, before any signature or content validation. A malicious representative could return a small compressed payload that inflates to an enormous buffer, exhausting client/process memory (a zlib decompression bomb; measured ~1029x amplification). Use decompress_to_vec_zlib_with_limit with a fixed cap sized to the largest legitimate staple bundle, mapping overflow to VoteError::MalformedStaple. Co-authored-by: Ty Schenk <schenkty@users.noreply.github.com>
larseidsvoll
left a comment
larseidsvoll
left a comment
There was a problem hiding this comment.
DoS class addressed: inflate uses decompress_to_vec_zlib_with_limit (8 MiB) and maps overflow to MalformedStaple. Confirm 8 MiB against largest legitimate staple (or document why) before Ready. Draft + Ty lock — no merge.
Document why the MAX_STAPLE_UNCOMPRESSED_BYTES cap is safe: a staple carries one confirmed block set plus at most one vote per representative (votes de-duplicated by issuer), blocks/votes are individually small (block text fields are length-capped), so a realistic staple is low single-digit MB. 8 MiB leaves headroom above any legitimate staple while bounding attacker-forced allocation. No functional change; cap unchanged. Co-authored-by: Ty Schenk <schenkty@users.noreply.github.com>
larseidsvoll
left a comment
larseidsvoll
left a comment
There was a problem hiding this comment.
Cap rationale on MAX_STAPLE_UNCOMPRESSED_BYTES addresses prior COMMENT; inflate limit + bomb tests still look correct. Draft + Ty lock — no merge.
larseidsvoll
left a comment
larseidsvoll
left a comment
There was a problem hiding this comment.
Verdict: APPROVE (re-stamp after #45)
Tip moved for main merge (rustls / RUSTSEC-2026-0285). Diff vs prior APPROVE unchanged: 8 MiB zlib inflate cap + bomb tests.
Security Audit green on this tip. Lint green; Tests still running.
Review only — do not bot-merge. Humans merge. No @ humans.
|
|
No security issues found. Tip |
3 participants
Summary
VoteStaple::verifydecompresses attacker-supplied compressed staple bytes withminiz_oxide::inflate::decompress_to_vec_zlib, which has no output-size limit, and does so before any signature or content validation. Because staples are decoded from remote representative responses (keetanetwork-clientdecode_staple/decode_staples, history pages) — and the representative is not fully trusted — a malicious or compromised peer can return a small compressed payload that inflates to an enormous buffer, exhausting client/process memory (a classic zlib decompression bomb).Measured amplification with the pinned
miniz_oxide 0.8.9: a ~200 KiB compressed input inflates to ~200 MiB (~1029x), scaling linearly (≈10 MB → ≈10 GB). No client-side response-body cap limits the compressed input.This is the same class of issue reported as HIGH in the sibling TypeScript node (unbounded zlib decompression of untrusted input), verified independently here.
Fix
Decompress under a fixed output cap using
decompress_to_vec_zlib_with_limit, mapping overflow to the existingVoteError::MalformedStaple. One-line behavior change toinflate, no public API change.Why the 8 MiB cap is safe (does not reject a legitimate staple)
A staple's canonical form is
SEQUENCE { blocks SEQUENCE OF OCTET STRING, votes SEQUENCE OF OCTET STRING }. It endorses one confirmed block set and carries at most one vote per representative (votes are de-duplicated by issuer invalidate_vote_invariants). Blocks are small — their text fields are individually length-capped (e.g. the 1024-byteexternalfield inkeetanetwork-blockvalidation) and a block serializes to a few KB; a vote certificate is an X.509-shaped record of similar order. So even a large round (hundreds of blocks and hundreds of representative votes at a few KB each) stays in the low single-digit megabytes.8 MiBtherefore leaves comfortable headroom above any realistic staple while bounding attacker-forced allocation by ~3 orders of magnitude versus the previously-unbounded path. The repository does not define a hard protocol maximum staple size; the cap is left at 8 MiB (per review, unchanged absent evidence it's wrong). If a precise protocol maximum is later established, tighten the constant to it. The rationale is captured in a doc comment onMAX_STAPLE_UNCOMPRESSED_BYTES.Tests
cargo test -p keetanetwork-vote— all pass, including:test_inflate_rejects_decompression_bomb— a tiny compressed input that would exceed the cap is rejected asMalformedStaplerather than allocated.test_inflate_accepts_within_cap— normal payloads still round-trip.Trace
keetanetwork-vote/src/staple.rsinflate(wasdecompress_to_vec_zlib, no cap)VoteStaple::verify(inflates before verification) ←keetanetwork-client/src/codec.rsdecode_staple← remote representative responses.Severity: medium (client/process memory-exhaustion DoS). The running node server that also parses staples is in a separate repository and would warrant the same fix there.
Draft — audit fix; do not merge without maintainer review.