crypto: refactor to avoid unsafe array iteration

PR-URL: #37364 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Zijian Liu <[email protected]>
nodejs · Feb 19, 2021 · 08a2383 · 08a2383
1 parent 445108d
commit 08a2383
Show file tree

Hide file tree

Showing 11 changed files with 57 additions and 36 deletions.
diff --git a/lib/internal/crypto/aes.js b/lib/internal/crypto/aes.js
@@ -8,6 +8,7 @@ const {
   ArrayPrototypePush,
   MathFloor,
   Promise,
+  ReflectApply,
   SafeSet,
   TypedArrayPrototypeSlice,
 } = primordials;
@@ -252,12 +253,12 @@ async function aesImportKey(
   extractable,
   keyUsages) {
   const { name } = algorithm;
-  const checkUsages = ['wrapKey', 'unwrapKey'];
+  const usagesSet = new SafeSet(keyUsages);
+  const checkUsages = [usagesSet, 'wrapKey', 'unwrapKey'];
   if (name !== 'AES-KW')
     ArrayPrototypePush(checkUsages, 'encrypt', 'decrypt');
 
-  const usagesSet = new SafeSet(keyUsages);
-  if (hasAnyNotIn(usagesSet, ...checkUsages)) {
+  if (ReflectApply(hasAnyNotIn, null, checkUsages)) {
     throw lazyDOMException(
       'Unsupported key usage for an AES key',
       'SyntaxError');

diff --git a/lib/internal/crypto/diffiehellman.js b/lib/internal/crypto/diffiehellman.js
@@ -2,10 +2,12 @@
 
 const {
   ArrayBufferPrototypeSlice,
+  ArrayPrototypePush,
   FunctionPrototypeCall,
   MathFloor,
   ObjectDefineProperty,
   Promise,
+  ReflectApply,
   SafeSet,
 } = primordials;
 
@@ -347,16 +349,15 @@ function deriveBitsDH(publicKey, privateKey, callback) {
 }
 
 function verifyAcceptableDhKeyUse(name, type, usages) {
-  let checkSet;
+  const args = [usages];
   switch (type) {
     case 'private':
-      checkSet = ['deriveBits', 'deriveKey'];
+      ArrayPrototypePush(args, 'deriveBits', 'deriveKey');
       break;
     case 'public':
-      checkSet = [];
       break;
   }
-  if (hasAnyNotIn(usages, ...checkSet)) {
+  if (ReflectApply(hasAnyNotIn, null, args)) {
     throw lazyDOMException(
       `Unsupported key usage for an ${name} key`,
       'SyntaxError');

diff --git a/lib/internal/crypto/dsa.js b/lib/internal/crypto/dsa.js
@@ -51,16 +51,16 @@ const {
 } = require('internal/crypto/util');
 
 function verifyAcceptableDsaKeyUse(name, type, usages) {
-  let checkSet;
+  let check;
   switch (type) {
     case 'private':
-      checkSet = ['sign'];
+      check = 'sign';
       break;
     case 'public':
-      checkSet = ['verify'];
+      check = 'verify';
       break;
   }
-  if (hasAnyNotIn(usages, ...checkSet)) {
+  if (hasAnyNotIn(usages, check)) {
     throw lazyDOMException(
       `Unsupported key usage for an ${name} key`,
       'SyntaxError');

diff --git a/lib/internal/crypto/ec.js b/lib/internal/crypto/ec.js
@@ -1,8 +1,10 @@
 'use strict';
 
 const {
+  ArrayPrototypePush,
   ObjectKeys,
   Promise,
+  ReflectApply,
   SafeSet,
 } = primordials;
 
@@ -59,10 +61,10 @@ const {
 } = require('internal/crypto/keys');
 
 function verifyAcceptableEcKeyUse(name, type, usages) {
-  let checkSet;
+  const args = [usages];
   switch (name) {
     case 'ECDH':
-      checkSet = ['deriveKey', 'deriveBits'];
+      ArrayPrototypePush(args, 'deriveKey', 'deriveBits');
       break;
     case 'NODE-ED25519':
       // Fall through
@@ -71,14 +73,14 @@ function verifyAcceptableEcKeyUse(name, type, usages) {
     case 'ECDSA':
       switch (type) {
         case 'private':
-          checkSet = ['sign'];
+          ArrayPrototypePush(args, 'sign');
           break;
         case 'public':
-          checkSet = ['verify'];
+          ArrayPrototypePush(args, 'verify');
           break;
       }
   }
-  if (hasAnyNotIn(usages, ...checkSet)) {
+  if (ReflectApply(hasAnyNotIn, null, args)) {
     throw lazyDOMException(
       `Unsupported key usage for a ${name} key`,
       'SyntaxError');

diff --git a/lib/internal/crypto/hkdf.js b/lib/internal/crypto/hkdf.js
@@ -134,7 +134,7 @@ function hkdfSync(hash, key, salt, info, length) {
   } = validateParameters(hash, key, salt, info, length));
 
   const job = new HKDFJob(kCryptoJobSync, hash, key, salt, info, length);
-  const [err, bits] = job.run();
+  const { 0: err, 1: bits } = job.run();
   if (err !== undefined)
     throw err;
 

diff --git a/lib/internal/crypto/keygen.js b/lib/internal/crypto/keygen.js
@@ -3,6 +3,7 @@
 const {
   FunctionPrototypeCall,
   ObjectDefineProperty,
+  SafeArrayIterator,
 } = primordials;
 
 const {
@@ -75,7 +76,7 @@ function generateKeyPair(type, options, callback) {
   job.ondone = (error, result) => {
     if (error) return FunctionPrototypeCall(callback, job, error);
     // If no encoding was chosen, return key objects instead.
-    let [pubkey, privkey] = result;
+    let { 0: pubkey, 1: privkey } = result;
     pubkey = wrapKey(pubkey, PublicKeyObject);
     privkey = wrapKey(privkey, PrivateKeyObject);
     FunctionPrototypeCall(callback, job, null, pubkey, privkey);
@@ -97,11 +98,11 @@ function handleError(ret) {
   if (ret == null)
     return; // async
 
-  const [err, keys] = ret;
+  const { 0: err, 1: keys } = ret;
   if (err !== undefined)
     throw err;
 
-  const [publicKey, privateKey] = keys;
+  const { 0: publicKey, 1: privateKey } = keys;
 
   // If no encoding was chosen, return key objects instead.
   return {
@@ -156,7 +157,7 @@ function parseKeyEncoding(keyType, options = {}) {
 function createJob(mode, type, options) {
   validateString(type, 'type');
 
-  const encoding = parseKeyEncoding(type, options);
+  const encoding = new SafeArrayIterator(parseKeyEncoding(type, options));
 
   if (options !== undefined)
     validateObject(options, 'options');
@@ -341,7 +342,7 @@ function handleGenerateKeyError(ret) {
   if (ret === undefined)
     return; // async
 
-  const [err, key] = ret;
+  const { 0: err, 1: key } = ret;
   if (err !== undefined)
     throw err;
 

diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js
@@ -2,6 +2,7 @@
 
 const {
   ArrayFrom,
+  ArrayPrototypeSlice,
   ObjectDefineProperty,
   ObjectSetPrototypeOf,
   Symbol,
@@ -160,6 +161,11 @@ const [
   }
 
   class AsymmetricKeyObject extends KeyObject {
+    // eslint-disable-next-line no-useless-constructor
+    constructor(type, handle) {
+      super(type, handle);
+    }
+
     get asymmetricKeyType() {
       return this[kAsymmetricKeyType] ||
              (this[kAsymmetricKeyType] = this[kHandle].getAsymmetricKeyType());
@@ -390,13 +396,21 @@ function getKeyObjectHandle(key, ctx) {
 }
 
 function getKeyTypes(allowKeyObject, bufferOnly = false) {
-  return [
+  const types = [
     'ArrayBuffer',
     'Buffer',
     'TypedArray',
     'DataView',
-    ...(!bufferOnly ? ['string'] : []),
-    ...(!bufferOnly && allowKeyObject ? ['KeyObject', 'CryptoKey'] : [])];
+    'string', // Only if bufferOnly == false
+    'KeyObject', // Only if allowKeyObject == true && bufferOnly == false
+    'CryptoKey', // Only if allowKeyObject == true && bufferOnly == false
+  ];
+  if (bufferOnly) {
+    return ArrayPrototypeSlice(types, 0, 4);
+  } else if (!allowKeyObject) {
+    return ArrayPrototypeSlice(types, 0, 5);
+  }
+  return types;
 }
 
 function prepareAsymmetricKey(key, ctx) {

diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js
@@ -74,7 +74,7 @@ function pbkdf2Sync(password, salt, iterations, keylen, digest) {
     keylen,
     digest);
 
-  const [err, result] = job.run();
+  const { 0: err, 1: result } = job.run();
   if (err !== undefined)
     throw err;
 

diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js
@@ -128,7 +128,7 @@ function randomFillSync(buf, offset = 0, size) {
     offset,
     size);
 
-  const [ err ] = job.run();
+  const err = job.run()[0];
   if (err)
     throw err;
 
@@ -471,7 +471,7 @@ function generatePrimeSync(size, options = {}) {
   validateUint32(size, 'size', true);
 
   const job = createRandomPrimeJob(kCryptoJobSync, size, options);
-  const [err, prime] = job.run();
+  const { 0: err, 1: prime } = job.run();
   if (err)
     throw err;
   return job.result(prime);
@@ -548,7 +548,7 @@ function checkPrimeSync(candidate, options = {}) {
   validateUint32(checks, 'options.checks');
 
   const job = new CheckPrimeJob(kCryptoJobSync, candidate, checks);
-  const [err, result] = job.run();
+  const { 0: err, 1: result } = job.run();
   if (err)
     throw err;
 

diff --git a/lib/internal/crypto/rsa.js b/lib/internal/crypto/rsa.js
@@ -1,7 +1,9 @@
 'use strict';
 
 const {
+  ArrayPrototypePush,
   Promise,
+  ReflectApply,
   SafeSet,
   Uint8Array,
 } = primordials;
@@ -71,29 +73,29 @@ const kRsaVariants = {
 };
 
 function verifyAcceptableRsaKeyUse(name, type, usages) {
-  let checkSet;
+  const args = [usages];
   switch (name) {
     case 'RSA-OAEP':
       switch (type) {
         case 'private':
-          checkSet = ['decrypt', 'unwrapKey'];
+          ArrayPrototypePush(args, 'decrypt', 'unwrapKey');
           break;
         case 'public':
-          checkSet = ['encrypt', 'wrapKey'];
+          ArrayPrototypePush(args, 'encrypt', 'wrapKey');
           break;
       }
       break;
     default:
       switch (type) {
         case 'private':
-          checkSet = ['sign'];
+          ArrayPrototypePush(args, 'sign');
           break;
         case 'public':
-          checkSet = ['verify'];
+          ArrayPrototypePush(args, 'verify');
           break;
       }
   }
-  if (hasAnyNotIn(usages, ...checkSet)) {
+  if (ReflectApply(hasAnyNotIn, null, args)) {
     throw lazyDOMException(
       `Unsupported key usage for an ${name} key`,
       'SyntaxError');

diff --git a/lib/internal/crypto/scrypt.js b/lib/internal/crypto/scrypt.js
@@ -74,7 +74,7 @@ function scryptSync(password, salt, keylen, options = defaults) {
   ({ password, salt, keylen } = options);
   const job = new ScryptJob(
     kCryptoJobSync, password, salt, N, r, p, maxmem, keylen);
-  const [err, result] = job.run();
+  const { 0: err, 1: result } = job.run();
 
   if (err !== undefined)
     throw err;