From 98a7ff59a1697d3bcf220240465c521392ca2e19 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Sun, 6 Sep 2026 10:14:34 +0200 Subject: [PATCH] lib: use Web IDL interface brand checks Use explicit brand predicates for interface conversion instead of prototype ancestry. Update CryptoKey and AbortSignal together with the shared converter contract. Read internal AbortSignal state during composition. Preserve genuine signals after prototype changes without invoking shadowed getters. Signed-off-by: Filip Skokan Assisted-by: GitHub Copilot --- lib/internal/abort_controller.js | 31 ++++++------ lib/internal/crypto/webidl.js | 4 +- lib/internal/streams/iter/webidl.js | 7 ++- lib/internal/webidl.js | 7 ++- test/parallel/test-internal-webidl.js | 48 +++++++++++++++++-- .../test-webcrypto-cryptokey-brand-check.js | 5 +- test/parallel/test-webcrypto-webidl-brand.js | 38 +++++++++++++++ 7 files changed, 111 insertions(+), 29 deletions(-) create mode 100644 test/parallel/test-webcrypto-webidl-brand.js diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 09b160e9fe5a..48dcdaafba2f 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -149,8 +149,8 @@ function refreshCompositeSignal(signal) { continue; } - if (sourceSignal.aborted) { - abortSignal(signal, sourceSignal.reason); + if (sourceSignal[kAborted]) { + abortSignal(signal, sourceSignal[kReason]); return; } } @@ -170,8 +170,8 @@ function followCompositeSignal(signal) { continue; } - if (sourceSignal.aborted) { - abortSignal(signal, sourceSignal.reason); + if (sourceSignal[kAborted]) { + abortSignal(signal, sourceSignal[kReason]); return; } @@ -217,6 +217,14 @@ function setWeakAbortSignalTimeout(weakRef, delay) { } class AbortSignal extends EventTarget { + #brand; + + static { + converters.AbortSignal = createInterfaceConverter( + 'AbortSignal', + (value) => typeof value === 'object' && value !== null && #brand in value, + ); + } /** * @param {symbol | undefined} dontThrowSymbol @@ -337,8 +345,9 @@ class AbortSignal extends EventTarget { gcPersistentSignals.add(signal); } - if (signal.aborted) { - abortSignal(resultSignal, signal.reason); + refreshCompositeSignal(signal); + if (signal[kAborted]) { + abortSignal(resultSignal, signal[kReason]); return resultSignal; } @@ -348,11 +357,6 @@ class AbortSignal extends EventTarget { } else if (!signal[kSourceSignals]) { continue; } else { - refreshCompositeSignal(signal); - if (signal.aborted) { - abortSignal(resultSignal, signal.reason); - return resultSignal; - } for (const sourceSignalWeakRef of signal[kSourceSignals]) { const sourceSignal = sourceSignalWeakRef.deref(); if (!sourceSignal) { @@ -360,8 +364,8 @@ class AbortSignal extends EventTarget { } assert(!sourceSignal[kComposite]); - if (sourceSignal.aborted) { - abortSignal(resultSignal, sourceSignal.reason); + if (sourceSignal[kAborted]) { + abortSignal(resultSignal, sourceSignal[kReason]); return resultSignal; } @@ -466,7 +470,6 @@ class AbortSignal extends EventTarget { } } -converters.AbortSignal = createInterfaceConverter('AbortSignal', AbortSignal.prototype); converters['sequence'] = createSequenceConverter(converters.AbortSignal); function ClonedAbortSignal() { diff --git a/lib/internal/crypto/webidl.js b/lib/internal/crypto/webidl.js index 7f73d1bd66de..cc24e3342396 100644 --- a/lib/internal/crypto/webidl.js +++ b/lib/internal/crypto/webidl.js @@ -17,10 +17,10 @@ const { const { isUint32, } = require('internal/validators'); -const { CryptoKey } = require('internal/crypto/webcrypto'); const { getCryptoKeyAlgorithm, getCryptoKeyType, + isCryptoKey, } = require('internal/crypto/keys'); const { bigIntArrayToUnsignedInt, @@ -618,7 +618,7 @@ converters.AesCtrParams = createDictionaryConverter( ]); converters.CryptoKey = createInterfaceConverter( - 'CryptoKey', CryptoKey.prototype); + 'CryptoKey', isCryptoKey); converters.EcdhKeyDeriveParams = createDictionaryConverter( 'EcdhKeyDeriveParams', [ diff --git a/lib/internal/streams/iter/webidl.js b/lib/internal/streams/iter/webidl.js index b971fa95071b..273a52443bf2 100644 --- a/lib/internal/streams/iter/webidl.js +++ b/lib/internal/streams/iter/webidl.js @@ -5,10 +5,10 @@ const { convertToInt, createDictionaryConverter, createEnumConverter, - createInterfaceConverter, createSequenceConverter, } = require('internal/webidl'); -const { AbortSignal } = require('internal/abort_controller'); +// Load AbortSignal to register its Web IDL converter in baseConverters. +require('internal/abort_controller'); const { isUint8Array } = require('internal/util/types'); const converters = { __proto__: null }; @@ -38,8 +38,7 @@ function allowStreamBufferOptions(options) { }; } -converters.AbortSignal = createInterfaceConverter( - 'AbortSignal', AbortSignal.prototype); +converters.AbortSignal = baseConverters.AbortSignal; converters.BackpressurePolicy = createEnumConverter('BackpressurePolicy', [ 'strict', 'unbounded', diff --git a/lib/internal/webidl.js b/lib/internal/webidl.js index 71513bbe86f4..2f86117afe9c 100644 --- a/lib/internal/webidl.js +++ b/lib/internal/webidl.js @@ -21,7 +21,6 @@ const { NumberMAX_SAFE_INTEGER, NumberMIN_SAFE_INTEGER, ObjectPrototypeHasOwnProperty, - ObjectPrototypeIsPrototypeOf, SafeArrayIterator, SafeSet, String, @@ -873,13 +872,13 @@ function createSequenceConverter(converter) { * Creates a converter for a Web IDL interface type. * @see https://webidl.spec.whatwg.org/#js-interface * @param {string} name Interface identifier. - * @param {object} prototype Interface prototype object. + * @param {(value: any) => boolean} brandCheck Interface brand predicate. * @returns {Converter} */ -function createInterfaceConverter(name, prototype) { +function createInterfaceConverter(name, brandCheck) { return (V, options = kEmptyObject) => { // Web IDL interface conversion step 1: return V if it implements I. - if (ObjectPrototypeIsPrototypeOf(prototype, V)) { + if (brandCheck(V)) { return V; } // Step 2: otherwise throw. diff --git a/test/parallel/test-internal-webidl.js b/test/parallel/test-internal-webidl.js index bd08648549be..406a625c26ab 100644 --- a/test/parallel/test-internal-webidl.js +++ b/test/parallel/test-internal-webidl.js @@ -1,7 +1,7 @@ // Flags: --expose-internals 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const vm = require('vm'); const webidl = require('internal/webidl'); @@ -511,10 +511,16 @@ assert.throws(() => webidl.requiredArguments(1, 2, opts), { } { - class Example {} + class Example { + #brand; + + static is(value) { + return typeof value === 'object' && value !== null && #brand in value; + } + } const converter = webidl.createInterfaceConverter( 'Example', - Example.prototype); + Example.is); const example = new Example(); assert.strictEqual(converter(example), example); @@ -523,6 +529,42 @@ assert.throws(() => webidl.requiredArguments(1, 2, opts), { code: 'ERR_INVALID_ARG_TYPE', message: 'Prefix: Context is not of type Example.', }); + assertInvalidArgType(() => converter({ __proto__: Example.prototype })); + assertInvalidArgType(() => converter(new Proxy(example, {}))); + Object.setPrototypeOf(example, null); + assert.strictEqual(converter(example), example); +} + +{ + const signal = AbortSignal.abort('reason'); + for (const value of [ + Object.create(AbortSignal.prototype, { aborted: { value: false } }), + { __proto__: signal }, + Object.create(AbortSignal.prototype, Object.getOwnPropertyDescriptors(signal)), + new Proxy(signal, {}), + ]) { + assertInvalidArgType(() => converters.AbortSignal(value)); + assertInvalidArgType(() => AbortSignal.any([value])); + } + + Object.setPrototypeOf(signal, null); + assert.strictEqual(converters.AbortSignal(signal), signal); + const composite = AbortSignal.any([signal]); + assert.strictEqual(composite.aborted, true); + assert.strictEqual(composite.reason, 'reason'); +} + +{ + const controller = new AbortController(); + Object.defineProperties(controller.signal, { + aborted: { get: common.mustNotCall('Unexpected aborted getter') }, + reason: { get: common.mustNotCall('Unexpected reason getter') }, + }); + const composite = AbortSignal.any([controller.signal]); + assert.strictEqual(composite.aborted, false); + controller.abort('reason'); + assert.strictEqual(composite.aborted, true); + assert.strictEqual(composite.reason, 'reason'); } { diff --git a/test/parallel/test-webcrypto-cryptokey-brand-check.js b/test/parallel/test-webcrypto-cryptokey-brand-check.js index 3fe8aaa181a2..9dd115f00721 100644 --- a/test/parallel/test-webcrypto-cryptokey-brand-check.js +++ b/test/parallel/test-webcrypto-cryptokey-brand-check.js @@ -54,6 +54,7 @@ const { subtle } = globalThis.crypto; assert.strictEqual(Object.getPrototypeOf(internalProto), CryptoKey.prototype); const invalidThis = { code: 'ERR_INVALID_THIS', name: 'TypeError' }; + const invalidArgType = { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError' }; // Plain object receiver. Object.entries(getters).forEach(([, getter]) => { @@ -94,10 +95,10 @@ const { subtle } = globalThis.crypto; assert.strictEqual(isCryptoKey(spoofed), false); await assert.rejects( subtle.sign('HMAC', spoofed, Buffer.from('payload')), - invalidThis); + invalidArgType); await assert.rejects( subtle.exportKey('jwk', spoofed), - invalidThis); + invalidArgType); // Subvert `instanceof CryptoKey` via Symbol.hasInstance, then // invoke the native getters on a forged object. The C++ tag diff --git a/test/parallel/test-webcrypto-webidl-brand.js b/test/parallel/test-webcrypto-webidl-brand.js new file mode 100644 index 000000000000..d4d785684906 --- /dev/null +++ b/test/parallel/test-webcrypto-webidl-brand.js @@ -0,0 +1,38 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const { subtle } = globalThis.crypto; +const { CryptoKey } = require('internal/crypto/keys'); +const { converters } = require('internal/crypto/webidl'); + +async function main() { + const bytes = new Uint8Array(16); + const key = await subtle.importKey('raw', bytes, 'AES-GCM', true, ['encrypt']); + + for (const value of [ + { __proto__: CryptoKey.prototype }, + { __proto__: key }, + Object.create(CryptoKey.prototype, Object.getOwnPropertyDescriptors(key)), + new Proxy(key, {}), + ]) { + assert.throws(() => converters.CryptoKey(value), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_TYPE', + }); + await assert.rejects(subtle.exportKey('raw', value), { + name: 'TypeError', + code: 'ERR_INVALID_ARG_TYPE', + }); + } + + Object.setPrototypeOf(key, null); + assert.strictEqual(converters.CryptoKey(key), key); + assert.deepStrictEqual(new Uint8Array(await subtle.exportKey('raw', key)), bytes); +} + +main().then(common.mustCall());