From c10f059d4222bc30902c36fd90eff6ef854a05bc Mon Sep 17 00:00:00 2001 From: thesmart Date: Sun, 1 Mar 2026 11:21:32 -0800 Subject: [PATCH 1/2] test: Add failing tests for NaN rounds vulnerability These tests currently FAIL, exposing a potential vulnerability where non-numeric round values in salt strings bypass bcrypt's work factor. The vulnerability: - Salt like "$2b$xx$..." causes parseInt to return NaN - NaN bypasses rounds validation (NaN < 4 and NaN > 31 are both false) - 1 << NaN becomes 1 << 0 = 1, reducing bcrypt to a single round - This makes the resulting hash trivially crackable Red Tests added: - invalidRoundsNaNProducesWeakHash: hash contains "$NaN$" - invalidRoundsNaN: fully non-numeric rounds "xx" - invalidRoundsNaNAsync: async version via callback - invalidRoundsPartialNaN: partial non-numeric "1x" **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() --- tests/index.js | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/index.js b/tests/index.js index 525a44d..8940d2f 100644 --- a/tests/index.js +++ b/tests/index.js @@ -233,6 +233,47 @@ const tests = [ var umd = require("../umd/index.js"); umd.genSalt().then(done); }, + + // ===== INVALID ROUNDS VALIDATION ===== + + function invalidRoundsNaNProducesWeakHash(done) { + // Demonstrates the vulnerability: without validation, NaN rounds + // produces a hash with "NaN" in the rounds field and only 1 bcrypt + // iteration (1 << NaN = 1), making it trivially crackable. + var malformedSalt = "$2b$xx$" + ".".repeat(22); + var hash = bcrypt.hashSync("password", malformedSalt); + // The hash contains "NaN" proving parseInt produced NaN + assert(!hash.includes("$NaN$"), "Expected hash to NOT contain $NaN$"); + done(); + }, + + function invalidRoundsNaN(done) { + // Non-numeric round values like "xx" must be rejected to prevent + // from crafting salts that bypass bcrypt's work factor. + // Without this check, parseInt returns NaN which reduces rounds to 1. + var malformedSalt = "$2b$xx$" + ".".repeat(22); + assert.throws(() => bcrypt.hashSync("password", malformedSalt), /Invalid/); + done(); + }, + + function invalidRoundsNaNAsync(done) { + // Async version: malformed rounds must error via callback, not + // silently produce a weak hash that's trivially crackable. + var malformedSalt = "$2b$xx$" + ".".repeat(22); + bcrypt.hash("password", malformedSalt, function (err) { + assert(err); + assert(/Invalid/.test(err.message)); + done(); + }); + }, + + function invalidRoundsPartialNaN(done) { + // Even partial non-numeric rounds (e.g., "1x") must be rejected. + // parseInt("1") * 10 + parseInt("x") = 10 + NaN = NaN + var malformedSalt = "$2b$1x$" + ".".repeat(22); + assert.throws(() => bcrypt.hashSync("password", malformedSalt), /Invalid/); + done(); + }, ]; function next() { From 0957e2d122d2b57b38fa4b210f77b6b2af3bd731 Mon Sep 17 00:00:00 2001 From: thesmart Date: Sun, 1 Mar 2026 11:22:23 -0800 Subject: [PATCH 2/2] fix: Validate rounds format to prevent NaN bypass The fix validates that rounds contains exactly 2 digits before parsing, rejecting malformed salts with a clear error message. Red tests are now green: - invalidRoundsNaN: fully non-numeric rounds "xx" - invalidRoundsNaNAsync: async version via callback - invalidRoundsPartialNaN: partial non-numeric "1x" - validRoundsLeadingZero: rounds can start with 0 - invalidRoundsZero: 0 rounds are rejected - invalidRounds32: 32 rounds are rejected Updated tests: - invalidRoundsNaNProducesWeakHash: invalid rounds throw --- index.js | 14 +++++++++++--- tests/index.js | 39 +++++++++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index 4a19b3d..3b70cf3 100644 --- a/index.js +++ b/index.js @@ -1075,9 +1075,17 @@ function _hash(password, salt, callback, progressCallback) { return; } else throw err; } - var r1 = parseInt(salt.substring(offset, offset + 1), 10) * 10, - r2 = parseInt(salt.substring(offset + 1, offset + 2), 10), - rounds = r1 + r2, + // Validate rounds format before parsing + var roundsStr = salt.substring(offset, offset + 2); + if (!/^\d{2}$/.test(roundsStr)) { + err = Error("Invalid rounds: " + roundsStr); + if (callback) { + nextTick(callback.bind(this, err)); + return; + } else throw err; + } + + var rounds = parseInt(roundsStr, 10), real_salt = salt.substring(offset + 3, offset + 25); password += minor >= "a" ? "\x00" : ""; diff --git a/tests/index.js b/tests/index.js index 8940d2f..0348e3a 100644 --- a/tests/index.js +++ b/tests/index.js @@ -237,13 +237,11 @@ const tests = [ // ===== INVALID ROUNDS VALIDATION ===== function invalidRoundsNaNProducesWeakHash(done) { - // Demonstrates the vulnerability: without validation, NaN rounds + // Demonstrates vulnerability fix: without validation, NaN rounds // produces a hash with "NaN" in the rounds field and only 1 bcrypt // iteration (1 << NaN = 1), making it trivially crackable. var malformedSalt = "$2b$xx$" + ".".repeat(22); - var hash = bcrypt.hashSync("password", malformedSalt); - // The hash contains "NaN" proving parseInt produced NaN - assert(!hash.includes("$NaN$"), "Expected hash to NOT contain $NaN$"); + assert.throws(() => bcrypt.hashSync("password", malformedSalt), /Invalid/); done(); }, @@ -274,6 +272,39 @@ const tests = [ assert.throws(() => bcrypt.hashSync("password", malformedSalt), /Invalid/); done(); }, + + function validRoundsLeadingZero(done) { + // Zero-padded rounds like "04" must be accepted. + // bcrypt uses 2-digit zero-padded rounds (04-31). + var salt = bcrypt.genSaltSync(4); + assert(salt.startsWith("$2b$04$"), "Expected salt to start with $2b$04$"); + var hash = bcrypt.hashSync("password", salt); + assert(hash.startsWith("$2b$04$"), "Expected hash to start with $2b$04$"); + assert(bcrypt.compareSync("password", hash)); + done(); + }, + + function invalidRoundsZero(done) { + // Rounds "00" is syntactically valid but outside allowed range (4-31). + // Must be rejected to prevent weak hashes. + var zeroRoundsSalt = "$2b$00$" + ".".repeat(22); + assert.throws( + () => bcrypt.hashSync("password", zeroRoundsSalt), + /Illegal number of rounds/, + ); + done(); + }, + + function invalidRounds32(done) { + // Rounds "00" is syntactically valid but outside allowed range (4-31). + // Must be rejected to prevent weak hashes. + var zeroRoundsSalt = "$2b$32$" + ".".repeat(22); + assert.throws( + () => bcrypt.hashSync("password", zeroRoundsSalt), + /Illegal number of rounds/, + ); + done(); + }, ]; function next() {