Skip to content

Commit 6c5256c

Browse files
fix: Inspect wallet_invokeMethod requests before sending (#3819)
Inspect payloads for `wallet_invokeMethod` to ensure that blocked RPC methods remain blocked. We block a subset of the RPC methods blocked in the EIP-1193 provider for the multichain API, but notably unblock `eth_signTransaction`. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Strengthens multichain request validation and tests. > > - Adds `assertMultichainOutboundRequest` and invokes it for `snap.request` multichain calls; inspects `wallet_invokeMethod` payloads and blocks `snap_*` and a defined subset of sensitive RPC methods > - Updates `BaseSnapExecutor` to route multichain requests through the new validator > - Expands tests: establish session before multichain calls, verify allowed `eth_chainId`, and assert blocking of `metamask_sendDomainMetadata`; preserves JSON sanitization checks > - Slight coverage increase in `coverage.json` > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6ff084b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent f1fe96d commit 6c5256c

File tree

4 files changed

+118
-4
lines changed

4 files changed

+118
-4
lines changed
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"branches": 90.19,
3-
"functions": 94.02,
4-
"lines": 90.26,
5-
"statements": 89.32
3+
"functions": 95.45,
4+
"lines": 91.16,
5+
"statements": 90.33
66
}

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

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,7 +614,10 @@ describe('BaseSnapExecutor', () => {
614614

615615
it('supports the multichain API using the snap global', async () => {
616616
const CODE = `
617-
module.exports.onRpcRequest = () => snap.request({ method: 'wallet_invokeMethod', params: { scope: 'eip155:1', request: { method: 'eth_chainId' } } });
617+
module.exports.onRpcRequest = async () => {
618+
await snap.request({ method: 'wallet_createSession', params: {} });
619+
return snap.request({ method: 'wallet_invokeMethod', params: { scope: 'eip155:1', request: { method: 'eth_chainId' } } });
620+
};
618621
`;
619622

620623
const executor = new TestSnapExecutor();
@@ -638,6 +641,17 @@ describe('BaseSnapExecutor', () => {
638641
],
639642
});
640643

644+
const sessionRequest = await executor.readRpc();
645+
await executor.writeRpc({
646+
name: 'metamask-multichain-provider',
647+
data: {
648+
jsonrpc: '2.0',
649+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
650+
id: sessionRequest.data.id!,
651+
result: { sessionScopes: {} },
652+
},
653+
});
654+
641655
const multichainRequest = await executor.readRpc();
642656
expect(multichainRequest).toStrictEqual({
643657
name: 'metamask-multichain-provider',
@@ -671,6 +685,52 @@ describe('BaseSnapExecutor', () => {
671685
});
672686
});
673687

688+
it('blocks certain RPC methods using the multichain API', async () => {
689+
const CODE = `
690+
module.exports.onRpcRequest = () => snap.request({ method: 'wallet_invokeMethod', params: { scope: 'eip155:1', request: { method: 'metamask_sendDomainMetadata' } } });
691+
`;
692+
693+
const executor = new TestSnapExecutor();
694+
await executor.executeSnap(1, MOCK_SNAP_ID, CODE, []);
695+
696+
expect(await executor.readCommand()).toStrictEqual({
697+
jsonrpc: '2.0',
698+
id: 1,
699+
result: 'OK',
700+
});
701+
702+
await executor.writeCommand({
703+
jsonrpc: '2.0',
704+
id: 2,
705+
method: 'snapRpc',
706+
params: [
707+
MOCK_SNAP_ID,
708+
HandlerType.OnRpcRequest,
709+
MOCK_ORIGIN,
710+
{ jsonrpc: '2.0', method: '', params: [] },
711+
],
712+
});
713+
714+
expect(await executor.readCommand()).toStrictEqual({
715+
jsonrpc: '2.0',
716+
id: 2,
717+
error: {
718+
code: -31001,
719+
message: 'Wrapped Snap Error',
720+
data: {
721+
cause: expect.objectContaining({
722+
code: -32601,
723+
message: 'The method does not exist / is not available.',
724+
data: {
725+
cause: null,
726+
method: 'metamask_sendDomainMetadata',
727+
},
728+
}),
729+
},
730+
},
731+
});
732+
});
733+
674734
it('sanitizes JSON before checking for blocked methods using snap global', async () => {
675735
const CODE = `
676736
const badToJSON = () => {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
withTeardown,
5050
isValidResponse,
5151
isMultichainRequest,
52+
assertMultichainOutboundRequest,
5253
} from './utils';
5354
import {
5455
ExecuteSnapRequestArgumentsStruct,
@@ -547,6 +548,7 @@ export class BaseSnapExecutor {
547548
assertSnapOutboundRequest(sanitizedArgs);
548549

549550
if (isMultichainRequest(sanitizedArgs)) {
551+
assertMultichainOutboundRequest(sanitizedArgs);
550552
return await withTeardown(
551553
originalMultichainRequest(sanitizedArgs),
552554
this as any,

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,19 @@ export const MULTICHAIN_API_METHODS = Object.freeze([
7070
'wallet_revokeSession',
7171
]);
7272

73+
// Subset of BLOCKED_RPC_METHODS
74+
export const BLOCKED_MULTICHAIN_RPC_METHODS = Object.freeze([
75+
'wallet_requestPermissions',
76+
'wallet_revokePermissions',
77+
'eth_decrypt',
78+
'eth_getEncryptionPublicKey',
79+
'metamask_sendDomainMetadata',
80+
'wallet_addEthereumChain',
81+
'wallet_watchAsset',
82+
'wallet_registerOnboarding',
83+
'wallet_scanQRCode',
84+
]);
85+
7386
/**
7487
* Check whether a validated request should be routed to the multichain API.
7588
*
@@ -80,6 +93,45 @@ export function isMultichainRequest(args: RequestArguments) {
8093
return MULTICHAIN_API_METHODS.includes(args.method);
8194
}
8295

96+
/**
97+
* Asserts the validity of request arguments for a multichain outbound request using the `snap.request` API.
98+
*
99+
* @param args - The arguments to validate.
100+
*/
101+
export function assertMultichainOutboundRequest(args: RequestArguments) {
102+
if (args.method !== 'wallet_invokeMethod') {
103+
return;
104+
}
105+
106+
// For `wallet_invokeMethod`, we need to inspect the parameters to determine
107+
// if the Snap is requesting a blocked RPC method.
108+
assert(
109+
isObject(args.params) &&
110+
isObject(args.params.request) &&
111+
typeof args.params.request.method === 'string',
112+
rpcErrors.invalidParams(),
113+
);
114+
115+
const innerMethod = args.params.request.method;
116+
117+
assert(
118+
!String.prototype.startsWith.call(innerMethod, 'snap_'),
119+
rpcErrors.methodNotFound({
120+
data: {
121+
method: innerMethod,
122+
},
123+
}),
124+
);
125+
assert(
126+
!BLOCKED_MULTICHAIN_RPC_METHODS.includes(innerMethod),
127+
rpcErrors.methodNotFound({
128+
data: {
129+
method: innerMethod,
130+
},
131+
}),
132+
);
133+
}
134+
83135
/**
84136
* Asserts the validity of request arguments for a snap outbound request using the `snap.request` API.
85137
*

0 commit comments

Comments
 (0)