crypto: avoid infinite loops in prime generation

PR-URL: https://github.com/nodejs/node/pull/37212
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This commit is contained in:
Tobias Nießen
2021-02-03 18:31:17 +01:00
committed by Rich Trott
parent fdd7a87b8a
commit 11d20106ea
2 changed files with 58 additions and 0 deletions

View File

@@ -127,6 +127,26 @@ Maybe<bool> RandomPrimeTraits::AdditionalConfig(
return Nothing<bool>();
}
if (params->add) {
if (BN_num_bits(params->add.get()) > bits) {
// If we allowed this, the best case would be returning a static prime
// that wasn't generated randomly. The worst case would be an infinite
// loop within OpenSSL, blocking the main thread or one of the threads
// in the thread pool.
THROW_ERR_OUT_OF_RANGE(env, "invalid options.add");
return Nothing<bool>();
}
if (params->rem) {
if (BN_cmp(params->add.get(), params->rem.get()) != 1) {
// This would definitely lead to an infinite loop if allowed since
// OpenSSL does not check this condition.
THROW_ERR_OUT_OF_RANGE(env, "invalid options.rem");
return Nothing<bool>();
}
}
}
params->bits = bits;
params->safe = safe;
params->prime.reset(BN_secure_new());

View File

@@ -181,6 +181,44 @@ generatePrime(
}
}
{
// This is impossible because it implies (prime % 2**64) == 1 and
// prime < 2**64, meaning prime = 1, but 1 is not prime.
for (const add of [2n ** 64n, 2n ** 65n]) {
assert.throws(() => {
generatePrimeSync(64, { add });
}, {
code: 'ERR_OUT_OF_RANGE',
message: 'invalid options.add'
});
}
// Any parameters with rem >= add lead to an impossible condition.
for (const rem of [7n, 8n, 3000n]) {
assert.throws(() => {
generatePrimeSync(64, { add: 7n, rem });
}, {
code: 'ERR_OUT_OF_RANGE',
message: 'invalid options.rem'
});
}
// This is possible, but not allowed. It implies prime == 7, which means that
// we did not actually generate a random prime.
assert.throws(() => {
generatePrimeSync(3, { add: 8n, rem: 7n });
}, {
code: 'ERR_OUT_OF_RANGE'
});
// This is possible and allowed (but makes little sense).
assert.strictEqual(generatePrimeSync(4, {
add: 15n,
rem: 13n,
bigint: true
}), 13n);
}
[1, 'hello', {}, []].forEach((i) => {
assert.throws(() => checkPrime(i), {
code: 'ERR_INVALID_ARG_TYPE'