Skip to content

Commit

Permalink
crypto: improve prime size argument validation
Browse files Browse the repository at this point in the history
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: #42207

PR-URL: #42234
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
tniessen committed Mar 8, 2022
1 parent 9412441 commit e8697cf
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 11 deletions.
5 changes: 3 additions & 2 deletions lib/internal/crypto/random.js
Expand Up @@ -43,6 +43,7 @@ const {
validateNumber,
validateBoolean,
validateFunction,
validateInt32,
validateObject,
validateUint32,
} = require('internal/validators');
Expand Down Expand Up @@ -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 = {};
Expand All @@ -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();
Expand Down
6 changes: 2 additions & 4 deletions src/crypto/crypto_random.cc
Expand Up @@ -122,11 +122,9 @@ Maybe<bool> RandomPrimeTraits::AdditionalConfig(
}
}

// The JS interface already ensures that the (positive) size fits into an int.
int bits = static_cast<int>(size);
if (bits < 0) {
THROW_ERR_OUT_OF_RANGE(env, "invalid size");
return Nothing<bool>();
}
CHECK_GT(bits, 0);

if (params->add) {
if (BN_num_bits(params->add.get()) > bits) {
Expand Down
12 changes: 7 additions & 5 deletions test/parallel/test-crypto-prime.js
Expand Up @@ -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/
});
});

Expand Down

0 comments on commit e8697cf

Please sign in to comment.