From 993bd9b134924238bc9aef55cf058c0ee22c732a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sun, 11 Sep 2022 00:21:14 +0200 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/44570 PR-URL: https://github.com/nodejs/node/pull/44575 Reviewed-By: Filip Skokan Reviewed-By: Luigi Pinca Reviewed-By: Rich Trott Reviewed-By: Mohammed Keyvanzadeh --- lib/internal/crypto/pbkdf2.js | 7 +++++-- test/parallel/test-crypto-pbkdf2.js | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js index 986ea557d63bf3..a9b5b1590f3b21 100644 --- 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 index c4dbedcb59160f..1f8e6a81f300e7 100644 --- 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',