Skip to content

Commit 55fd6b6

Browse files
tniessendanielleadams
authored andcommitted
crypto: avoid infinite loops in prime generation
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>
1 parent a693baa commit 55fd6b6

File tree

2 files changed

+58
-0
lines changed

2 files changed

+58
-0
lines changed

src/crypto/crypto_random.cc

+20
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,26 @@ Maybe<bool> RandomPrimeTraits::AdditionalConfig(
127127
return Nothing<bool>();
128128
}
129129

130+
if (params->add) {
131+
if (BN_num_bits(params->add.get()) > bits) {
132+
// If we allowed this, the best case would be returning a static prime
133+
// that wasn't generated randomly. The worst case would be an infinite
134+
// loop within OpenSSL, blocking the main thread or one of the threads
135+
// in the thread pool.
136+
THROW_ERR_OUT_OF_RANGE(env, "invalid options.add");
137+
return Nothing<bool>();
138+
}
139+
140+
if (params->rem) {
141+
if (BN_cmp(params->add.get(), params->rem.get()) != 1) {
142+
// This would definitely lead to an infinite loop if allowed since
143+
// OpenSSL does not check this condition.
144+
THROW_ERR_OUT_OF_RANGE(env, "invalid options.rem");
145+
return Nothing<bool>();
146+
}
147+
}
148+
}
149+
130150
params->bits = bits;
131151
params->safe = safe;
132152
params->prime.reset(BN_secure_new());

test/parallel/test-crypto-prime.js

+38
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,44 @@ generatePrime(
181181
}
182182
}
183183

184+
{
185+
// This is impossible because it implies (prime % 2**64) == 1 and
186+
// prime < 2**64, meaning prime = 1, but 1 is not prime.
187+
for (const add of [2n ** 64n, 2n ** 65n]) {
188+
assert.throws(() => {
189+
generatePrimeSync(64, { add });
190+
}, {
191+
code: 'ERR_OUT_OF_RANGE',
192+
message: 'invalid options.add'
193+
});
194+
}
195+
196+
// Any parameters with rem >= add lead to an impossible condition.
197+
for (const rem of [7n, 8n, 3000n]) {
198+
assert.throws(() => {
199+
generatePrimeSync(64, { add: 7n, rem });
200+
}, {
201+
code: 'ERR_OUT_OF_RANGE',
202+
message: 'invalid options.rem'
203+
});
204+
}
205+
206+
// This is possible, but not allowed. It implies prime == 7, which means that
207+
// we did not actually generate a random prime.
208+
assert.throws(() => {
209+
generatePrimeSync(3, { add: 8n, rem: 7n });
210+
}, {
211+
code: 'ERR_OUT_OF_RANGE'
212+
});
213+
214+
// This is possible and allowed (but makes little sense).
215+
assert.strictEqual(generatePrimeSync(4, {
216+
add: 15n,
217+
rem: 13n,
218+
bigint: true
219+
}), 13n);
220+
}
221+
184222
[1, 'hello', {}, []].forEach((i) => {
185223
assert.throws(() => checkPrime(i), {
186224
code: 'ERR_INVALID_ARG_TYPE'

0 commit comments

Comments
 (0)