diff --git a/lib/internal/crypto/keys.js b/lib/internal/crypto/keys.js index 152db0a5f5d1..f2281e7fb483 100644 --- a/lib/internal/crypto/keys.js +++ b/lib/internal/crypto/keys.js @@ -1128,6 +1128,9 @@ const { if (depth < 0) return this; + if (!isCryptoKey(this)) + throw new ERR_INVALID_THIS('CryptoKey'); + const opts = { ...options, depth: options.depth == null ? null : options.depth - 1, @@ -1142,14 +1145,20 @@ const { } get type() { + if (!isCryptoKey(this)) + throw new ERR_INVALID_THIS('CryptoKey'); return getCryptoKeyType(this); } get extractable() { + if (!isCryptoKey(this)) + throw new ERR_INVALID_THIS('CryptoKey'); return getCryptoKeyExtractable(this); } get algorithm() { + if (!isCryptoKey(this)) + throw new ERR_INVALID_THIS('CryptoKey'); const slots = getSlots(this); let cached = slots[kSlotClonedAlgorithm]; if (cached === undefined) { @@ -1160,6 +1169,8 @@ const { } get usages() { + if (!isCryptoKey(this)) + throw new ERR_INVALID_THIS('CryptoKey'); const slots = getSlots(this); let cached = slots[kSlotClonedUsages]; if (cached === undefined) { @@ -1210,12 +1221,8 @@ const { return #slots in key || isNativeCryptoKey(key); }; getSlots = (key) => { - if (!key || typeof key !== 'object') - throw new ERR_INVALID_THIS('CryptoKey'); - if (#slots in key) { - const cached = key.#slots; - if (cached !== undefined) return cached; - } + const cached = key.#slots; + if (cached !== undefined) return cached; const slots = nativeGetCryptoKeySlots(key); slots[kSlotAlgorithm] = cloneInternalAlgorithm(slots[kSlotAlgorithm]); if (slots[kSlotSeedData] !== undefined) diff --git a/test/parallel/test-webcrypto-cryptokey-brand-check.js b/test/parallel/test-webcrypto-cryptokey-brand-check.js index 9dd115f00721..9174aebe8f05 100644 --- a/test/parallel/test-webcrypto-cryptokey-brand-check.js +++ b/test/parallel/test-webcrypto-cryptokey-brand-check.js @@ -1,13 +1,10 @@ 'use strict'; -// The four CryptoKey prototype getters (`type`, `extractable`, -// `algorithm`, `usages`) are user-configurable per Web IDL, so they -// can be invoked with an arbitrary `this`. The native callbacks that -// implement them must brand-check their receiver and throw cleanly -// (ERR_INVALID_THIS) rather than crashing the process or returning -// garbage. This test exercises four progressively more hostile -// receiver shapes, including subverting `instanceof` via -// `Symbol.hasInstance`, to make sure the C++ brand check holds. +// CryptoKey prototype getters and methods can be invoked with an +// arbitrary `this`. They must brand-check their receiver and throw +// cleanly (ERR_INVALID_THIS) rather than crashing the process or +// returning garbage. This test exercises invalid receiver shapes, +// including subverting `instanceof` via `Symbol.hasInstance`. // // It also verifies that `util.types.isCryptoKey()` cannot be fooled // by prototype spoofing. @@ -17,7 +14,7 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('node:assert'); -const { types: { isCryptoKey } } = require('node:util'); +const { inspect, types: { isCryptoKey } } = require('node:util'); const { subtle } = globalThis.crypto; (async () => { @@ -29,22 +26,16 @@ const { subtle } = globalThis.crypto; const CryptoKey = key.constructor; - // Capture the underlying prototype getters once, so that subsequent + // Capture the underlying prototype members once, so that subsequent // tampering with `CryptoKey.prototype` cannot affect what we call. - const getters = { - type: Object.getOwnPropertyDescriptor(CryptoKey.prototype, 'type').get, - extractable: - Object.getOwnPropertyDescriptor(CryptoKey.prototype, 'extractable').get, - algorithm: - Object.getOwnPropertyDescriptor(CryptoKey.prototype, 'algorithm').get, - usages: - Object.getOwnPropertyDescriptor(CryptoKey.prototype, 'usages').get, - }; + const descriptors = Object.getOwnPropertyDescriptors(CryptoKey.prototype); // Sanity: each getter works on a real CryptoKey. - Object.entries(getters).forEach(([name, getter]) => { - assert.notStrictEqual(getter.call(key), undefined, `baseline ${name}`); - }); + for (const name of Reflect.ownKeys(descriptors)) { + const { get } = descriptors[name]; + if (get !== undefined) + Reflect.apply(get, key, []); + } assert.strictEqual(isCryptoKey(key), true); assert.strictEqual(Object.hasOwn(CryptoKey, 'getSlots'), false); const internalProto = Object.getPrototypeOf(key); @@ -56,36 +47,51 @@ const { subtle } = globalThis.crypto; const invalidThis = { code: 'ERR_INVALID_THIS', name: 'TypeError' }; const invalidArgType = { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError' }; + async function assertInvalidReceiver(receiver) { + for (const name of Reflect.ownKeys(descriptors)) { + if (name === 'constructor') continue; + const descriptor = descriptors[name]; + const args = name === inspect.custom ? [0, {}] : []; + for (const kind of ['get', 'set', 'value']) { + const member = descriptor[kind]; + if (typeof member !== 'function') continue; + await assert.rejects( + async () => Reflect.apply(member, receiver, args), + invalidThis, + `CryptoKey.${String(name)} (${kind})`, + ); + } + } + } + // Plain object receiver. - Object.entries(getters).forEach(([, getter]) => { - assert.throws(() => getter.call({}), invalidThis); - }); + await assertInvalidReceiver({}); // Null-prototype object receiver. - Object.entries(getters).forEach(([, getter]) => { - assert.throws(() => getter.call({ __proto__: null }), invalidThis); - }); + await assertInvalidReceiver({ __proto__: null }); // Primitive receiver. - Object.entries(getters).forEach(([, getter]) => { - assert.throws(() => getter.call(1), invalidThis); - }); + await assertInvalidReceiver(1); // Null. - Object.entries(getters).forEach(([, getter]) => { - // eslint-disable-next-line no-useless-call - assert.throws(() => getter.call(null), invalidThis); - }); + await assertInvalidReceiver(null); // Undefined. - Object.entries(getters).forEach(([, getter]) => { - assert.throws(() => getter.call(), invalidThis); - }); + await assertInvalidReceiver(undefined); // Function - Object.entries(getters).forEach(([, getter]) => { - assert.throws(() => getter.call(function() {}), invalidThis); - }); + await assertInvalidReceiver(function() {}); + + const revoked = Proxy.revocable(key, {}); + revoked.revoke(); + for (const receiver of [ + { __proto__: CryptoKey.prototype }, + { __proto__: key }, + new Proxy(key, {}), + revoked.proxy, + ]) { + await assertInvalidReceiver(receiver); + } // Prototype spoofing with InternalCryptoKey.prototype must not pass // util.types.isCryptoKey(). @@ -111,9 +117,7 @@ const { subtle } = globalThis.crypto; const fake = { foo: 'bar' }; assert.strictEqual(fake instanceof CryptoKey, true); assert.strictEqual(isCryptoKey(fake), false); - Object.entries(getters).forEach(([, getter]) => { - assert.throws(() => getter.call(fake), invalidThis); - }); + await assertInvalidReceiver(fake); // Subverted `instanceof` plus a real BaseObject of a different // kind (a Buffer) as the receiver. Without the C++ tag check @@ -121,13 +125,11 @@ const { subtle } = globalThis.crypto; const buf = Buffer.alloc(16); assert.strictEqual(buf instanceof CryptoKey, true); assert.strictEqual(isCryptoKey(buf), false); - Object.entries(getters).forEach(([, getter]) => { - assert.throws(() => getter.call(buf), invalidThis); - }); + await assertInvalidReceiver(buf); // The real CryptoKey continues to work after all of the above. - assert.strictEqual(getters.type.call(key), 'secret'); - assert.strictEqual(getters.extractable.call(key), true); - assert.strictEqual(getters.algorithm.call(key).name, 'HMAC'); - assert.deepStrictEqual(getters.usages.call(key), ['sign']); + assert.strictEqual(descriptors.type.get.call(key), 'secret'); + assert.strictEqual(descriptors.extractable.get.call(key), true); + assert.strictEqual(descriptors.algorithm.get.call(key).name, 'HMAC'); + assert.deepStrictEqual(descriptors.usages.get.call(key), ['sign']); })().then(common.mustCall()); diff --git a/test/parallel/test-webcrypto-cryptokey-clone-transfer.js b/test/parallel/test-webcrypto-cryptokey-clone-transfer.js index 4983e1c0bda3..2567ac69454d 100644 --- a/test/parallel/test-webcrypto-cryptokey-clone-transfer.js +++ b/test/parallel/test-webcrypto-cryptokey-clone-transfer.js @@ -17,6 +17,7 @@ if (!common.hasCrypto) common.skip('missing crypto'); const assert = require('node:assert'); +const { KeyObject } = require('node:crypto'); const { inspect } = require('node:util'); const { once } = require('node:events'); const { Worker, MessageChannel } = require('node:worker_threads'); @@ -320,6 +321,17 @@ async function checkRsaPssTransferToWorker({ publicKey, privateKey }) { { name: 'AES-GCM', iv }, k, ciphertext); assert.deepStrictEqual(Buffer.from(decrypted), plaintext); } + + const bytes = new Uint8Array(await subtle.exportKey('raw', key)); + const nullPrototypeClone = structuredClone(key); + Object.setPrototypeOf(nullPrototypeClone, null); + assert.deepStrictEqual( + new Uint8Array(await subtle.exportKey('raw', nullPrototypeClone)), bytes); + const typeGetter = Object.getOwnPropertyDescriptor(key.constructor.prototype, 'type').get; + const customInspect = key[inspect.custom]; + assert.strictEqual(typeGetter.call(nullPrototypeClone), 'secret'); + assert.strictEqual(typeof customInspect.call(nullPrototypeClone, 0, {}), 'string'); + assert.deepStrictEqual(KeyObject.from(structuredClone(key)).export(), Buffer.from(bytes)); } // ECDSA keypair (public extractable, private non-extractable)