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 525a44d..0348e3a 100644 --- a/tests/index.js +++ b/tests/index.js @@ -233,6 +233,78 @@ const tests = [ var umd = require("../umd/index.js"); umd.genSalt().then(done); }, + + // ===== INVALID ROUNDS VALIDATION ===== + + function invalidRoundsNaNProducesWeakHash(done) { + // 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); + assert.throws(() => bcrypt.hashSync("password", malformedSalt), /Invalid/); + 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 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() {