cloudflare / cloudflare/quiche
octets: `put_varint_with_len` accepts values that do not fit the requested length
- Dominant language
- Rust
- Stars
- 11.8k
- Forks
- 1.1k
- Avg merge
- 21h 9m
- Merged PRs (30d)
- 6
Description
`OctetsMut::put_varint_with_len()` in octets/src/lib.rs can return `Ok` when the requested
length `len` cannot represent the supplied value `v`. The resulting bytes can decode to
a different value.
https://github.com/cloudflare/quiche/blob/039d2a96126b00c61c409877ab456cfb0b437aea/octets/src/lib.rs#L519-L553
For example, a two-byte QUIC variable-length integer can represent at most
`16_383`, but `put_varint_with_len(16_384, 2)` succeeds and produces `40 00`, which `get_varint` reads back as 0. `put_varint_with_len(256, 1)` writes `00`, which is worse.
It seems that whoever wrote this assumes all callers should give valid inputs, and all the callers in this repo do seem fine.
However, this API is public and other users could unknowingly call it with an unrepresentable value if they ever forget the first 2 bytes get stripped.
So, the fix would be to either:
- Add the assumption in the docstring, or
- Add bounds checks and reject values too big
If you truly want end-to-end safety, maybe put the bounds check in `put_u`.
BTW, the `if self.cap() < len` check is redundant because `put_u` already checks the same thing.
Contributor guide
Research direction
Start in octets/src/lib.rs at OctetsMut::put_varint_with_len() and inspect put_u(), including the existing capacity check. Decide whether the public API should document the valid-value assumption or reject values that do not fit len; done means unrepresentable inputs can no longer silently decode to a different value.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100