Skip to content

Avoid panic from deserialized Uniform<char> where range == 0 - #1831

Open
dhardy wants to merge 5 commits into
masterfrom
push-qvsoqsoosqmp
Open

Avoid panic from deserialized Uniform<char> where range == 0#1831
dhardy wants to merge 5 commits into
masterfrom
push-qvsoqsoosqmp

Conversation

@dhardy

@dhardy dhardy commented Aug 30, 2026

Copy link
Copy Markdown
Member
  • Added a CHANGELOG.md entry

Summary

Fixes #1827. Closes #1829.

Details

Fixes the internal method UniformInt::max which previously ignored the special cases range == 0 and where low + range overflows.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to prevent panics when sampling a Uniform<char> that has been deserialized from (potentially attacker-controlled) serde data, by tightening the internal maximum-value logic used by the UniformChar deserialization guard.

Changes:

  • Update UniformInt::max to treat range == 0 (full-range marker) as having a maximum of $ty::MAX.
  • Refactor and extend the Uniform<char> bad-deserialization test to cover the range == 0, low = 5 payload.
  • Add a changelog entry documenting the fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/distr/uniform_other.rs Refactors Uniform<char> serde failure test helper and adds coverage for an additional failing payload.
src/distr/uniform_int.rs Adjusts UniformInt::max behavior when range == 0.
CHANGELOG.md Documents the intended panic-avoidance fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/distr/uniform_int.rs
Comment thread src/distr/uniform_other.rs
@dhardy
dhardy force-pushed the push-qvsoqsoosqmp branch from 32adc51 to 8224741 Compare August 30, 2026 09:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/distr/uniform_other.rs:324

  • In builds with --no-default-features --features serde (i.e. without the alloc feature), the col parameter is unused because it’s only referenced inside #[cfg(feature = "alloc")], which can trigger an unused_variables warning in this test.
        fn do_test(json: &str, col: usize) {
            let result = serde_json::from_str::<Uniform<char>>(json);
            assert!(result.is_err());
            let err = result.unwrap_err();
            assert_eq!(err.classify(), serde_json::error::Category::Data);

Comment thread src/distr/uniform_int.rs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

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.

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

2 participants