From a28436b97a0fa691bce127e222b52c851f0fa616 Mon Sep 17 00:00:00 2001 From: James M Snell <jasnell@gmail.com> Date: Sun, 15 Aug 2021 07:50:24 -0700 Subject: [PATCH 1/3] workers: add brand checks for detached MessageEvent accessors Signed-off-by: James M Snell <jasnell@gmail.com> --- lib/internal/worker/io.js | 44 ++++++++++++++++--- test/parallel/test-messageevent-brandcheck.js | 21 +++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-messageevent-brandcheck.js diff --git a/lib/internal/worker/io.js b/lib/internal/worker/io.js index 95c5515d1414eb..9ec95509b56778 100644 --- a/lib/internal/worker/io.js +++ b/lib/internal/worker/io.js @@ -107,6 +107,10 @@ function validateMessagePort(port, name) { throw new ERR_INVALID_ARG_TYPE(name, 'MessagePort', port); } +function isMessageEvent(value) { + return value != null && kData in value; +} + class MessageEvent extends Event { constructor(type, { data = null, @@ -131,19 +135,49 @@ class MessageEvent extends Event { ObjectDefineProperties(MessageEvent.prototype, { data: { - get() { return this[kData]; }, enumerable: true, configurable: true + get() { + if (!isMessageEvent(this)) + throw new ERR_INVALID_THIS('MessageEvent'); + return this[kData]; + }, + enumerable: true, + configurable: true, }, origin: { - get() { return this[kOrigin]; }, enumerable: true, configurable: true + get() { + if (!isMessageEvent(this)) + throw new ERR_INVALID_THIS('MessageEvent'); + return this[kOrigin]; + }, + enumerable: true, + configurable: true, }, lastEventId: { - get() { return this[kLastEventId]; }, enumerable: true, configurable: true + get() { + if (!isMessageEvent(this)) + throw new ERR_INVALID_THIS('MessageEvent'); + return this[kLastEventId]; + }, + enumerable: true, + configurable: true, }, source: { - get() { return this[kSource]; }, enumerable: true, configurable: true + get() { + if (!isMessageEvent(this)) + throw new ERR_INVALID_THIS('MessageEvent'); + return this[kSource]; + }, + enumerable: true, + configurable: true, }, ports: { - get() { return this[kPorts]; }, enumerable: true, configurable: true + get() { + if (!isMessageEvent(this)) + throw new ERR_INVALID_THIS('MessageEvent'); + return this[kPorts]; + }, + enumerable: true, + configurable: true, }, }); diff --git a/test/parallel/test-messageevent-brandcheck.js b/test/parallel/test-messageevent-brandcheck.js new file mode 100644 index 00000000000000..2174b3f7bbc94c --- /dev/null +++ b/test/parallel/test-messageevent-brandcheck.js @@ -0,0 +1,21 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); +const assert = require('assert'); + +const { + MessageEvent, +} = require('internal/worker/io'); + +[ + 'data', + 'origin', + 'lastEventId', + 'source', + 'ports', +].forEach((i) => { + assert.throws(() => Reflect.get(MessageEvent.prototype, i, {}), { + code: 'ERR_INVALID_THIS', + }); +}); From 5a9deb43ce55ae94d80cde7770a730e6d2df956a Mon Sep 17 00:00:00 2001 From: James M Snell <jasnell@gmail.com> Date: Sun, 15 Aug 2021 08:37:20 -0700 Subject: [PATCH 2/3] event_target: add brand checks for detached accessors --- lib/internal/event_target.js | 208 +++++++++++++++---- test/parallel/test-eventtarget-brandcheck.js | 69 ++++++ 2 files changed, 237 insertions(+), 40 deletions(-) create mode 100644 test/parallel/test-eventtarget-brandcheck.js diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index ce867cc1e35793..03b2540878c625 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -8,6 +8,7 @@ const { FunctionPrototypeCall, NumberIsInteger, ObjectAssign, + ObjectCreate, ObjectDefineProperties, ObjectDefineProperty, ObjectGetOwnPropertyDescriptor, @@ -39,6 +40,7 @@ const { customInspectSymbol } = require('internal/util'); const { inspect } = require('util'); const kIsEventTarget = SymbolFor('nodejs.event_target'); +const kIsNodeEventTarget = Symbol('kIsNodeEventTarget'); const EventEmitter = require('events'); const { @@ -80,6 +82,10 @@ const isTrusted = ObjectGetOwnPropertyDescriptor({ } }, 'isTrusted').get; +function isEvent(value) { + return typeof value?.[kType] === 'string'; +} + class Event { constructor(type, options = null) { if (arguments.length === 0) @@ -110,6 +116,8 @@ class Event { } [customInspectSymbol](depth, options) { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); const name = this.constructor.name; if (depth < 0) return name; @@ -127,46 +135,111 @@ class Event { } stopImmediatePropagation() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); this[kStop] = true; } preventDefault() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); this[kDefaultPrevented] = true; } - get target() { return this[kTarget]; } - get currentTarget() { return this[kTarget]; } - get srcElement() { return this[kTarget]; } + get target() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kTarget]; + } - get type() { return this[kType]; } + get currentTarget() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kTarget]; + } - get cancelable() { return this[kCancelable]; } + get srcElement() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kTarget]; + } + + get type() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kType]; + } + + get cancelable() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kCancelable]; + } get defaultPrevented() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); return this[kCancelable] && this[kDefaultPrevented]; } - get timeStamp() { return this[kTimestamp]; } + get timeStamp() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kTimestamp]; + } // The following are non-op and unused properties/methods from Web API Event. // These are not supported in Node.js and are provided purely for // API completeness. - composedPath() { return this[kIsBeingDispatched] ? [this[kTarget]] : []; } - get returnValue() { return !this.defaultPrevented; } - get bubbles() { return this[kBubbles]; } - get composed() { return this[kComposed]; } + composedPath() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kIsBeingDispatched] ? [this[kTarget]] : []; + } + + get returnValue() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return !this.defaultPrevented; + } + + get bubbles() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kBubbles]; + } + + get composed() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kComposed]; + } + get eventPhase() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); return this[kIsBeingDispatched] ? Event.AT_TARGET : Event.NONE; } - get cancelBubble() { return this[kPropagationStopped]; } + + get cancelBubble() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); + return this[kPropagationStopped]; + } + set cancelBubble(value) { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); if (value) { this.stopPropagation(); } } + stopPropagation() { + if (!isEvent(this)) + throw new ERR_INVALID_THIS('Event'); this[kPropagationStopped] = true; } @@ -176,12 +249,34 @@ class Event { static BUBBLING_PHASE = 3; } -ObjectDefineProperty(Event.prototype, SymbolToStringTag, { - writable: false, - enumerable: false, - configurable: true, - value: 'Event', -}); +const kEnumerableProperty = ObjectCreate(null); +kEnumerableProperty.enumerable = true; + +ObjectDefineProperties( + Event.prototype, { + [SymbolToStringTag]: { + writable: false, + enumerable: false, + configurable: true, + value: 'Event', + }, + stopImmediatePropagation: kEnumerableProperty, + preventDefault: kEnumerableProperty, + target: kEnumerableProperty, + currentTarget: kEnumerableProperty, + srcElement: kEnumerableProperty, + type: kEnumerableProperty, + cancelable: kEnumerableProperty, + defaultPrevented: kEnumerableProperty, + timeStamp: kEnumerableProperty, + composedPath: kEnumerableProperty, + returnValue: kEnumerableProperty, + bubbles: kEnumerableProperty, + composed: kEnumerableProperty, + eventPhase: kEnumerableProperty, + cancelBubble: kEnumerableProperty, + stopPropagation: kEnumerableProperty, + }); class NodeCustomEvent extends Event { constructor(type, options) { @@ -297,6 +392,8 @@ class EventTarget { [kRemoveListener](size, type, listener, capture) {} addEventListener(type, listener, options = {}) { + if (!isEventTarget(this)) + throw new ERR_INVALID_THIS('EventTarget'); if (arguments.length < 2) throw new ERR_MISSING_ARGS('type', 'listener'); @@ -368,6 +465,8 @@ class EventTarget { } removeEventListener(type, listener, options = {}) { + if (!isEventTarget(this)) + throw new ERR_INVALID_THIS('EventTarget'); if (!shouldAddListener(listener)) return; @@ -393,12 +492,12 @@ class EventTarget { } dispatchEvent(event) { - if (!(event instanceof Event)) - throw new ERR_INVALID_ARG_TYPE('event', 'Event', event); - if (!isEventTarget(this)) throw new ERR_INVALID_THIS('EventTarget'); + if (!(event instanceof Event)) + throw new ERR_INVALID_ARG_TYPE('event', 'Event', event); + if (event[kIsBeingDispatched]) throw new ERR_EVENT_RECURSION(event.type); @@ -479,6 +578,8 @@ class EventTarget { return new NodeCustomEvent(type, { detail: nodeValue }); } [customInspectSymbol](depth, options) { + if (!isEventTarget(this)) + throw new ERR_INVALID_THIS('EventTarget'); const name = this.constructor.name; if (depth < 0) return name; @@ -492,15 +593,15 @@ class EventTarget { } ObjectDefineProperties(EventTarget.prototype, { - addEventListener: { enumerable: true }, - removeEventListener: { enumerable: true }, - dispatchEvent: { enumerable: true } -}); -ObjectDefineProperty(EventTarget.prototype, SymbolToStringTag, { - writable: false, - enumerable: false, - configurable: true, - value: 'EventTarget', + addEventListener: kEnumerableProperty, + removeEventListener: kEnumerableProperty, + dispatchEvent: kEnumerableProperty, + [SymbolToStringTag]: { + writable: false, + enumerable: false, + configurable: true, + value: 'EventTarget', + } }); function initNodeEventTarget(self) { @@ -508,6 +609,7 @@ function initNodeEventTarget(self) { } class NodeEventTarget extends EventTarget { + static [kIsNodeEventTarget] = true; static defaultMaxListeners = 10; constructor() { @@ -516,42 +618,60 @@ class NodeEventTarget extends EventTarget { } setMaxListeners(n) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); EventEmitter.setMaxListeners(n, this); } getMaxListeners() { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); return this[kMaxEventTargetListeners]; } eventNames() { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); return ArrayFrom(this[kEvents].keys()); } listenerCount(type) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); const root = this[kEvents].get(String(type)); return root !== undefined ? root.size : 0; } off(type, listener, options) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); this.removeEventListener(type, listener, options); return this; } removeListener(type, listener, options) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); this.removeEventListener(type, listener, options); return this; } on(type, listener) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); this.addEventListener(type, listener, { [kIsNodeStyleListener]: true }); return this; } addListener(type, listener) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); this.addEventListener(type, listener, { [kIsNodeStyleListener]: true }); return this; } emit(type, arg) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); validateString(type, 'type'); const hadListeners = this.listenerCount(type) > 0; this[kHybridDispatch](arg, type); @@ -559,12 +679,16 @@ class NodeEventTarget extends EventTarget { } once(type, listener) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); this.addEventListener(type, listener, { once: true, [kIsNodeStyleListener]: true }); return this; } removeAllListeners(type) { + if (!isNodeEventTarget(this)) + throw new ERR_INVALID_THIS('NodeEventTarget'); if (type !== undefined) { this[kEvents].delete(String(type)); } else { @@ -576,17 +700,17 @@ class NodeEventTarget extends EventTarget { } ObjectDefineProperties(NodeEventTarget.prototype, { - setMaxListeners: { enumerable: true }, - getMaxListeners: { enumerable: true }, - eventNames: { enumerable: true }, - listenerCount: { enumerable: true }, - off: { enumerable: true }, - removeListener: { enumerable: true }, - on: { enumerable: true }, - addListener: { enumerable: true }, - once: { enumerable: true }, - emit: { enumerable: true }, - removeAllListeners: { enumerable: true }, + setMaxListeners: kEnumerableProperty, + getMaxListeners: kEnumerableProperty, + eventNames: kEnumerableProperty, + listenerCount: kEnumerableProperty, + off: kEnumerableProperty, + removeListener: kEnumerableProperty, + on: kEnumerableProperty, + addListener: kEnumerableProperty, + once: kEnumerableProperty, + emit: kEnumerableProperty, + removeAllListeners: kEnumerableProperty, }); // EventTarget API @@ -631,6 +755,10 @@ function isEventTarget(obj) { return obj?.constructor?.[kIsEventTarget]; } +function isNodeEventTarget(obj) { + return obj?.constructor?.[kIsNodeEventTarget]; +} + function addCatch(promise) { const then = promise.then; if (typeof then === 'function') { diff --git a/test/parallel/test-eventtarget-brandcheck.js b/test/parallel/test-eventtarget-brandcheck.js new file mode 100644 index 00000000000000..ffb0dcdaf08fb5 --- /dev/null +++ b/test/parallel/test-eventtarget-brandcheck.js @@ -0,0 +1,69 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); +const assert = require('assert'); + +const { + Event, + EventTarget, + NodeEventTarget, +} = require('internal/event_target'); + +[ + 'target', + 'currentTarget', + 'srcElement', + 'type', + 'cancelable', + 'defaultPrevented', + 'timeStamp', + 'returnValue', + 'bubbles', + 'composed', + 'eventPhase', +].forEach((i) => { + assert.throws(() => Reflect.get(Event.prototype, i, {}), { + code: 'ERR_INVALID_THIS', + }); +}); + +[ + 'stopImmediatePropagation', + 'preventDefault', + 'composedPath', + 'cancelBubble', + 'stopPropagation', +].forEach((i) => { + assert.throws(() => Reflect.apply(Event.prototype[i], [], {}), { + code: 'ERR_INVALID_THIS', + }); +}); + +[ + 'addEventListener', + 'removeEventListener', + 'dispatchEvent', +].forEach((i) => { + assert.throws(() => Reflect.apply(EventTarget.prototype[i], [], {}), { + code: 'ERR_INVALID_THIS', + }); +}); + +[ + 'setMaxListeners', + 'getMaxListeners', + 'eventNames', + 'listenerCount', + 'off', + 'removeListener', + 'on', + 'addListener', + 'once', + 'emit', + 'removeAllListeners', +].forEach((i) => { + assert.throws(() => Reflect.apply(NodeEventTarget.prototype[i], [], {}), { + code: 'ERR_INVALID_THIS', + }); +}); From ae8fe16183d495dc6a5756b49cbc18c6fab47d04 Mon Sep 17 00:00:00 2001 From: James M Snell <jasnell@gmail.com> Date: Mon, 16 Aug 2021 07:12:22 -0700 Subject: [PATCH 3/3] event_target: protect property defs against prototype polution Signed-off-by: James M Snell <jasnell@gmail.com> --- lib/internal/worker/io.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/internal/worker/io.js b/lib/internal/worker/io.js index 9ec95509b56778..5d03f43ef6df62 100644 --- a/lib/internal/worker/io.js +++ b/lib/internal/worker/io.js @@ -142,6 +142,7 @@ ObjectDefineProperties(MessageEvent.prototype, { }, enumerable: true, configurable: true, + set: undefined, }, origin: { get() { @@ -151,6 +152,7 @@ ObjectDefineProperties(MessageEvent.prototype, { }, enumerable: true, configurable: true, + set: undefined, }, lastEventId: { get() { @@ -160,6 +162,7 @@ ObjectDefineProperties(MessageEvent.prototype, { }, enumerable: true, configurable: true, + set: undefined, }, source: { get() { @@ -169,6 +172,7 @@ ObjectDefineProperties(MessageEvent.prototype, { }, enumerable: true, configurable: true, + set: undefined, }, ports: { get() { @@ -178,6 +182,7 @@ ObjectDefineProperties(MessageEvent.prototype, { }, enumerable: true, configurable: true, + set: undefined, }, });