From 305d44e9551158795e96ea95d6f7ebe93c792ed2 Mon Sep 17 00:00:00 2001 From: Abhishek Chauhan Date: Fri, 20 Mar 2026 08:37:46 -0500 Subject: [PATCH] fix: validate salt rounds to prevent hang on negative values Negative salt rounds (e.g., -1) caused bcrypt.hash() to hang indefinitely because the C++ layer clamped negative values to 31 (the maximum), resulting in 2^31 iterations that would take hundreds of years to complete. This commit adds input validation to reject negative rounds early with a clear error message. Also fixes a pre-existing bug where genSalt errors in hash() were not properly propagated to the callback. Fixes #1218 Co-Authored-By: Claude Opus 4.6 --- bcrypt.js | 15 +++++++++++++-- test/async.test.js | 18 ++++++++++++++++++ test/promise.test.js | 8 ++++++++ test/sync.test.js | 8 ++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/bcrypt.js b/bcrypt.js index 62da525..5e58ffe 100644 --- a/bcrypt.js +++ b/bcrypt.js @@ -14,6 +14,8 @@ function genSaltSync(rounds, minor) { rounds = 10; } else if (typeof rounds !== 'number') { throw new Error('rounds must be a number'); + } else if (rounds < 0) { + throw new Error('rounds must be a positive integer'); } if (!minor) { @@ -57,6 +59,12 @@ function genSalt(rounds, minor, cb) { return process.nextTick(function () { cb(error); }); + } else if (rounds < 0) { + // callback error asynchronously + error = new Error('rounds must be a positive integer'); + return process.nextTick(function () { + cb(error); + }); } if (!minor) { @@ -145,8 +153,11 @@ function hash(data, salt, cb) { if (typeof salt === 'number') { - return module.exports.genSalt(salt, function (err, salt) { - return bindings.encrypt(data, salt, cb); + return module.exports.genSalt(salt, function (err, generatedSalt) { + if (err) { + return cb(err); + } + return bindings.encrypt(data, generatedSalt, cb); }); } diff --git a/test/async.test.js b/test/async.test.js index fb59367..d0230ea 100644 --- a/test/async.test.js +++ b/test/async.test.js @@ -207,3 +207,21 @@ test('hash_compare_one_param', done => { done(); }); }) + +test('salt_rounds_is_negative', done => { + expect.assertions(2); + bcrypt.genSalt(-1, function (err, salt) { + expect(err instanceof Error).toBe(true) + expect(err.message).toBe('rounds must be a positive integer') + done(); + }); +}) + +test('hash_rounds_is_negative', done => { + expect.assertions(2); + bcrypt.hash('password', -1, function (err, hash) { + expect(err instanceof Error).toBe(true) + expect(err.message).toBe('rounds must be a positive integer') + done(); + }); +}) diff --git a/test/promise.test.js b/test/promise.test.js index 0103418..c892372 100644 --- a/test/promise.test.js +++ b/test/promise.test.js @@ -23,6 +23,10 @@ test('salt_rounds_is_string_non_number', () => { return expect(bcrypt.genSalt('b')).rejects.toThrow('rounds must be a number'); }) +test('salt_rounds_is_negative', () => { + return expect(bcrypt.genSalt(-1)).rejects.toThrow('rounds must be a positive integer'); +}) + test('hash_returns_promise_on_null_callback', () => { expect(typeof bcrypt.hash('password', 10, null).then).toStrictEqual('function') }) @@ -57,6 +61,10 @@ test('hash_one_param', () => { return expect(bcrypt.hash('password')).rejects.toThrow('data and salt arguments required'); }) +test('hash_rounds_is_negative', () => { + return expect(bcrypt.hash('password', -1)).rejects.toThrow('rounds must be a positive integer'); +}) + test('hash_salt_validity', () => { expect.assertions(2); return Promise.all( diff --git a/test/sync.test.js b/test/sync.test.js index 2e6809a..6e6aa54 100644 --- a/test/sync.test.js +++ b/test/sync.test.js @@ -123,3 +123,11 @@ test('getRounds', () => { expect(9).toStrictEqual(bcrypt.getRounds(hash)) expect(() => bcrypt.getRounds('')).toThrow("invalid hash provided"); }); + +test('salt_rounds_is_negative', () => { + expect(() => bcrypt.genSaltSync(-1)).toThrowError('rounds must be a positive integer'); +}) + +test('hash_rounds_is_negative', () => { + expect(() => bcrypt.hashSync('password', -1)).toThrowError('rounds must be a positive integer'); +})