Sitelet https://github.com/RustCrypto/formats/pull/2461
Skip to content

tls_codec: enforce the mls vector length limit in DeserializeBytes - #2461

Open
yuxi-liu-wired wants to merge 1 commit into
RustCrypto:masterfrom
yuxi-liu-wired:fix/tls-codec-mls-bytes-length
Open

yuxi-liu-wired wants to merge 1 commit into
RustCrypto:masterfrom
yuxi-liu-wired:fix/tls-codec-mls-bytes-length

Conversation

@yuxi-liu-wired

Copy link
Copy Markdown
Contributor

With the mls feature, variable-length vector lengths are limited to 2^30 - 1. RFC 9420 2.1.2 allows only the 1-, 2- and 4-byte length encodings, and says vectors whose length starts with the 11 prefix (8 bytes) "MUST be rejected". quic_vec.rs enforces this in ContentLength::new, but only the Read path goes through it. DeserializeBytes for ContentLength builds Self(value) directly, so the two paths disagree:

// tls_codec with features ["mls", "std"]; 8-byte length header for 2^30 (minimal for that value)
let mut input = vec![0xc0, 0, 0, 0, 0x40, 0, 0, 0];
input.resize(8 + (1 << 30), 7);

VLBytes::tls_deserialize(&mut input.as_slice())   // Err(InvalidVectorLength)
VLBytes::tls_deserialize_bytes(&input)             // Ok(1073741824-byte vector)

With fewer content bytes than declared, the bytes path reports DecodingError("16 bytes were read but 1073741824 were expected") for VLBytes (and hits the debug_assert_eq! that #2414 removes in debug builds), and EndOfStream for Vec<u8>, instead of InvalidVectorLength. VLByteVec and Vec<T> use the same ContentLength path.

The fix calls ContentLength::new in DeserializeBytes too, so both paths reject the length header itself. Without mls, new is a no-op, so nothing changes there.

Test: mls_length_above_30_bits_is_rejected in tests/decode_bytes.rs (#[cfg(feature = "mls")]) checks VLBytes, VLByteVec and Vec<u8>. It fails on master. The tls_codec.yml matrix passed locally: powerset tests on stable and 1.85, wasm32/thumbv7em builds, derive tests, benches, fuzz build, fmt, clippy. I skipped the i686 job because this machine has no multilib.

This PR was produced by AI agents (Claude) and reviewed by me before submission.

With the `mls` feature, variable-length vector lengths are limited to
2^30 - 1 (RFC 9420 2.1.2: only 1-, 2- and 4-byte length encodings are
valid; the "11" prefix MUST be rejected). The limit was only checked
by ContentLength::new, which the Read path (Deserialize) calls, but
DeserializeBytes for ContentLength built the value directly. So
VLBytes, VLByteVec and Vec<T>::tls_deserialize_bytes accepted an
8-byte length header, decoding a 2^30-byte vector the Read path
rejects, and reporting a truncated header as EndOfStream or a
DecodingError (or a debug_assert panic) instead of InvalidVectorLength.

Route the bytes path through ContentLength::new as well.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant