Skip to content

Commit 06d9679

Browse files
committed
* Unbounded CBOR Materialization in CWT Metadata Decoding
** Classification - **Type:** Denial of service - **Severity:** Medium - **Confidence:** Certain - **Weakness:** Uncontrolled resource consumption ** Affected Locations - `src/cwt_token.rs:68` - `CWTToken::decode_metadata` - `CWTToken::verify` ** Summary Both CWT entry points deserialize the complete attacker-supplied token into a `CBORValue` before enforcing any structural limit. A compact CBOR array containing millions of one-byte values therefore causes disproportionate memory allocation and decoding work before the subsequent four-element check rejects it. `CWTToken::decode_metadata` takes no `VerificationOptions`, so the library's one-million-byte default never applied to it at all. `CWTToken::verify` does honor `max_token_length`, but that only helps applications that leave the default in place. ** Provenance Identified by the [Swival.dev Security Scanner](https://swival.dev), independently reproduced, and patched against the affected source. ** Preconditions - An application passes attacker-supplied CWTs to `CWTToken::decode_metadata`, directly or through `decode_cwt_metadata` on a key. - Or it calls `verify_cwt_token` with `max_token_length` raised or set to `None`, which is exactly what an application handling large binary CWTs has to do. ** Proof A release-mode reproducer supplied two shapes: 1. **Flat.** CBOR tag 18 (`0xd2`), a definite-length array with a 32-bit element count (`0x9a`), then N one-byte CBOR `null` values. 2. **Nested.** A well-shaped four-element envelope (`0xd2 0x84 0x40`) whose unprotected header is the same oversized array. Measured against the unpatched source with a counting global allocator: | Shape | Elements | Input size | Time | Allocated | | ------ | ---------- | ---------- | ------ | --------- | | flat | 5,000,000 | 5,000,006 | 75 ms | 268 MB | | flat | 50,000,000 | 50,000,006 | 979 ms | 2.15 GB | | nested | 5,000,000 | 5,000,010 | 83 ms | 268 MB | | nested | 50,000,000 | 50,000,010 | 778 ms | 2.15 GB | Cost is linear in the element count. The amplification factor is about 43x, consistent with `ciborium::value::Value` occupying 32 bytes per element plus vector growth. The nested shape reaches the same figures through `verify` with `max_token_length: None`, so a token-size cap alone does not close it. The state-amplification ledger is: 1. **Attacker unit:** Each compact, one-byte CBOR array element. 2. **Amplified state:** One materialized `CBORValue`, vector storage, and decoding work per element. 3. **Intended limit:** `MAX_CWT_HEADER_LENGTH` is 4096 bytes and already bounds everything in the envelope except the payload. 4. **Limit gap:** That check ran only after the whole token had been materialized. 5. **Timing:** The complete attacker-sized array is materialized before `parts_cbor.len() == 4` and the header-length check reject it. ** Why This Is A Real Bug The rejection occurs only after attacker-controlled state has been fully materialized, so malformed input still consumes substantial CPU and memory. Concurrent requests or larger accepted bodies can cause memory pressure, OOM termination, and request-processing unavailability. ** Fix Requirement Bound the work before CBOR deserialization runs, without capping the payload. The obvious fix, rejecting tokens above `DEFAULT_MAX_TOKEN_LENGTH`, is the wrong one here and an earlier revision of this report proposed it. CWT is a binary format and a token carrying multiple gigabytes of claims is legitimate; a one-million-byte cap on `decode_metadata` breaks those applications outright, and it cannot be configured away because the function takes no options. It would also make metadata decoding stricter than verification, since `verify` accepts anything the caller's `max_token_length` allows. Worse, it leaves the real exposure open: an application that legitimately handles large CWTs must set `max_token_length: None`, and at that point `verify` will happily materialize a two-gigabyte bomb. The distinction the fix has to draw is not large versus small, but payload versus framing. A large payload is one byte string and materializes 1:1. A large *envelope* is where the amplification lives, and the code already declares its intended bound for that: `MAX_CWT_HEADER_LENGTH`, 4096 bytes, checked as `token_len - payload_len`. ** Patch Rationale `scan_cwt_envelope` walks the COSE framing directly over the token bytes, before `from_cbor` runs, and returns the payload length. `ensure_cwt_header_budget` computes `header_len` from that and applies the existing 4096-byte check up front instead of after the fact. The scan validates the tag, requires a four-element array, skips the two header buckets, reads the payload head, and skips the signature slot that follows it. It allocates nothing and decodes nothing. Both entry points call the same helper, so `verify` is protected regardless of what `max_token_length` is set to. The signature slot is walked rather than left to the subtraction, because `header_len` only accounts for bytes that are actually present. Without that step, `d2 84 40 a0 40 9a 004C4B40` is a ten-byte token announcing five million elements in a slot the budget then waves through, and it is harmless only because ciborium happens to grow sequences incrementally instead of preallocating from the announced length. Depending on an unstated allocation policy in a dependency is not a bound. Payload size is deliberately not bounded. Before authentication, the only thing `ciborium` can materialize beyond a 1:1 copy of the payload is the 4096 bytes of framing, so the amplification ceiling is roughly 128 KB. Three properties keep the scan itself from becoming the new CPU sink: - Every item the walk accepts consumes at least one byte, so the item count can never exceed the byte count. Announced element counts are not trusted; a header claiming 2^64-1 elements simply runs out of input. - Each slot is walked through a `MAX_CWT_HEADER_LENGTH`-sized window. Any token that would pass the header-length check fits inside one, so the windows cost nothing for well-formed input while capping hostile input at about 4096 item visits. - Nesting is capped at `MAX_CWT_DEPTH`, which is 16. The decode goes through `from_reader_with_recursion_limit` with the same constant, so the scan and the decoder share one limit by construction rather than the scan mirroring ciborium's private default and hoping it does not move. Sixteen is far more than any real CWT needs, and it is a quarter of the worst-case scan cost that ciborium's default of 256 allowed. Indefinite-length encodings stay accepted, since `ciborium` accepts them today and removing that would be its own breaking change. Chunked strings are handled by counting each chunk's head overhead against the header budget, which bounds that loop at about 4096 chunks as well. Additional info 31 is only honored for major types 2 through 5, per RFC 8949; on any other major type it is malformed CBOR and the scan says so rather than guessing at an item length. ** Verification `cargo test --features cwt,jwe` and `cargo test --no-default-features --features pure-rust,cwt,jwe`: 72 unit tests and 30 doctests pass on both backends. `cargo fmt --check` is clean and the clippy warning count is unchanged at 15 pre-existing warnings. Attack shapes, same reproducer and allocator as above: | Shape | Input size | Before | After | | -------------------- | ---------- | ---------------- | ------------- | | flat, 5M elements | 5,000,006 | 75 ms / 268 MB | 46 ns / 88 B | | flat, 50M elements | 50,000,006 | 979 ms / 2.15 GB | 27 ns / 88 B | | nested, 5M elements | 5,000,010 | 83 ms / 268 MB | 6.7 us / 88 B | | nested, 50M elements | 50,000,010 | 778 ms / 2.15 GB | 5.7 us / 88 B | The residual 88 bytes is the error value itself. Scan cost against inputs built to maximize the walk, all rejected: | Shape | 4 KB | 64 KB | 8 MB | | --------------------------------------------- | ------ | ------ | ------ | | unprotected header announcing 2^64-1 elements | 7.4 us | 4.9 us | 4.8 us | | 14 levels deep, repeated to fill the window | 8.2 us | 6.6 us | 6.7 us | | indefinite payload in one-byte chunks | 3.6 us | 2.5 us | 2.8 us | Cost is flat in input size, which is the property that matters: sending more bytes buys no more scan work. The 8.2 us figure is the worst case found, and it is the window walk running its full 4096 items just under the depth limit. At ciborium's default depth of 256 the same shape cost 24 us, which is what `MAX_CWT_DEPTH` being 16 buys. No regression on well-formed tokens: | Payload | Before | After | | ------- | -------- | -------- | | 64 B | 409 ns | 391 ns | | 10 KB | 768 ns | 766 ns | | 1 MB | 30.36 us | 29.28 us | | 20 MB | 1.566 ms | 1.519 ms | Large tokens still work, which the earlier size-cap patch would have broken. A 4,000,037-byte HS256 CWT, four times `DEFAULT_MAX_TOKEN_LENGTH`, decodes its metadata as `HS256` and verifies with `max_token_length: None`. The committed version of that test uses 1.1 MB, which clears the same threshold for a quarter of the runtime. Four regression tests accompany the patch: - `large_cwt_payload_is_accepted`, an oversized token that decodes and verifies. - `envelope_scan_rejects_hostile_shapes`, one shape per guard in the scan: outer array arity, an announced count in the unprotected header, nesting depth, chunked-payload overhead, and the ten-byte token described above. Each shape is sized from the constant its guard uses, so the rows track the limits rather than restating them. - `envelope_scan_accepts_indefinite_length_encodings`, pinning that no legal CBOR encoding was dropped. - `envelope_scan_depth_limit_matches_the_decoder`, which walks nesting depths across the limit and asserts the scan never rejects what the decoder accepts. Measured on the depth shape, the scan accepts 12 levels and rejects 32, so that row is discriminating rather than incidentally failing on truncation. ** Behavior Changes - Malformed envelopes now fail with `JWTError::CWTDecodingError` from the scan rather than with a `ciborium` decoding error. The error type is the same `anyhow::Error` either way, and the variant is the one the surrounding code already uses for this. - The header-length check now runs before the payload is decoded rather than after, so a token that is both malformed and oversized reports the header-length failure first. - CBOR nesting is capped at 16 levels, down from the 256 that ciborium allowed by default. This applies to every decode in the CWT path: the envelope, both protected headers, and the claims payload. Measured against the envelope shape, 13 levels of nesting inside a header bucket still decode, since the tag, the outer array and the bucket map account for the other three. Anything deeper than that in a real CWT is a bug or an attack, but callers with pathologically nested custom claims would see them rejected. - No signature change, no new option, nothing removed. ** Residual Risk The payload's *contents* are still parsed into a full `CBORValue` tree with no structural bound. A 5 MB payload holding a compact array of five million one-byte elements materializes the same ~268 MB as the original bug, just wrapped in a byte string the scan waves through by design. This sits behind `authentication_or_signature_fn`, so it takes a valid key and is amplification by an authenticated caller rather than an open door. Closing it would mean bounding the claims map itself, which is a policy decision about legitimate CWT payload shapes, not a framing one. An application that accepts gigabyte CWTs also pays a 1:1 copy of the payload during decoding, plus two more copies inside `verify`, which re-serializes the Sig_structure. That is inherent to the current design and applies equally to well-formed tokens; bounding it means streaming verification, a much larger change. Two related notes, neither of which is exposure today: - The scan rejects nested indefinite-length byte strings (`5f 5f 41 2a ff ff`), which `ciborium` accepts. RFC 8949 section 3.2.3 forbids them and no encoder emits them, so this is strictness in the safe direction, but it is the one shape where the scan is narrower than the decoder. - `ciborium-ll` exposes `Decoder::pull() -> Header`, which is the same data model as the hand-written head parser and would make the scan and the decoder share one implementation. It is already compiled as a transitive dependency but is not re-exported by `ciborium`, so using it means naming it in `Cargo.toml`. That is a dependency decision, not a cleanup.
1 parent acdfbcf commit 06d9679

1 file changed

Lines changed: 354 additions & 17 deletions

File tree

0 commit comments

Comments
 (0)