NaN Round Number Parsing

[thesmart]

Non-numeric characters in the rounds position (e.g., $2b$xx$...) causes parseInt to return NaN, which bypasses security validation and reduced bcrypt to a single round.

Location

index.js lines 1078-1080:

var r1 = parseInt(salt.substring(offset, offset + 1), 10) * 10,
    r2 = parseInt(salt.substring(offset + 1, offset + 2), 10),
    rounds = r1 + r2,

The Bug

When non-numeric characters are in the rounds position, parseInt returns NaN:

• rounds = NaN + NaN = NaN
• Line 948: if (rounds < 4 || rounds > 31) → PASSES (NaN comparisons are false)
• Line 964: rounds = (1 << rounds) >>> 0 → 1 << NaN = 1 << 0 = 1 round only!

Red tests added to demonstrate issue:

• invalidRoundsNaNProducesWeakHash: hash contains "$NaN$"
• invalidRoundsNaN: fully non-numeric rounds "xx"
• invalidRoundsNaNAsync: async version via callback
• invalidRoundsPartialNaN: partial non-numeric "1x"

The Fix

The fix validates that rounds contains exactly 2 digits before parsing, rejecting malformed salts with a clear error message.

Severity: LOW

In most real-world scenarios, salts come from genSalt() which always produces valid output. The malformed salt would have to come from:

• A buggy application
• Corrupted data
• Deliberately malicious input to compare()

The real value of this fix is:

• Defense in depth - reject invalid input rather than silently degrading
• Fail loudly - better to throw than produce a weak hash
• Correctness - a crypto library should validate all inputs

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions!