Skip to content

Commit

Permalink
crypto: avoid infinite loops in prime generation
Browse files Browse the repository at this point in the history
PR-URL: #37212
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
tniessen authored and danielleadams committed Feb 16, 2021
1 parent a693baa commit 55fd6b6
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
20 changes: 20 additions & 0 deletions src/crypto/crypto_random.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
38 changes: 38 additions & 0 deletions test/parallel/test-crypto-prime.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit 55fd6b6

Please sign in to comment.