Uniform<char> serde guard is bypassable, letting sample() panic

[hey-jj]

The deserialization guard added for Uniform<char> rejects a sampler whose computed max() exceeds the char range, but max() is built from wrapping arithmetic. A crafted serde payload with range: 0 and a small low passes the guard and then panics inside sample, on both debug and release builds.

Reproducer

rand 0.10.2 with the serde feature, plus rand_pcg and serde_json.

use rand::distr::{Distribution, Uniform};
use rand::SeedableRng;

fn main() {
    // range == 0 is the internal full-range marker. With low = 5,
    // max() = 0u32.wrapping_sub(1).wrapping_add(5) = 4, which passes the guard.
    let d: Uniform<char> =
        serde_json::from_str(r#"{"sampler":{"low":5,"range":0,"thresh":0}}"#).unwrap();
    let mut rng = rand_pcg::Pcg64Mcg::seed_from_u64(1);
    for _ in 0..1000 {
        let _c = d.sample(&mut rng);
    }
    println!("no panic");
}

Observed

thread 'main' panicked at .../rand-0.10.2/src/distr/uniform_other.rs:103:14:
rand::distr::uniform::UniformChar: invalid Unicode scalar value (likely memory corruption)

Deserialization succeeds and the first sample call panics. The same payload panics under cargo run --release.

Expected

Deserialization should reject a sampler that can yield a value outside the char range, the way it already rejects {"low":4294967200,"range":0,"thresh":0}. A Uniform<char> that passes deserialization should sample a valid char without panicking.

Root cause

src/distr/uniform_other.rs:46 guards on sampler.max():

if sampler.max() > char::MAX as u32 - CHAR_SURROGATE_LEN {
    return Err(serde::de::Error::custom("bad sampler range for UniformChar"));
}

UniformInt::max() at src/distr/uniform_int.rs:81 is range.wrapping_sub(1).wrapping_add(low). When range is 0, that marker means full u32 range at sample time, but max() computes low - 1, a small number that clears the guard. At sample time the full-range branch returns rng.random::<u32>(), almost always above 0x10FFFF, so char::from_u32(x).expect(...) panics at src/distr/uniform_other.rs:103. A second payload, {"low":4294967280,"range":32,"thresh":0}, makes low + range - 1 wrap so max() reads 0x0F and also clears the guard, panicking on the add-overflow in debug and producing out-of-distribution chars in release.

Scope

Any code path that deserializes an attacker-influenced Uniform<char> and then samples from it. The guard came in with the fix for the earlier memory-safety report (PR #1790), and the existing test_char_bad_deser covers only the large-max() payload, so the range: 0 marker and the wrapping cases slip through. The current sample path uses the safe char::from_u32(...).expect(...), so the observed failure on this version is a reachable panic that aborts the calling thread, a denial of service on attacker-controlled serde input.

[dhardy]

The root cause here appears to be that the Deserialize impl for UniformInt does not validate the result. If only there was an easy way to do this without writing a custom deserializer.

We could hack up a solution with an extra test in the deserializer for UniformChar but I'm somewhat dubious that this is the right approach given that your supposed attacker would still be able to configure your Uniform<char> to yield whatever char the attacker desires.

[hey-jj]

re: the security angle... 100%. Bad framing. You may still consider fixing that Deserialize accepts a value that breaks the invariant, then a safe api panics on plain data.

re: the layer... agree. the check belongs in UniformInt's deserialize.

re: "if only there was an easy way"... a #[serde(try_from = ...)] shadow struct is probably cleanest, or a manual deserialize impl.

Docs option solves it too.

[dhardy]

Sure, this issue can be fixed (PR welcome). But yes, I didn't understand why this was reported as a security issue.