From 7830593086b57166eec190845449a7ed0f9f0e0f Mon Sep 17 00:00:00 2001
From: Abhishek Chauhan <abhishek.chauhan@sonatype.com>
Date: Wed, 18 Mar 2026 13:22:53 -0500
Subject: [PATCH] fix: reject negative salt rounds in hash()

Previously, passing a negative value for salt rounds (e.g., `bcrypt.hash('password', -5, cb)`)
would cause the library to hang indefinitely. This occurred because:

1. Negative values passed JavaScript's validation (checking only for type)
2. In the native C++ code, the negative value was cast to u_int8_t, causing
   integer underflow (e.g., -5 becomes 251)
3. This resulted in `1 << 251` rounds, an astronomically large iteration count

This fix adds early validation in the JavaScript layer to reject negative
rounds with a clear error message "rounds must be a positive number", matching
the existing pattern for type validation.

Also fixes a related issue where genSalt errors were not properly propagated
through the hash() callback when salt rounds were invalid.

Fixes #1218
---
 bcrypt.js            | 11 +++++++++++
 test/async.test.js   | 18 ++++++++++++++++++
 test/promise.test.js |  8 ++++++++
 test/sync.test.js    |  8 ++++++++
 4 files changed, 45 insertions(+)

diff --git a/bcrypt.js b/bcrypt.js
index 62da525..ced0c8e 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 number');
     }
 
     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 number');
+        return process.nextTick(function () {
+            cb(error);
+        });
     }
 
     if (!minor) {
@@ -146,6 +154,9 @@ function hash(data, salt, cb) {
 
     if (typeof salt === 'number') {
         return module.exports.genSalt(salt, function (err, salt) {
+            if (err) {
+                return cb(err);
+            }
             return bindings.encrypt(data, salt, cb);
         });
     }
diff --git a/test/async.test.js b/test/async.test.js
index fb59367..8448669 100644
--- a/test/async.test.js
+++ b/test/async.test.js
@@ -34,6 +34,15 @@ test('salt_rounds_is_string_non_number', done => {
     });
 })
 
+test('salt_rounds_is_negative', done => {
+    expect.assertions(2);
+    bcrypt.genSalt(-5, function (err, salt) {
+        expect(err instanceof Error).toBe(true)
+        expect(err.message).toBe('rounds must be a positive number')
+        done();
+    });
+})
+
 test('salt_minor', done => {
     expect.assertions(3);
     bcrypt.genSalt(10, 'a', function (err, value) {
@@ -75,6 +84,15 @@ test('hash_rounds', done => {
     });
 })
 
+test('hash_rounds_is_negative', done => {
+    expect.assertions(2);
+    bcrypt.hash('bacon', -5, function (err, hash) {
+        expect(err instanceof Error).toBe(true);
+        expect(err.message).toBe('rounds must be a positive number');
+        done();
+    });
+})
+
 test('hash_empty_strings', done => {
     expect.assertions(1);
     bcrypt.genSalt(10, function (err, salt) {
diff --git a/test/promise.test.js b/test/promise.test.js
index 0103418..0af58ba 100644
--- a/test/promise.test.js
+++ b/test/promise.test.js
@@ -23,6 +23,14 @@ 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(-5)).rejects.toThrow('rounds must be a positive number');
+})
+
+test('hash_rounds_is_negative', () => {
+    return expect(bcrypt.hash('password', -5)).rejects.toThrow('rounds must be a positive number');
+})
+
 test('hash_returns_promise_on_null_callback', () => {
     expect(typeof bcrypt.hash('password', 10, null).then).toStrictEqual('function')
 })
diff --git a/test/sync.test.js b/test/sync.test.js
index 2e6809a..f01010e 100644
--- a/test/sync.test.js
+++ b/test/sync.test.js
@@ -23,6 +23,10 @@ test('salt_rounds_is_NaN', () => {
     expect(() => bcrypt.genSaltSync('b')).toThrowError("rounds must be a number");
 })
 
+test('salt_rounds_is_negative', () => {
+    expect(() => bcrypt.genSaltSync(-5)).toThrowError("rounds must be a positive number");
+})
+
 test('salt_minor_a', () => {
     const salt = bcrypt.genSaltSync(10, 'a');
     const [_, version, rounds] = salt.split('$');
@@ -46,6 +50,10 @@ test('hash_rounds', () => {
     expect(bcrypt.getRounds(hash)).toStrictEqual(8)
 })
 
+test('hash_rounds_is_negative', () => {
+    expect(() => bcrypt.hashSync('password', -5)).toThrowError('rounds must be a positive number');
+})
+
 test('hash_empty_string', () => {
     expect(() => bcrypt.hashSync('', bcrypt.genSaltSync(10))).not.toThrow();
     expect(() => bcrypt.hashSync('password', '')).toThrowError('Invalid salt. Salt must be in the form of: $Vers$log2(NumRounds)$saltvalue');