Skip to content

fix: do not panic on a prime being 1 when loading a secret key#624

Merged
dignifiedquire merged 2 commits into0-9-xfrom
0-9-fix-invalid-prime
Jan 6, 2026
Merged

fix: do not panic on a prime being 1 when loading a secret key#624
dignifiedquire merged 2 commits into0-9-xfrom
0-9-fix-invalid-prime

Conversation

@dignifiedquire
Copy link
Copy Markdown
Member

@dignifiedquire dignifiedquire commented Jan 6, 2026

@dignifiedquire dignifiedquire changed the title fix: do not panic on a prime being 1 fix: do not panic on a prime being 1 when loading a secret key Jan 6, 2026
Comment thread src/key.rs
@@ -391,7 +391,7 @@ impl RsaPrivateKey {
let mut m = BigUint::one();
for prime in &self.primes {
// Any primes ≤ 1 will cause divide-by-zero panics later.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment could perhaps say that 0 and 1 are not prime numbers, but...

@dignifiedquire dignifiedquire merged commit 2926c91 into 0-9-x Jan 6, 2026
9 checks passed
@dignifiedquire dignifiedquire deleted the 0-9-fix-invalid-prime branch January 6, 2026 14:58
mbfm added a commit to eclipse-opendut/opendut that referenced this pull request Jan 7, 2026
tarcieri pushed a commit that referenced this pull request May 1, 2026
…#692)

Per @tarcieri's reply on #690:

> The fix for the logic is there, however it looks like the regression
test wasn't carried over and probably should be.
> As it were, the entire implementation diverged as we moved from
`num-bigint` to `crypto-bigint`.

The production fix lives on master already (`validate_private_key_parts`
rejects any `prime <= one` with `Error::InvalidPrime`,
src/key.rs:760-763). What was missing was the regression test added
alongside it in upstream commit `2926c91bef` (PR #624). This PR ports
just that test.

## Adaptations vs the original test

- **Type swap**: original used `num-bigint::BigUint` constructors
(`BigUint::from_u64`, `BigUint::zero()`); ported to
`crypto-bigint::BoxedUint::from(u64)` since that's what current master's
`from_components` API takes.
- **API path**: the original numeric inputs (`n=239, e=185, d=0,
primes=[1, 239]`) include an `e` below master's `MIN_PUB_EXPONENT`
bound, so the test calls `from_components_with_large_exponent` (gated
`#[cfg(feature = "hazmat")]`, matching the existing
`test_from_components_with_small_exponent` /
`test_from_components_with_large_exponent` neighbors) rather than
`from_components`. Ordering inside `validate_skip_exponent_size` ->
`validate_private_key_parts` still hits the `prime <= one` check first,
so we exercise exactly the path the original test did.
- **Assertion**: `Err(Error::InvalidPrime)` (not a panic) — same intent
as the original.

No production-code changes. Single-file diff in `src/key.rs`.

Refs: GHSA-9c48-w39g-hm26, #690, #624, upstream `2926c91bef`.

Co-authored-by: vulgraph <[email protected]>
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.

2 participants