From e669ef36f97df0bcfd9af75de5cdd2cfaf462435 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 7 Feb 2025 12:34:42 +0100 Subject: [PATCH 1/4] perf: Stop using additional encoding for messages in mobile --- .../src/services/webview/WebViewMessageStream.ts | 9 ++------- .../src/webview/WebViewExecutorStream.ts | 4 +--- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts index b8720353a5..8e0816f8ff 100644 --- a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts +++ b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts @@ -2,7 +2,7 @@ import type { PostMessageEvent } from '@metamask/post-message-stream'; import { BasePostMessageStream } from '@metamask/post-message-stream'; import { isValidStreamMessage } from '@metamask/post-message-stream/dist/utils'; import { logError } from '@metamask/snaps-utils'; -import { assert, bytesToBase64, stringToBytes } from '@metamask/utils'; +import { assert } from '@metamask/utils'; export type WebViewInterface = { injectJavaScript(js: string): void; @@ -65,12 +65,7 @@ export class WebViewMessageStream extends BasePostMessageStream { data, }); - // To prevent XSS, we base64 encode the message before injecting it. - // This adds significant performance overhead. - // TODO: Should we use mobile native base64 here? - const bytes = stringToBytes(json); - const base64 = bytesToBase64(bytes); - this.#webView.injectJavaScript(`window.postMessage('${base64}')`); + this.#webView.injectJavaScript(`window.postMessage('${json}')`); } private _onMessage(event: PostMessageEvent): void { diff --git a/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts b/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts index 4f75d6baa9..b762c73fcf 100644 --- a/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts +++ b/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts @@ -1,7 +1,6 @@ import type { PostMessageEvent } from '@metamask/post-message-stream'; import { BasePostMessageStream } from '@metamask/post-message-stream'; import { isValidStreamMessage } from '@metamask/post-message-stream/dist/utils'; -import { base64ToBytes, bytesToString } from '@metamask/utils'; type WebViewExecutorStreamArgs = { name: string; @@ -66,8 +65,7 @@ export class WebViewExecutorStream extends BasePostMessageStream { return; } - const bytes = base64ToBytes(event.data); - const message = JSON.parse(bytesToString(bytes)); + const message = JSON.parse(event.data); // Notice that we don't check targetWindow or targetOrigin here. // This doesn't seem possible to do in RN. From b5bf9319ec66df37c7e1639bab9a9862d858dcb4 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 11 Feb 2025 12:52:08 +0100 Subject: [PATCH 2/4] Encode as bytes --- .../src/services/webview/WebViewMessageStream.ts | 6 ++++-- .../src/webview/WebViewExecutorStream.ts | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts index 8e0816f8ff..2c39e6103b 100644 --- a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts +++ b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts @@ -2,7 +2,7 @@ import type { PostMessageEvent } from '@metamask/post-message-stream'; import { BasePostMessageStream } from '@metamask/post-message-stream'; import { isValidStreamMessage } from '@metamask/post-message-stream/dist/utils'; import { logError } from '@metamask/snaps-utils'; -import { assert } from '@metamask/utils'; +import { assert, stringToBytes } from '@metamask/utils'; export type WebViewInterface = { injectJavaScript(js: string): void; @@ -65,7 +65,9 @@ export class WebViewMessageStream extends BasePostMessageStream { data, }); - this.#webView.injectJavaScript(`window.postMessage('${json}')`); + const bytes = new Uint8Array(stringToBytes(json)); + + this.#webView.injectJavaScript(`window.postMessage([${bytes.toString()}])`); } private _onMessage(event: PostMessageEvent): void { diff --git a/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts b/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts index b762c73fcf..fd156b3f6a 100644 --- a/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts +++ b/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.ts @@ -1,6 +1,7 @@ import type { PostMessageEvent } from '@metamask/post-message-stream'; import { BasePostMessageStream } from '@metamask/post-message-stream'; import { isValidStreamMessage } from '@metamask/post-message-stream/dist/utils'; +import { bytesToString } from '@metamask/utils'; type WebViewExecutorStreamArgs = { name: string; @@ -61,11 +62,12 @@ export class WebViewExecutorStream extends BasePostMessageStream { } private _onMessage(event: PostMessageEvent): void { - if (typeof event.data !== 'string') { + if (!Array.isArray(event.data)) { return; } - const message = JSON.parse(event.data); + const json = bytesToString(new Uint8Array(event.data as number[])); + const message = JSON.parse(json); // Notice that we don't check targetWindow or targetOrigin here. // This doesn't seem possible to do in RN. From b2df1e42a46339f9065b78d2c9c817981a01f601 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 11 Feb 2025 16:12:44 +0100 Subject: [PATCH 3/4] Fix test --- .../src/services/webview/WebViewMessageStream.test.ts | 4 ++-- packages/snaps-controllers/src/test-utils/webview.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.test.ts b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.test.ts index fd271db799..81d2876c19 100644 --- a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.test.ts +++ b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.test.ts @@ -22,12 +22,12 @@ describe('WebViewMessageStream', () => { expect(await responsePromise).toBe(555); expect(mockWebViewA.injectJavaScript).toHaveBeenCalledWith( - `window.postMessage('eyJ0YXJnZXQiOiJiIiwiZGF0YSI6MTExfQ==')`, + `window.postMessage([123,34,116,97,114,103,101,116,34,58,34,98,34,44,34,100,97,116,97,34,58,34,83,89,78,34,125])`, ); // Inject { target: "foo", data: 111 } mockWebViewA.injectJavaScript( - `window.postMessage('eyJ0YXJnZXQiOiJmb28iLCJkYXRhIjoxMTF9')`, + `window.postMessage([123,34,116,97,114,103,101,116,34,58,34,102,111,111,34,44,34,100,97,116,97,34,58,49,49,49,125])`, ); const listener = jest.fn(); diff --git a/packages/snaps-controllers/src/test-utils/webview.ts b/packages/snaps-controllers/src/test-utils/webview.ts index 4838f57c95..c51eff998f 100644 --- a/packages/snaps-controllers/src/test-utils/webview.ts +++ b/packages/snaps-controllers/src/test-utils/webview.ts @@ -1,16 +1,16 @@ -import { base64ToBytes, bytesToString } from '@metamask/utils'; +import { bytesToString } from '@metamask/utils'; import { WebViewMessageStream } from '../services/webview/WebViewMessageStream'; /** - * Parses the injected JS and returns the decoded base64 payload. + * Parses the injected JS and returns the JSON payload. * * @param js - The injected JS. - * @returns The parsed base64 payload as a string. + * @returns The decoded JSON as a string. */ export function parseInjectedJS(js: string) { - const base64 = js.slice(20, -2); - const bytes = base64ToBytes(base64); + const byteString = js.slice(19, -1); + const bytes = new Uint8Array(JSON.parse(byteString)); return bytesToString(bytes); } From 7c7c37b9f5c0c71b872d7dc5756bff22a9dd25bd Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 11 Feb 2025 17:02:34 +0100 Subject: [PATCH 4/4] Fix test --- .../src/services/webview/WebViewMessageStream.ts | 2 ++ .../src/webview/WebViewExecutorStream.test.ts | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts index 2c39e6103b..edd25c45b5 100644 --- a/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts +++ b/packages/snaps-controllers/src/services/webview/WebViewMessageStream.ts @@ -65,6 +65,8 @@ export class WebViewMessageStream extends BasePostMessageStream { data, }); + // To prevent XSS, we encode the message before injecting it. + // This adds significant performance overhead for larger messages. const bytes = new Uint8Array(stringToBytes(json)); this.#webView.injectJavaScript(`window.postMessage([${bytes.toString()}])`); diff --git a/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.test.ts b/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.test.ts index 497e53ed6d..604b409912 100644 --- a/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.test.ts +++ b/packages/snaps-execution-environments/src/webview/WebViewExecutorStream.test.ts @@ -1,5 +1,5 @@ import { sleep } from '@metamask/snaps-utils/test-utils'; -import { bytesToBase64, stringToBytes } from '@metamask/utils'; +import { stringToBytes } from '@metamask/utils'; import { WebViewExecutorStream } from './WebViewExecutorStream'; @@ -8,9 +8,8 @@ describe('WebViewExecutorStream', () => { const addEventListener = jest.fn(); const postMessage = jest.fn().mockImplementation((message) => { const bytes = stringToBytes(message); - const base64 = bytesToBase64(bytes); addEventListener.mock.calls.forEach(([_type, listener]) => { - setTimeout(() => listener({ data: base64 })); + setTimeout(() => listener({ data: Array.from(bytes) })); }); }); const mockWindow = {