Skip to content

fix(jsbn): prevent modInverse hang for zero input#1136

Closed
Kr0emer wants to merge 1 commit intodigitalbazaar:mainfrom
Kr0emer:main
Closed

fix(jsbn): prevent modInverse hang for zero input#1136
Kr0emer wants to merge 1 commit intodigitalbazaar:mainfrom
Kr0emer:main

Conversation

@Kr0emer
Copy link
Copy Markdown
Contributor

@Kr0emer Kr0emer commented Feb 20, 2026

Summary

This PR fixes a hang in jsbn when calling BigInteger#modInverse with a zero value.

When this == 0 and m is odd/non-zero (for example 0.modInverse(3)), bnModInverse() can enter an infinite loop in the while(v.isEven()) path, causing a potential DoS.

Root Cause

v is initialized as this.clone().
If this is zero, v stays zero forever, and right-shifting zero never changes it, so the loop does not terminate.

Fix

Add an early zero-input guard at the entry of bnModInverse():

  • if(this.signum() == 0) return BigInteger.ZERO;

This keeps existing jsbn behavior for non-invertible cases (returning BigInteger.ZERO) and avoids introducing a breaking API change.

Tests

Added a regression test:

  • tests/unit/jsbn.js
  • included in tests/unit/index.js

The new test verifies BigInteger(0).modInverse(3) returns 0 and does not hang.

@sei-vsarvepalli
Copy link
Copy Markdown
Contributor

@davidlehn - would like you to review when possible. The patch seems very reasonable.

@Kr0emer
Copy link
Copy Markdown
Contributor Author

Kr0emer commented Mar 3, 2026

@davidlehn - would like you to review when possible. The patch seems very reasonable.

Thanks @sei-vsarvepalli. The previous PR was submitted to the wrong location. I've resubmitted the fix under the correct advisory page: GHSA-5m6q-g25r-mvwx

@davidlehn please review when you get a chance.

@davidlehn
Copy link
Copy Markdown
Member

Fixed in 1.4.0.
See also: GHSA-5m6q-g25r-mvwx.

@davidlehn davidlehn closed this Mar 24, 2026
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.

3 participants