From b298291af8c1c6fe4007a18166cb297c5e5be5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Tue, 8 Mar 2022 18:33:38 +0100 Subject: [PATCH] crypto: improve prime size argument validation The current validation in JavaScript is insufficient and also produces an incorrect error message, restricting the size parameter to 32-bit values, whereas the C++ backend restricts the size parameter to the positive range of an int. This change tightens the validation in JavaScript and adapts the error message accordingly, making the validation in C++ superfluous. Refs: https://github.com/nodejs/node/pull/42207 PR-URL: https://github.com/nodejs/node/pull/42234 Reviewed-By: Anna Henningsen Reviewed-By: Filip Skokan Reviewed-By: James M Snell --- lib/internal/crypto/random.js | 5 +++-- src/crypto/crypto_random.cc | 6 ++---- test/parallel/test-crypto-prime.js | 12 +++++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js index 1423ea4295443e..43d7a022a67ea5 100644 --- a/lib/internal/crypto/random.js +++ b/lib/internal/crypto/random.js @@ -43,6 +43,7 @@ const { validateNumber, validateBoolean, validateCallback, + validateInt32, validateObject, validateUint32, } = require('internal/validators'); @@ -460,7 +461,7 @@ function createRandomPrimeJob(type, size, options) { } function generatePrime(size, options, callback) { - validateUint32(size, 'size', true); + validateInt32(size, 'size', 1); if (typeof options === 'function') { callback = options; options = {}; @@ -482,7 +483,7 @@ function generatePrime(size, options, callback) { } function generatePrimeSync(size, options = {}) { - validateUint32(size, 'size', true); + validateInt32(size, 'size', 1); const job = createRandomPrimeJob(kCryptoJobSync, size, options); const { 0: err, 1: prime } = job.run(); diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc index fc88deb460314c..7a05dcc16b7389 100644 --- a/src/crypto/crypto_random.cc +++ b/src/crypto/crypto_random.cc @@ -122,11 +122,9 @@ Maybe RandomPrimeTraits::AdditionalConfig( } } + // The JS interface already ensures that the (positive) size fits into an int. int bits = static_cast(size); - if (bits < 0) { - THROW_ERR_OUT_OF_RANGE(env, "invalid size"); - return Nothing(); - } + CHECK_GT(bits, 0); if (params->add) { if (BN_num_bits(params->add.get()) > bits) { diff --git a/test/parallel/test-crypto-prime.js b/test/parallel/test-crypto-prime.js index 2d3f39aec15a08..bab98c4e9697e6 100644 --- a/test/parallel/test-crypto-prime.js +++ b/test/parallel/test-crypto-prime.js @@ -41,12 +41,14 @@ const pCheckPrime = promisify(checkPrime); }); }); -[-1, 0, 2 ** 31, 2 ** 31 + 1, 2 ** 32 - 1, 2 ** 32].forEach((i) => { - assert.throws(() => generatePrime(i, common.mustNotCall()), { - code: 'ERR_OUT_OF_RANGE' +[-1, 0, 2 ** 31, 2 ** 31 + 1, 2 ** 32 - 1, 2 ** 32].forEach((size) => { + assert.throws(() => generatePrime(size, common.mustNotCall()), { + code: 'ERR_OUT_OF_RANGE', + message: />= 1 && <= 2147483647/ }); - assert.throws(() => generatePrimeSync(i), { - code: 'ERR_OUT_OF_RANGE' + assert.throws(() => generatePrimeSync(size), { + code: 'ERR_OUT_OF_RANGE', + message: />= 1 && <= 2147483647/ }); });