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: nodejs/node#42207

PR-URL: nodejs/node#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 authored and guangwong committed Oct 10, 2022
1 parent cbc4934 commit ff47a29
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
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const {
validateNumber,
validateBoolean,
validateCallback,
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 ff47a29

Please sign in to comment.