NaN Round Number Parsing #167

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