From 9c1061f06d763a8587c81dfff13dc64cf3140561 Mon Sep 17 00:00:00 2001 From: david0xd Date: Thu, 2 Mar 2023 16:21:59 +0100 Subject: [PATCH 1/3] Add fix for event security (MessageEvent source issue) --- src/window/WindowPostMessageStream.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/window/WindowPostMessageStream.ts b/src/window/WindowPostMessageStream.ts index 7bdff60..8bdd0d3 100644 --- a/src/window/WindowPostMessageStream.ts +++ b/src/window/WindowPostMessageStream.ts @@ -11,6 +11,17 @@ interface WindowPostMessageStreamArgs { targetWindow?: Window; } +// @ts-expect-error Object should be defined. +const getSource = Object.getOwnPropertyDescriptor( + MessageEvent.prototype, + 'source', +).get; +// @ts-expect-error Object should be defined. +const getOrigin = Object.getOwnPropertyDescriptor( + MessageEvent.prototype, + 'origin', +).get; + /** * A {@link Window.postMessage} stream. */ @@ -78,8 +89,11 @@ export class WindowPostMessageStream extends BasePostMessageStream { const message = event.data; if ( - (this._targetOrigin !== '*' && event.origin !== this._targetOrigin) || - event.source !== this._targetWindow || + (this._targetOrigin !== '*' && + // @ts-expect-error getOrigin should be defined. + getOrigin.call(event) !== this._targetOrigin) || + // @ts-expect-error getSource should be defined. + getSource.call(event) !== this._targetWindow || !isValidStreamMessage(message) || message.target !== this._name ) { From ef1b439139ecb4d8e979697f1e3d2c4b571efa7c Mon Sep 17 00:00:00 2001 From: david0xd Date: Fri, 3 Mar 2023 15:01:11 +0100 Subject: [PATCH 2/3] Refactor (remove ts-expect-error and add assertions) --- src/window/WindowPostMessageStream.ts | 34 ++++++++++++++++++++------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/window/WindowPostMessageStream.ts b/src/window/WindowPostMessageStream.ts index 8bdd0d3..6ad1692 100644 --- a/src/window/WindowPostMessageStream.ts +++ b/src/window/WindowPostMessageStream.ts @@ -1,3 +1,4 @@ +import { assert } from '@metamask/utils'; import { BasePostMessageStream, PostMessageEvent, @@ -11,16 +12,25 @@ interface WindowPostMessageStreamArgs { targetWindow?: Window; } -// @ts-expect-error Object should be defined. -const getSource = Object.getOwnPropertyDescriptor( +const getSourceDescriptor = Object.getOwnPropertyDescriptor( MessageEvent.prototype, 'source', -).get; -// @ts-expect-error Object should be defined. -const getOrigin = Object.getOwnPropertyDescriptor( +); +assert( + getSourceDescriptor, + 'MessageEvent.prototype.source getter is not defined.', +); +const getSource = getSourceDescriptor.get; + +const getOriginDescriptor = Object.getOwnPropertyDescriptor( MessageEvent.prototype, 'origin', -).get; +); +assert( + getOriginDescriptor, + 'MessageEvent.prototype.origin getter is not defined.', +); +const getOrigin = getOriginDescriptor.get; /** * A {@link Window.postMessage} stream. @@ -88,11 +98,19 @@ export class WindowPostMessageStream extends BasePostMessageStream { private _onMessage(event: PostMessageEvent): void { const message = event.data; + assert( + getOrigin, + `Function was expected for 'getOrigin', but ${typeof getOrigin} was provided instead.`, + ); + + assert( + getSource, + `Function was expected for 'getSource', but ${typeof getSource} was provided instead.`, + ); + if ( (this._targetOrigin !== '*' && - // @ts-expect-error getOrigin should be defined. getOrigin.call(event) !== this._targetOrigin) || - // @ts-expect-error getSource should be defined. getSource.call(event) !== this._targetWindow || !isValidStreamMessage(message) || message.target !== this._name From b3d25001f8559b2863f3a4567698e102f5e7a660 Mon Sep 17 00:00:00 2001 From: david0xd Date: Mon, 6 Mar 2023 15:42:37 +0100 Subject: [PATCH 3/3] Refactor (review proposal) --- src/window/WindowPostMessageStream.ts | 40 +++++++++------------------ 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/src/window/WindowPostMessageStream.ts b/src/window/WindowPostMessageStream.ts index 6ad1692..76df938 100644 --- a/src/window/WindowPostMessageStream.ts +++ b/src/window/WindowPostMessageStream.ts @@ -12,25 +12,19 @@ interface WindowPostMessageStreamArgs { targetWindow?: Window; } -const getSourceDescriptor = Object.getOwnPropertyDescriptor( +/* istanbul ignore next */ +const getSource = Object.getOwnPropertyDescriptor( MessageEvent.prototype, 'source', -); -assert( - getSourceDescriptor, - 'MessageEvent.prototype.source getter is not defined.', -); -const getSource = getSourceDescriptor.get; - -const getOriginDescriptor = Object.getOwnPropertyDescriptor( +)?.get; +assert(getSource, 'MessageEvent.prototype.source getter is not defined.'); + +/* istanbul ignore next */ +const getOrigin = Object.getOwnPropertyDescriptor( MessageEvent.prototype, 'origin', -); -assert( - getOriginDescriptor, - 'MessageEvent.prototype.origin getter is not defined.', -); -const getOrigin = getOriginDescriptor.get; +)?.get; +assert(getOrigin, 'MessageEvent.prototype.origin getter is not defined.'); /** * A {@link Window.postMessage} stream. @@ -98,25 +92,17 @@ export class WindowPostMessageStream extends BasePostMessageStream { private _onMessage(event: PostMessageEvent): void { const message = event.data; - assert( - getOrigin, - `Function was expected for 'getOrigin', but ${typeof getOrigin} was provided instead.`, - ); - - assert( - getSource, - `Function was expected for 'getSource', but ${typeof getSource} was provided instead.`, - ); - + /* eslint-disable @typescript-eslint/no-non-null-assertion */ if ( (this._targetOrigin !== '*' && - getOrigin.call(event) !== this._targetOrigin) || - getSource.call(event) !== this._targetWindow || + getOrigin!.call(event) !== this._targetOrigin) || + getSource!.call(event) !== this._targetWindow || !isValidStreamMessage(message) || message.target !== this._name ) { return; } + /* eslint-enable @typescript-eslint/no-non-null-assertion */ this._onData(message.data); }