Skip to content

Handle power-of-two moduli in mul_mod_special#1282

Open
tob-joe wants to merge 1 commit into
RustCrypto:masterfrom
tob-joe:fix-mul-mod-special-zero-c
Open

Handle power-of-two moduli in mul_mod_special#1282
tob-joe wants to merge 1 commit into
RustCrypto:masterfrom
tob-joe:fix-mul-mod-special-zero-c

Conversation

@tob-joe

@tob-joe tob-joe commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Closes #1281.

mul_mod_special computes multiplication modulo MAX + 1 - c, i.e. 2^bits - c. When c = 0, that modulus is 2^bits, so reduction is ordinary fixed-width wrapping multiplication.

The one-limb fixed and boxed implementations reached a limb reduction path which constructs NonZero<Limb> from 0 when c = 0, causing a panic. This adds an early wrapping_mul return for the power-of-two modulus case and regression tests for both fixed and boxed integers.

Testing:

  • cargo test --all-features mul_mod_special_zero_c_is_wrapping_multiplication -- --nocapture
  • cargo test --all-features uint::mul_mod::tests -- --nocapture
  • cargo test --all-features uint::boxed::mul_mod::tests -- --nocapture
  • cargo fmt --all -- --check
  • git diff --check

This work was completed by Trail of Bits as part of the Patch The Planet project in collaboration with OpenAI. The vulnerability was identified primarily by the Codex coding agent, and manually reviewed before submission.

For mul_mod_special, c = 0 represents the modulus 2^bits. Multiplication modulo that value is ordinary wrapping multiplication.

Return wrapping_mul for this case before the one-limb reduction paths attempt to construct NonZero<Limb> from zero, and cover both fixed and boxed integer implementations.

Co-authored-by: GPT 5.5 <gpt-5.5@openai.com>
@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.02%. Comparing base (3f7dede) to head (441d3a5).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1282      +/-   ##
==========================================
+ Coverage   91.01%   91.02%   +0.01%     
==========================================
  Files         189      189              
  Lines       22160    22186      +26     
==========================================
+ Hits        20169    20195      +26     
  Misses       1991     1991              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andrewwhitehead

andrewwhitehead commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

These methods are not marked as vartime, I don't think it's appropriate to be short circuiting based on the value of the modulus. They should probably accept a NonZero<Limb> instead, although that would be a breaking change.

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.

mul_mod_special panics for c = 0 in one-limb implementations

2 participants