Enforce JSON-RPC response size limits (#2201)

Enforces a size limit on JSON-RPC responses from Snaps, throwing an
error if the object is larger than 64 MB.

64 MB was chosen because it seems to be the limit set by Chrome for
postMessage between the extension and dapps:
https://source.chromium.org/chromium/chromium/src/+/main:extensions/renderer/api/messaging/messaging_util.cc;l=65?q=%22Message%20length%20exceeded%20maximum%20allowed%20length%22&ss=chromium

---------

Co-authored-by: Maarten Zuidhoorn <maarten@zuidhoorn.com>

packages/snaps-execution-environments/coverage.json

@@ -1,6 +1,6 @@
 {
-  "branches": 81.04,
-  "functions": 90.13,
-  "lines": 90.73,
-  "statements": 90.11
+  "branches": 80.66,
+  "functions": 90.19,
+  "lines": 90.83,
+  "statements": 90.2
 }

packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts

@@ -2007,6 +2007,44 @@ describe('BaseSnapExecutor', () => {
     });
   });
 
+  it('throws when trying to respond with value that is too large', async () => {
+    const CODE = `
+      module.exports.onRpcRequest = () => '1'.repeat(100_000_000);
+    `;
+
+    const executor = new TestSnapExecutor();
+    await executor.executeSnap(1, MOCK_SNAP_ID, CODE, []);
+
+    expect(await executor.readCommand()).toStrictEqual({
+      jsonrpc: '2.0',
+      id: 1,
+      result: 'OK',
+    });
+
+    await executor.writeCommand({
+      jsonrpc: '2.0',
+      id: 2,
+      method: 'snapRpc',
+      params: [
+        MOCK_SNAP_ID,
+        HandlerType.OnRpcRequest,
+        MOCK_ORIGIN,
+        { jsonrpc: '2.0', method: '', params: [] },
+      ],
+    });
+
+    expect(await executor.readCommand()).toStrictEqual({
+      jsonrpc: '2.0',
+      id: 2,
+      error: {
+        code: -32603,
+        message:
+          'JSON-RPC responses must be JSON serializable objects smaller than 64 MB.',
+        stack: expect.any(String),
+      },
+    });
+  });
+
   it('throws when receiving an invalid RPC request', async () => {
     const executor = new TestSnapExecutor();
 

packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts

@@ -26,8 +26,6 @@ import type {
   Json,
 } from '@metamask/utils';
 import {
-  isObject,
-  isValidJson,
   assert,
   isJsonRpcRequest,
   hasProperty,
@@ -49,6 +47,7 @@ import {
   sanitizeRequestArguments,
   proxyStreamProvider,
   withTeardown,
+  isValidResponse,
 } from './utils';
 import {
   ExecuteSnapRequestArgumentsStruct,
@@ -301,27 +300,27 @@ export class BaseSnapExecutor {
     });
   }
 
-  async #notify(requestObject: Omit<JsonRpcNotification, 'jsonrpc'>) {
-    if (!isValidJson(requestObject) || !isObject(requestObject)) {
+  async #notify(notification: Omit<JsonRpcNotification, 'jsonrpc'>) {
+    if (!isValidResponse(notification)) {
       throw rpcErrors.internal(
-        'JSON-RPC notifications must be JSON serializable objects',
+        'JSON-RPC notifications must be JSON serializable objects smaller than 64 MB.',
       );
     }
 
     await this.#write({
-      ...requestObject,
+      ...notification,
       jsonrpc: '2.0',
     });
   }
 
-  async #respond(id: JsonRpcId, requestObject: Record<string, unknown>) {
-    if (!isValidJson(requestObject) || !isObject(requestObject)) {
+  async #respond(id: JsonRpcId, response: Record<string, unknown>) {
+    if (!isValidResponse(response)) {
       // Instead of throwing, we directly respond with an error.
       // This prevents an issue where we wouldn't respond when errors were non-serializable
       await this.#write({
         error: serializeError(
           rpcErrors.internal(
-            'JSON-RPC responses must be JSON serializable objects.',
+            'JSON-RPC responses must be JSON serializable objects smaller than 64 MB.',
           ),
         ),
         id,
@@ -331,7 +330,7 @@ export class BaseSnapExecutor {
     }
 
     await this.#write({
-      ...requestObject,
+      ...response,
       id,
       jsonrpc: '2.0',
     });

packages/snaps-execution-environments/src/common/utils.test.ts

@@ -2,6 +2,7 @@ import {
   BLOCKED_RPC_METHODS,
   assertEthereumOutboundRequest,
   assertSnapOutboundRequest,
+  isValidResponse,
 } from './utils';
 
 describe('assertSnapOutboundRequest', () => {
@@ -77,3 +78,22 @@ describe('assertEthereumOutboundRequest', () => {
     );
   });
 });
+
+describe('isValidResponse', () => {
+  it('returns false if the value is not an object', () => {
+    // @ts-expect-error Intentionally using bad params
+    expect(isValidResponse('foo')).toBe(false);
+  });
+
+  it('returns false if the value is not JSON serializable', () => {
+    expect(isValidResponse({ foo: BigInt(0) })).toBe(false);
+  });
+
+  it('returns false if the value is too large', () => {
+    expect(isValidResponse({ foo: '1'.repeat(100_000_000) })).toBe(false);
+  });
+
+  it('returns true if the value is a valid JSON object', () => {
+    expect(isValidResponse({ foo: 'bar' })).toBe(true);
+  });
+});

packages/snaps-execution-environments/src/common/utils.ts

@@ -1,9 +1,21 @@
 import type { StreamProvider } from '@metamask/providers';
 import type { RequestArguments } from '@metamask/providers/dist/BaseProvider';
 import { rpcErrors } from '@metamask/rpc-errors';
-import { assert, assertStruct, getSafeJson, JsonStruct } from '@metamask/utils';
+import {
+  assert,
+  assertStruct,
+  getJsonSize,
+  getSafeJson,
+  isObject,
+  JsonStruct,
+} from '@metamask/utils';
 
 import { log } from '../logging';
+
+// 64 MB - we chose this number because it is the size limit for postMessage
+// between the extension and the dapp enforced by Chrome.
+const MAX_RESPONSE_JSON_SIZE = 64_000_000;
+
 /**
  * Make proxy for Promise and handle the teardown process properly.
  * If the teardown is called in the meanwhile, Promise result will not be
@@ -174,3 +186,23 @@ export function sanitizeRequestArguments(value: unknown): RequestArguments {
   const json = JSON.parse(JSON.stringify(value));
   return getSafeJson(json) as RequestArguments;
 }
+
+/**
+ * Check if the input is a valid response.
+ *
+ * @param response - The response.
+ * @returns True if the response is valid, otherwise false.
+ */
+export function isValidResponse(response: Record<string, unknown>) {
+  if (!isObject(response)) {
+    return false;
+  }
+
+  try {
+    // If the JSON is invalid this will throw and we should return false.
+    const size = getJsonSize(response);
+    return size < MAX_RESPONSE_JSON_SIZE;
+  } catch {
+    return false;
+  }
+}