Reject full-range and wrapping UniformChar samplers on deserialize - #1829
Open
ChrisJr404 wants to merge 1 commit into
Open
Reject full-range and wrapping UniformChar samplers on deserialize#1829ChrisJr404 wants to merge 1 commit into
ChrisJr404 wants to merge 1 commit into
Conversation
The UniformChar deserialization guard checked UniformInt::max(), which is built from wrapping arithmetic, so a payload with range == 0 (the full-range marker) or one whose low + range - 1 wraps cleared the guard and then panicked in sample() on an invalid Unicode scalar value. Check low and range directly with checked arithmetic instead, rejecting range == 0 and any inclusive max that overflows or leaves the char range.
ChrisJr404
force-pushed
the
fix-uniformchar-deser-fullrange
branch
from
August 24, 2026 23:03
a66ccb6 to
454f0ba
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHANGELOG.mdentrySummary
Fixes #1827. The
Uniform<char>deserialization guard checkedUniformInt::max(), which is built from wrapping arithmetic, so a crafted payload could clear it and then panic insidesample. Validatelowandrangedirectly with checked arithmetic instead.Motivation
The guard added in #1790 rejects a sampler whose
max()exceeds the char range, butmax()isrange.wrapping_sub(1).wrapping_add(low). Two cases slip through:{"sampler":{"low":5,"range":0,"thresh":0}} {"sampler":{"low":4294967280,"range":32,"thresh":0}}range == 0isUniformInt's full-u32-range marker, somax()readslow - 1(a small number that passes), and at sample time the full-range branch returns an arbitraryu32, almost always above0x10FFFF. The second payload wrapslow + range - 1down to a small value. Both then hitchar::from_u32(x).expect(...)and panic, which is reachable on attacker-influenced serde input.The check can't move into
UniformInt::deserializebecauserange == 0is a legitimate state forUniform<u32>(e.g.new_inclusive(0, u32::MAX)), so it belongs in the char layer where a bounded range is always expected.Details
lowandrangewithchecked_sub/checked_add, rejectingrange == 0and any max that overflows or exceeds the compressed char range.test_char_bad_deserwith both payloads above, and addedtest_char_deser_roundtripto confirm a normalUniform<char>still serializes, deserializes, and samples.Tested with
cargo test --features serde(all pass);cargo fmt --checkandcargo clippy --all-targets --features serde -- -D warningsare clean.