Sanitize JSON before checking for blocked methods (#1762)

MetaMask · Sep 20, 2023 · b0bce3f · b0bce3f
1 parent 7e9657d
commit b0bce3f
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 12 deletions.
diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts
@@ -710,6 +710,116 @@ describe('BaseSnapExecutor', () => {
     });
   });
 
+  it('sanitizes JSON before checking for blocked methods using snap global', async () => {
+    const CODE = `
+    const badToJSON = () => {
+      const x = []
+    
+      x.method = 'snap_dialog';
+    
+      x.toJSON = () => {
+        return {
+          method: 'wallet_requestSnaps',
+          params: [],
+        };
+      };
+    
+      return snap.request(x);
+    }
+    
+    module.exports.onRpcRequest = () => badToJSON()
+    `;
+
+    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',
+      error: {
+        code: -32601,
+        message: 'The method does not exist / is not available.',
+        data: {
+          method: 'wallet_requestSnaps',
+        },
+        stack: expect.any(String),
+      },
+      id: 2,
+    });
+  });
+
+  it('sanitizes JSON before checking for blocked methods using ethereum global', async () => {
+    const CODE = `
+    const badToJSON = () => {
+      const x = []
+    
+      x.method = 'eth_requestAccounts';
+    
+      x.toJSON = () => {
+        return {
+          method: 'wallet_requestSnaps',
+          params: [],
+        };
+      };
+    
+      return ethereum.request(x);
+    }
+    
+    module.exports.onRpcRequest = () => badToJSON()
+    `;
+
+    const executor = new TestSnapExecutor();
+    await executor.executeSnap(1, MOCK_SNAP_ID, CODE, ['ethereum']);
+
+    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',
+      error: {
+        code: -32601,
+        message: 'The method does not exist / is not available.',
+        data: {
+          method: 'wallet_requestSnaps',
+        },
+        stack: expect.any(String),
+      },
+      id: 2,
+    });
+  });
+
   it('notifies execution service of out of band errors via unhandledrejection', async () => {
     const CODE = `
       module.exports.onRpcRequest = async () => 'foo';

diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.ts
@@ -409,14 +409,11 @@ export class BaseSnapExecutor {
     const originalRequest = provider.request.bind(provider);
 
     const request = async (args: RequestArguments) => {
-      assertSnapOutboundRequest(args);
-      const sanitizedArgs = getSafeJson(args);
+      const sanitizedArgs = getSafeJson(args) as RequestArguments;
+      assertSnapOutboundRequest(sanitizedArgs);
       this.notify({ method: 'OutboundRequest' });
       try {
-        return await withTeardown(
-          originalRequest(sanitizedArgs as unknown as RequestArguments),
-          this as any,
-        );
+        return await withTeardown(originalRequest(sanitizedArgs), this as any);
       } finally {
         this.notify({ method: 'OutboundResponse' });
       }
@@ -453,14 +450,11 @@ export class BaseSnapExecutor {
     const originalRequest = provider.request.bind(provider);
 
     const request = async (args: RequestArguments) => {
-      assertEthereumOutboundRequest(args);
-      const sanitizedArgs = getSafeJson(args);
+      const sanitizedArgs = getSafeJson(args) as RequestArguments;
+      assertEthereumOutboundRequest(sanitizedArgs);
       this.notify({ method: 'OutboundRequest' });
       try {
-        return await withTeardown(
-          originalRequest(sanitizedArgs as unknown as RequestArguments),
-          this as any,
-        );
+        return await withTeardown(originalRequest(sanitizedArgs), this as any);
       } finally {
         this.notify({ method: 'OutboundResponse' });
       }