crypto: avoid infinite loops in prime generation

tniessen · danielleadams · commit 55fd6b661160 · 2021-02-16T09:12:53.000-05:00
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>
diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc
@@ -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());
diff --git a/test/parallel/test-crypto-prime.js b/test/parallel/test-crypto-prime.js
@@ -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'
