crypto: harden WebCrypto against prototype pollution

Avoid re-wrapping native WebCrypto promises with PromiseResolve(),
since resolving a promise can read its user-mutated constructor.

Add a helper for chaining internal WebCrypto job promises without
consulting Promise species state, and use it for intermediate job
results.

Also align JWK wrapping and unwrapping with the spec's fresh-global
JSON handling by detaching internal JWK values from user prototypes.
Use the internal UTF-8 encoder/decoder bindings instead of shared
TextEncoder/TextDecoder prototype methods.

Expand the WebCrypto prototype pollution regression test to cover
SubtleCrypto methods, export formats, zero-length KDF results, JWK
toJSON/kty pollution, and encoder/decoder prototype poisoning.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>

Author: panva

lib/internal/crypto/diffiehellman.js

@@ -5,7 +5,6 @@ const {
   FunctionPrototypeCall,
   MathCeil,
   ObjectDefineProperty,
-  PromisePrototypeThen,
   TypedArrayPrototypeGetBuffer,
   Uint8Array,
 } = primordials;
@@ -59,6 +58,7 @@ const {
 const {
   getArrayBufferOrView,
   jobPromise,
+  jobPromiseThen,
   toBuf,
   kHandle,
 } = require('internal/crypto/util');
@@ -368,7 +368,7 @@ function ecdhDeriveBits(algorithm, baseKey, length) {
   if (length === null)
     return bits;
 
-  return PromisePrototypeThen(bits, (bits) => {
+  return jobPromiseThen(bits, (bits) => {
     // If the length is not a multiple of 8 the nearest ceiled
     // multiple of 8 is sliced.
     const sliceLength = MathCeil(length / 8);

lib/internal/crypto/util.js

@@ -14,6 +14,7 @@ const {
   ObjectEntries,
   ObjectKeys,
   ObjectPrototypeHasOwnProperty,
+  PromisePrototypeThen,
   PromiseReject,
   SafeMap,
   SafeSet,
@@ -678,6 +679,18 @@ function jobPromise(getJob) {
   }
 }
 
+// Promise.prototype.then gets promise.constructor to determine the result
+// promise's species. These promises are internal WebCrypto intermediates, so
+// make that lookup stay on the promise itself instead of user-mutated state.
+function jobPromiseThen(promise, onFulfilled, onRejected) {
+  ObjectDefineProperty(promise, 'constructor', {
+    __proto__: null,
+    configurable: true,
+    value: undefined,
+  });
+  return PromisePrototypeThen(promise, onFulfilled, onRejected);
+}
+
 // In WebCrypto, the publicExponent option in RSA is represented as a
 // WebIDL "BigInteger"... that is, a Uint8Array that allows an arbitrary
 // number of leading zero bits. Our conventional APIs for reading
@@ -900,6 +913,7 @@ module.exports = {
   validateByteSource,
   validateKeyOps,
   jobPromise,
+  jobPromiseThen,
   validateMaxBufferLength,
   bigIntArrayToUnsignedBigInt,
   bigIntArrayToUnsignedInt,

lib/internal/crypto/webcrypto.js

@@ -1,19 +1,24 @@
 'use strict';
 
 const {
+  ArrayIsArray,
   ArrayPrototypeSlice,
   FunctionPrototypeCall,
   JSONParse,
   JSONStringify,
   ObjectDefineProperties,
-  PromisePrototypeThen,
+  ObjectDefineProperty,
+  ObjectKeys,
+  ObjectSetPrototypeOf,
   PromiseReject,
   PromiseResolve,
   ReflectApply,
   ReflectConstruct,
+  SafeArrayIterator,
   StringPrototypeRepeat,
   StringPrototypeSlice,
   StringPrototypeStartsWith,
+  SymbolIterator,
   SymbolToStringTag,
   TypedArrayPrototypeGetBuffer,
 } = primordials;
@@ -26,7 +31,10 @@ const {
   kWebCryptoCipherDecrypt,
 } = internalBinding('crypto');
 
-const { TextDecoder, TextEncoder } = require('internal/encoding');
+const {
+  decodeUTF8,
+  encodeUtf8String,
+} = internalBinding('encoding_binding');
 
 const {
   codes: {
@@ -55,6 +63,7 @@ const {
 
 const {
   getBlockSize,
+  jobPromiseThen,
   normalizeAlgorithm,
   normalizeHashName,
   validateMaxBufferLength,
@@ -71,13 +80,20 @@ const {
   randomUUID: _randomUUID,
 } = require('internal/crypto/random');
 
+const {
+  isPromise,
+} = require('internal/util/types');
+
 let webidl;
 
 // WebCrypto methods return promises, including for synchronous validation
 // failures. Keep that conversion in one place so method bodies stay readable.
 function callSubtleCryptoMethod(fn, receiver, args) {
   try {
-    return PromiseResolve(ReflectApply(fn, receiver, args));
+    const result = ReflectApply(fn, receiver, args);
+    // PromiseResolve(promise) reads promise.constructor. Avoid re-wrapping
+    // native promises so Promise.prototype.constructor pollution is ignored.
+    return isPromise(result) ? result : PromiseResolve(result);
   } catch (err) {
     return PromiseReject(err);
   }
@@ -384,7 +400,7 @@ function deriveKeyImpl(
       throw lazyDOMException('Unrecognized algorithm name', 'NotSupportedError');
   }
 
-  return PromisePrototypeThen(bits, (bits) => FunctionPrototypeCall(
+  return jobPromiseThen(bits, (bits) => FunctionPrototypeCall(
     importKeySync,
     this,
     'raw-secret', bits, derivedKeyAlgorithm, extractable, keyUsages,
@@ -569,32 +585,37 @@ function exportKeyRawSecret(key, format) {
   }
 }
 
+// Creates a descriptor for an own enumerable JWK parameter.
+function jwkPropertyDescriptor(value) {
+  return {
+    __proto__: null,
+    configurable: true,
+    enumerable: true,
+    value,
+    writable: true,
+  };
+}
+
 function exportKeyJWK(key) {
   const algorithm = getCryptoKeyAlgorithm(key);
-  const parameters = {
-    key_ops: ArrayPrototypeSlice(getCryptoKeyUsages(key), 0),
-    ext: getCryptoKeyExtractable(key),
-  };
+  let alg;
   switch (algorithm.name) {
     case 'RSASSA-PKCS1-v1_5': {
-      const alg = normalizeHashName(
+      alg = normalizeHashName(
         algorithm.hash.name,
         normalizeHashName.kContextJwkRsa);
-      if (alg) parameters.alg = alg;
       break;
     }
     case 'RSA-PSS': {
-      const alg = normalizeHashName(
+      alg = normalizeHashName(
         algorithm.hash.name,
         normalizeHashName.kContextJwkRsaPss);
-      if (alg) parameters.alg = alg;
       break;
     }
     case 'RSA-OAEP': {
-      const alg = normalizeHashName(
+      alg = normalizeHashName(
         algorithm.hash.name,
         normalizeHashName.kContextJwkRsaOaep);
-      if (alg) parameters.alg = alg;
       break;
     }
     case 'ECDSA':
@@ -620,7 +641,7 @@ function exportKeyJWK(key) {
     case 'Ed25519':
       // Fall through
     case 'Ed448':
-      parameters.alg = algorithm.name;
+      alg = algorithm.name;
       break;
     case 'AES-CTR':
       // Fall through
@@ -631,30 +652,40 @@ function exportKeyJWK(key) {
     case 'AES-OCB':
       // Fall through
     case 'AES-KW':
-      parameters.alg = require('internal/crypto/aes')
+      alg = require('internal/crypto/aes')
         .getAlgorithmName(algorithm.name, algorithm.length);
       break;
     case 'ChaCha20-Poly1305':
-      parameters.alg = 'C20P';
+      alg = 'C20P';
       break;
     case 'HMAC': {
-      const alg = normalizeHashName(
+      alg = normalizeHashName(
         algorithm.hash.name,
         normalizeHashName.kContextJwkHmac);
-      if (alg) parameters.alg = alg;
       break;
     }
     case 'KMAC128':
-      parameters.alg = 'K128';
+      alg = 'K128';
       break;
     case 'KMAC256': {
-      parameters.alg = 'K256';
+      alg = 'K256';
       break;
     }
     default:
       return undefined;
   }
 
+  const parameters = {};
+  const descriptors = {
+    __proto__: null,
+    key_ops: jwkPropertyDescriptor(ArrayPrototypeSlice(getCryptoKeyUsages(key), 0)),
+    ext: jwkPropertyDescriptor(getCryptoKeyExtractable(key)),
+  };
+  if (alg !== undefined) {
+    descriptors.alg = jwkPropertyDescriptor(alg);
+  }
+  ObjectDefineProperties(parameters, descriptors);
+
   return getCryptoKeyHandle(key).exportJwk(parameters, true);
 }
 
@@ -748,6 +779,64 @@ function exportKeyImpl(format, key) {
   return exportKeySync(format, key);
 }
 
+// Parsed JWK arrays are detached from Array.prototype but still need to pass
+// WebIDL sequence conversion, which reads @@iterator from the value.
+function safeArrayIterator() {
+  return new SafeArrayIterator(this);
+}
+
+// The WebCrypto spec parses and stringifies JWKs in a fresh global object.
+// Detach internal JSON values from the current global's mutable prototypes to
+// approximate those fresh-realm semantics without creating a new realm.
+function detachFromUserPrototypes(value) {
+  if (value === null || typeof value !== 'object')
+    return;
+
+  ObjectSetPrototypeOf(value, null);
+
+  if (ArrayIsArray(value)) {
+    ObjectDefineProperty(value, SymbolIterator, {
+      __proto__: null,
+      configurable: true,
+      value: safeArrayIterator,
+    });
+    for (let n = 0; n < value.length; n++)
+      detachFromUserPrototypes(value[n]);
+    return;
+  }
+
+  const keys = ObjectKeys(value);
+  for (let n = 0; n < keys.length; n++)
+    detachFromUserPrototypes(value[keys[n]]);
+}
+
+// Parse wrapped JWK bytes according to WebCrypto's "parse a JWK" procedure.
+function parseJwk(keyData) {
+  let key;
+  try {
+    // WebCrypto parses JWKs in a fresh global. Detach parsed JSON values
+    // from user-mutated prototypes before WebIDL dictionary conversion.
+    // Wrapped JWKs may be produced outside WebCrypto, so parse using the
+    // spec-required UTF-8.
+    const json = decodeUTF8(keyData, false, true);
+    const result = JSONParse(json);
+    detachFromUserPrototypes(result);
+    key = webidl.converters.JsonWebKey(result);
+  } catch (err) {
+    throw lazyDOMException(
+      'Invalid wrapped JWK key',
+      { name: 'DataError', cause: err });
+  }
+
+  if (key.kty === undefined) {
+    throw lazyDOMException(
+      'Invalid wrapped JWK key',
+      'DataError');
+  }
+
+  return key;
+}
+
 function aliasKeyFormat(format) {
   switch (format) {
     case 'raw-public':
@@ -967,15 +1056,21 @@ function wrapKeyImpl(format, key, wrappingKey, algorithm) {
   let keyData = exportKeySync(format, key);
 
   if (format === 'jwk') {
-    const ec = new TextEncoder();
-    const raw = JSONStringify(keyData);
+    // The WebCrypto spec stringifies JWKs in a new global object. Rather
+    // than create a new realm here, detach this internally generated JWK from
+    // user-mutated prototypes so JSON.stringify cannot read inherited toJSON
+    // hooks from the current global.
+    detachFromUserPrototypes(keyData);
+    const json = JSONStringify(keyData);
     // As per the NOTE in step 13 https://w3c.github.io/webcrypto/#SubtleCrypto-method-wrapKey
     // we're padding AES-KW wrapped JWK to make sure it is always a multiple of 8 bytes
     // in length
-    if (algorithm.name === 'AES-KW' && raw.length % 8 !== 0) {
-      keyData = ec.encode(raw + StringPrototypeRepeat(' ', 8 - (raw.length % 8)));
+    // The spec then UTF-8 encodes json.
+    if (algorithm.name === 'AES-KW' && json.length % 8 !== 0) {
+      keyData = encodeUtf8String(
+        json + StringPrototypeRepeat(' ', 8 - (json.length % 8)));
     } else {
-      keyData = ec.encode(raw);
+      keyData = encodeUtf8String(json);
     }
   }
 
@@ -1067,17 +1162,9 @@ function unwrapKeyImpl(
     wrappedKey,
     'unwrapKey');
 
-  return PromisePrototypeThen(keyData, (keyData) => {
+  return jobPromiseThen(keyData, (keyData) => {
     if (format === 'jwk') {
-      // The fatal: true option is only supported in builds that have ICU.
-      const options = process.versions.icu !== undefined ?
-        { fatal: true } : undefined;
-      const dec = new TextDecoder('utf-8', options);
-      try {
-        keyData = JSONParse(dec.decode(keyData));
-      } catch {
-        throw lazyDOMException('Invalid wrapped JWK key', 'DataError');
-      }
+      keyData = parseJwk(keyData);
     }
 
     return FunctionPrototypeCall(
@@ -1453,7 +1540,7 @@ function encapsulateKeyImpl(
       throw lazyDOMException('Unrecognized algorithm name', 'NotSupportedError');
   }
 
-  return PromisePrototypeThen(encapsulateBits, (encapsulateBits) => {
+  return jobPromiseThen(encapsulateBits, (encapsulateBits) => {
     const sharedKey = FunctionPrototypeCall(
       importKeySync,
       this,
@@ -1593,7 +1680,7 @@ function decapsulateKeyImpl(
       throw lazyDOMException('Unrecognized algorithm name', 'NotSupportedError');
   }
 
-  return PromisePrototypeThen(decapsulatedBits, (decapsulatedBits) => FunctionPrototypeCall(
+  return jobPromiseThen(decapsulatedBits, (decapsulatedBits) => FunctionPrototypeCall(
     importKeySync,
     this,
     'raw-secret', decapsulatedBits, normalizedSharedKeyAlgorithm, extractable,