crypto: fix generateKeyPair type checks

Change saltLength, divisorLength, primeLength and generator checks in generateKeyPair to int32 from uint32, to align with c++ code. fixes: #38358 PR-URL: #38364 Fixes: #38358 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Apr 29, 2021 · 4b073b0 · 4b073b0
1 parent f455e08
commit 4b073b0
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 5 deletions.
diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js
@@ -40,6 +40,7 @@ const {
 const { customPromisifyArgs } = require('internal/util');
 
 const {
+  isInt32,
   isUint32,
   validateCallback,
   validateString,
@@ -194,7 +195,7 @@ function createJob(mode, type, options) {
         throw new ERR_INVALID_ARG_VALUE('options.hash', hash);
       if (mgf1Hash !== undefined && typeof mgf1Hash !== 'string')
         throw new ERR_INVALID_ARG_VALUE('options.mgf1Hash', mgf1Hash);
-      if (saltLength !== undefined && !isUint32(saltLength))
+      if (saltLength !== undefined && (!isInt32(saltLength) || saltLength < 0))
         throw new ERR_INVALID_ARG_VALUE('options.saltLength', saltLength);
 
       return new RsaKeyPairGenJob(
@@ -217,7 +218,7 @@ function createJob(mode, type, options) {
       let { divisorLength } = options;
       if (divisorLength == null) {
         divisorLength = -1;
-      } else if (!isUint32(divisorLength)) {
+      } else if (!isInt32(divisorLength) || divisorLength < 0) {
         throw new ERR_INVALID_ARG_VALUE('options.divisorLength', divisorLength);
       }
 
@@ -292,15 +293,15 @@ function createJob(mode, type, options) {
         if (!isArrayBufferView(prime))
           throw new ERR_INVALID_ARG_VALUE('options.prime', prime);
       } else if (primeLength != null) {
-        if (!isUint32(primeLength))
+        if (!isInt32(primeLength) || primeLength < 0)
           throw new ERR_INVALID_ARG_VALUE('options.primeLength', primeLength);
       } else {
         throw new ERR_MISSING_OPTION(
           'At least one of the group, prime, or primeLength options');
       }
 
       if (generator != null) {
-        if (!isUint32(generator))
+        if (!isInt32(generator) || generator < 0)
           throw new ERR_INVALID_ARG_VALUE('options.generator', generator);
       }
       return new DhKeyPairGenJob(

diff --git a/test/parallel/test-crypto-keygen.js b/test/parallel/test-crypto-keygen.js
@@ -958,7 +958,7 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
   }
 
   // Test invalid divisor lengths.
-  for (const divisorLength of ['a', true, {}, [], 4096.1]) {
+  for (const divisorLength of ['a', true, {}, [], 4096.1, 2147483648, -1]) {
     assert.throws(() => generateKeyPair('dsa', {
       modulusLength: 2048,
       divisorLength
@@ -1081,6 +1081,52 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     message: 'Unknown DH group'
   });
 
+  assert.throws(() => {
+    generateKeyPair('dh', {
+      primeLength: 2147483648
+    }, common.mustNotCall());
+  }, {
+    name: 'TypeError',
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: "The property 'options.primeLength' is invalid. " +
+             'Received 2147483648',
+  });
+
+  assert.throws(() => {
+    generateKeyPair('dh', {
+      primeLength: -1
+    }, common.mustNotCall());
+  }, {
+    name: 'TypeError',
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: "The property 'options.primeLength' is invalid. " +
+             'Received -1',
+  });
+
+  assert.throws(() => {
+    generateKeyPair('dh', {
+      primeLength: 2,
+      generator: 2147483648,
+    }, common.mustNotCall());
+  }, {
+    name: 'TypeError',
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: "The property 'options.generator' is invalid. " +
+             'Received 2147483648',
+  });
+
+  assert.throws(() => {
+    generateKeyPair('dh', {
+      primeLength: 2,
+      generator: -1,
+    }, common.mustNotCall());
+  }, {
+    name: 'TypeError',
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: "The property 'options.generator' is invalid. " +
+             'Received -1',
+  });
+
   // Test incompatible options.
   const allOpts = {
     group: 'modp5',
@@ -1142,6 +1188,35 @@ const sec1EncExp = (cipher) => getRegExpForPEM('EC PRIVATE KEY', cipher);
     });
   }
 
+  // too long salt length
+  assert.throws(() => {
+    generateKeyPair('rsa-pss', {
+      modulusLength: 512,
+      saltLength: 2147483648,
+      hash: 'sha256',
+      mgf1Hash: 'sha256'
+    }, common.mustNotCall());
+  }, {
+    name: 'TypeError',
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: "The property 'options.saltLength' is invalid. " +
+             'Received 2147483648'
+  });
+
+  assert.throws(() => {
+    generateKeyPair('rsa-pss', {
+      modulusLength: 512,
+      saltLength: -1,
+      hash: 'sha256',
+      mgf1Hash: 'sha256'
+    }, common.mustNotCall());
+  }, {
+    name: 'TypeError',
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: "The property 'options.saltLength' is invalid. " +
+             'Received -1'
+  });
+
   // Invalid private key type.
   for (const type of ['foo', 'spki']) {
     assert.throws(() => {