crypto: restrict PBKDF2 args to signed int

OpenSSL internally represents the output length and the iteration count as signed integers, which is why node's C++ implementation expects these arguments to fit into signed integers as well. The JavaScript validation logic, however, only requires the arguments to be unsigned 32-bit integers, which is a superset of non-negative (signed) 32-bit integers. Change the JavaScript validation to match the expectation within C++. Fixes: #44570 PR-URL: #44575 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
nodejs · Sep 26, 2022 · 993bd9b · 993bd9b
1 parent 39b65d2
commit 993bd9b
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 4 deletions.
diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js
@@ -15,6 +15,7 @@ const {
 
 const {
   validateFunction,
+  validateInt32,
   validateInteger,
   validateString,
   validateUint32,
@@ -91,8 +92,10 @@ function check(password, salt, iterations, keylen, digest) {
 
   password = getArrayBufferOrView(password, 'password');
   salt = getArrayBufferOrView(salt, 'salt');
-  validateUint32(iterations, 'iterations', true);
-  validateUint32(keylen, 'keylen');
+  // OpenSSL uses a signed int to represent these values, so we are restricted
+  // to the 31-bit range here (which is plenty).
+  validateInt32(iterations, 'iterations', 1);
+  validateInt32(keylen, 'keylen', 0);
 
   return { password, salt, iterations, keylen, digest };
 }

diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js
@@ -63,7 +63,7 @@ assert.throws(
   }
 );
 
-for (const iterations of [-1, 0]) {
+for (const iterations of [-1, 0, 2147483648]) {
   assert.throws(
     () => crypto.pbkdf2Sync('password', 'salt', iterations, 20, 'sha1'),
     {
@@ -98,7 +98,7 @@ for (const iterations of [-1, 0]) {
     });
 });
 
-[-1, 4294967297].forEach((input) => {
+[-1, 2147483648, 4294967296].forEach((input) => {
   assert.throws(
     () => {
       crypto.pbkdf2('password', 'salt', 1, input, 'sha256',