From 0b1b616a9cede7567b5e784f262e7939ccb1fa98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Tue, 8 Mar 2022 18:33:38 +0100
Subject: [PATCH] crypto: improve prime size argument validation

The current validation in JavaScript is insufficient and also produces
an incorrect error message, restricting the size parameter to 32-bit
values, whereas the C++ backend restricts the size parameter to the
positive range of an int.

This change tightens the validation in JavaScript and adapts the error
message accordingly, making the validation in C++ superfluous.

Refs: https://github.com/nodejs/node/pull/42207

PR-URL: https://github.com/nodejs/node/pull/42234
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/internal/crypto/random.js      |  5 +++--
 src/crypto/crypto_random.cc        |  6 ++----
 test/parallel/test-crypto-prime.js | 12 +++++++-----
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js
index 1423ea4295443e..43d7a022a67ea5 100644
--- a/lib/internal/crypto/random.js
+++ b/lib/internal/crypto/random.js
@@ -43,6 +43,7 @@ const {
   validateNumber,
   validateBoolean,
   validateCallback,
+  validateInt32,
   validateObject,
   validateUint32,
 } = require('internal/validators');
@@ -460,7 +461,7 @@ function createRandomPrimeJob(type, size, options) {
 }
 
 function generatePrime(size, options, callback) {
-  validateUint32(size, 'size', true);
+  validateInt32(size, 'size', 1);
   if (typeof options === 'function') {
     callback = options;
     options = {};
@@ -482,7 +483,7 @@ function generatePrime(size, options, callback) {
 }
 
 function generatePrimeSync(size, options = {}) {
-  validateUint32(size, 'size', true);
+  validateInt32(size, 'size', 1);
 
   const job = createRandomPrimeJob(kCryptoJobSync, size, options);
   const { 0: err, 1: prime } = job.run();
diff --git a/src/crypto/crypto_random.cc b/src/crypto/crypto_random.cc
index fc88deb460314c..7a05dcc16b7389 100644
--- a/src/crypto/crypto_random.cc
+++ b/src/crypto/crypto_random.cc
@@ -122,11 +122,9 @@ Maybe<bool> RandomPrimeTraits::AdditionalConfig(
     }
   }
 
+  // The JS interface already ensures that the (positive) size fits into an int.
   int bits = static_cast<int>(size);
-  if (bits < 0) {
-    THROW_ERR_OUT_OF_RANGE(env, "invalid size");
-    return Nothing<bool>();
-  }
+  CHECK_GT(bits, 0);
 
   if (params->add) {
     if (BN_num_bits(params->add.get()) > bits) {
diff --git a/test/parallel/test-crypto-prime.js b/test/parallel/test-crypto-prime.js
index 2d3f39aec15a08..bab98c4e9697e6 100644
--- a/test/parallel/test-crypto-prime.js
+++ b/test/parallel/test-crypto-prime.js
@@ -41,12 +41,14 @@ const pCheckPrime = promisify(checkPrime);
   });
 });
 
-[-1, 0, 2 ** 31, 2 ** 31 + 1, 2 ** 32 - 1, 2 ** 32].forEach((i) => {
-  assert.throws(() => generatePrime(i, common.mustNotCall()), {
-    code: 'ERR_OUT_OF_RANGE'
+[-1, 0, 2 ** 31, 2 ** 31 + 1, 2 ** 32 - 1, 2 ** 32].forEach((size) => {
+  assert.throws(() => generatePrime(size, common.mustNotCall()), {
+    code: 'ERR_OUT_OF_RANGE',
+    message: />= 1 && <= 2147483647/
   });
-  assert.throws(() => generatePrimeSync(i), {
-    code: 'ERR_OUT_OF_RANGE'
+  assert.throws(() => generatePrimeSync(size), {
+    code: 'ERR_OUT_OF_RANGE',
+    message: />= 1 && <= 2147483647/
   });
 });