Skip to content

Commit dd42214

Browse files
tniessendanielleadams
authored andcommitted
crypto: unify validation of checkPrime checks
Previously, the JS layer would validate that the value of the 'checks' option was an unsigned 32-bit integer, otherwise throwing an appropriate error but with a slightly misleading error message. Then the C++ layer would validate that the value was an unsigned 31-bit integer, otherwise throwing an appropriate error, but with a different (and even less helpful) error message. Instead, make the JS layer aware of the 31-bit restriction so that no validation in C++ is necessary and so that the error message always matches the exact requirement. PR-URL: #47165 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 0dc4971 commit dd42214

File tree

3 files changed

+19
-14
lines changed

3 files changed

+19
-14
lines changed

lib/internal/crypto/random.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ const {
5252
validateFunction,
5353
validateInt32,
5454
validateObject,
55-
validateUint32,
5655
} = require('internal/validators');
5756

5857
const {
@@ -563,7 +562,8 @@ function checkPrime(candidate, options = kEmptyObject, callback) {
563562
checks = 0,
564563
} = options;
565564

566-
validateUint32(checks, 'options.checks');
565+
// The checks option is unsigned but must fit into a signed C int for OpenSSL.
566+
validateInt32(checks, 'options.checks', 0);
567567

568568
const job = new CheckPrimeJob(kCryptoJobAsync, candidate, checks);
569569
job.ondone = callback;
@@ -591,7 +591,8 @@ function checkPrimeSync(candidate, options = kEmptyObject) {
591591
checks = 0,
592592
} = options;
593593

594-
validateUint32(checks, 'options.checks');
594+
// The checks option is unsigned but must fit into a signed C int for OpenSSL.
595+
validateInt32(checks, 'options.checks', 0);
595596

596597
const job = new CheckPrimeJob(kCryptoJobSync, candidate, checks);
597598
const { 0: err, 1: result } = job.run();

src/crypto/crypto_random.cc

+4-11
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ using v8::ArrayBuffer;
1515
using v8::BackingStore;
1616
using v8::False;
1717
using v8::FunctionCallbackInfo;
18+
using v8::Int32;
1819
using v8::Just;
1920
using v8::Local;
2021
using v8::Maybe;
@@ -185,8 +186,6 @@ Maybe<bool> CheckPrimeTraits::AdditionalConfig(
185186
const FunctionCallbackInfo<Value>& args,
186187
unsigned int offset,
187188
CheckPrimeConfig* params) {
188-
Environment* env = Environment::GetCurrent(args);
189-
190189
ArrayBufferOrViewContents<unsigned char> candidate(args[offset]);
191190

192191
params->candidate =
@@ -195,15 +194,9 @@ Maybe<bool> CheckPrimeTraits::AdditionalConfig(
195194
candidate.size(),
196195
nullptr));
197196

198-
CHECK(args[offset + 1]->IsUint32()); // Checks
199-
200-
const int checks = static_cast<int>(args[offset + 1].As<Uint32>()->Value());
201-
if (checks < 0) {
202-
THROW_ERR_OUT_OF_RANGE(env, "invalid options.checks");
203-
return Nothing<bool>();
204-
}
205-
206-
params->checks = checks;
197+
CHECK(args[offset + 1]->IsInt32()); // Checks
198+
params->checks = args[offset + 1].As<Int32>()->Value();
199+
CHECK_GE(params->checks, 0);
207200

208201
return Just(true);
209202
}

test/parallel/test-crypto-prime.js

+11
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,17 @@ for (const checks of ['hello', {}, []]) {
240240
});
241241
}
242242

243+
for (const checks of [-(2 ** 31), -1, 2 ** 31, 2 ** 32 - 1, 2 ** 32, 2 ** 50]) {
244+
assert.throws(() => checkPrime(2n, { checks }, common.mustNotCall()), {
245+
code: 'ERR_OUT_OF_RANGE',
246+
message: /<= 2147483647/
247+
});
248+
assert.throws(() => checkPrimeSync(2n, { checks }), {
249+
code: 'ERR_OUT_OF_RANGE',
250+
message: /<= 2147483647/
251+
});
252+
}
253+
243254
assert(!checkPrimeSync(Buffer.from([0x1])));
244255
assert(checkPrimeSync(Buffer.from([0x2])));
245256
assert(checkPrimeSync(Buffer.from([0x3])));

0 commit comments

Comments
 (0)