Fix BoxedUint GCD with mixed-precision zero operands

[tob-joe]

Summary

BoxedUint::gcd does not correctly handle mixed-precision zero operands.

The constant-time path only substitutes a nonzero placeholder for the left operand before calling the nonzero GCD helper. That causes two problems:

• gcd(0_wide, x_narrow) can panic when the final constant-time correction assigns a narrow value into a wider result.
• gcd(x_wide, 0_narrow) can return the wrong value when x has more trailing zero bits than the zero right operand has precision.

The second case occurs because gcd_nz computes a common power-of-two factor using min(lhs.trailing_zeros(), rhs.trailing_zeros()). For a zero BoxedUint, trailing_zeros() is capped by that value's precision, so a narrow zero right operand can cause the result to lose part of the power-of-two factor.

Fix

This changes the BoxedUint GCD wrapper to handle zero operands symmetrically.

For the variable-time path, the wrapper now returns the nonzero operand directly when either side is zero, and returns zero for gcd(0, 0).

For the constant-time path, the wrapper now substitutes one for both zero operands before calling gcd_nz, then conditionally corrects the result with the original operands. This avoids passing a zero right operand into gcd_nz while keeping the output precision a public function of the input precisions.

Regression Tests

The new tests cover:

• gcd(0_wide, x_narrow)
• gcd(x_narrow, 0_wide)
• gcd(x_wide, 0_narrow) where x has more trailing zero bits than the zero operand has precision
• the same right-zero wrong-result case for gcd_vartime
• mixed-precision nonzero operands
• mixed-precision gcd(0, 0)

Verification

cargo test --all-features uint::boxed::gcd::tests::gcd_ -- --nocapture
cargo fmt --all -- --check
cargo clippy --all-targets --all-features -- -D warnings
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.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.02%. Comparing base (6a7935c) to head (9d89865).
⚠️ Report is 19 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1291      +/-   ##
==========================================
- Coverage   91.26%   91.02%   -0.25%     
==========================================
  Files         189      189              
  Lines       22390    22634     +244     
==========================================
+ Hits        20434    20602     +168     
- Misses       1956     2032      +76     

☔ 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]

I think we should be explicit in the size of the BoxedUint of the result, as we are (generally) for other operations, and always return the maximum bit precision between the two operands in this case. For the vartime implementation, this should just mean returning ref.resize(max_precision) instead of using clone. This could use documenting as well.

[andrewwhitehead]

This seems to be the real issue. Previously, if g was zero and smaller than f, then k would max out at the number of bits in g. This could lead to the wrong GCD being computed.

[tob-joe]

oh yeah, apologies for that being unclear! Currently if z holds 0, gcd(z,x) panics if x.precision() < z.precision(), while gcd(x,z) = x >> max(0, x.trailing_zeros() - z.precision())

[andrewwhitehead]

That's from two commits I added to your PR BTW, it's an alternative fix really.