Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions app/core/RPCMethods/RPCMethodMiddleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1704,6 +1704,86 @@ describe('getRpcMethodMiddleware', () => {
expect(spy).toBeCalledTimes(1);
});
});

describe.each([
['eth_signTypedData_v3', 'V3'],
['eth_signTypedData_v4', 'V4'],
])(
'%s canonicalizes duplicate JSON keys to prevent display/signing divergence',
(methodName, version) => {
// Raw JSON with duplicate "value" keys — cannot be built with JSON.stringify.
// A malicious dapp sends a small value first, a nested object to break the
// regex boundary, then a large duplicate value that JSON.parse keeps.
const maliciousData = [
'{',
'"types":{"EIP712Domain":[],"Permit":[{"name":"value","type":"uint256"}]},',
'"primaryType":"Permit",',
'"domain":{"chainId":"0x1"},',
'"message":{',
'"value":"1000000000000000",',
'"details":{"token":"0x0000000000000000000000000000000000000001"},',
'"value":"999999999999999999999999"',
'}',
'}',
].join('');

const canonicalData = JSON.stringify(JSON.parse(maliciousData));

async function sendMaliciousRequest() {
MockEngine.context.SignatureController.newUnsignedTypedMessage.mockReset();
MockEngine.context.SignatureController.newUnsignedTypedMessage.mockResolvedValue(
signatureMock,
);

const { middleware } = setupSignature();

const request = {
jsonrpc,
id: 1,
method: methodName,
params: [addressMock, maliciousData],
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (await callMiddleware({ middleware, request })) as any;
}

it('strips duplicate keys so SignatureController receives canonical data', async () => {
await sendMaliciousRequest();

const receivedData = (
Engine.context.SignatureController
.newUnsignedTypedMessage as jest.Mock
).mock.calls[0][0].data;

expect(receivedData).toBe(canonicalData);
expect(receivedData).not.toBe(maliciousData);
});

it('resolves duplicate message.value to the last occurrence', async () => {
await sendMaliciousRequest();

const receivedData = (
Engine.context.SignatureController
.newUnsignedTypedMessage as jest.Mock
).mock.calls[0][0].data;

const parsed = JSON.parse(receivedData);
expect(parsed.message.value).toBe('999999999999999999999999');
});

it('produces data that re-parses identically (no hidden duplicates)', async () => {
await sendMaliciousRequest();

const receivedData = (
Engine.context.SignatureController
.newUnsignedTypedMessage as jest.Mock
).mock.calls[0][0].data;

expect(JSON.stringify(JSON.parse(receivedData))).toBe(receivedData);
});
},
);
});

describe('checkActiveAccountAndChainId', () => {
Expand Down
23 changes: 22 additions & 1 deletion app/core/RPCMethods/RPCMethodMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,27 @@ const withRemoteSessionId = (
? { remote_session_id: analytics.remote_session_id }
: {};

/**
* Round-trips a typed-data JSON string through JSON.parse / JSON.stringify to
* produce a canonical representation. JSON.parse silently drops duplicate keys
* (keeping the last value per key), so the canonical string can never contain
* duplicates that let a regex-based extraction diverge from the parsed object
* used at signing time.
*
* This mirrors the normalizeTypedMessage step that the extension performs via
* @metamask/eth-json-rpc-middleware before data reaches SignatureController.
*/
function canonicalizeTypedMessageData(data: string): string {
if (typeof data !== 'string') {
return data;
}
try {
return JSON.stringify(JSON.parse(data));
} catch {
return data;
}
}

const generateRawSignature = async ({
version,
req,
Expand Down Expand Up @@ -275,7 +296,7 @@ const generateRawSignature = async ({

const rawSig = await signatureController.newUnsignedTypedMessage(
{
data: req.params[1],
data: canonicalizeTypedMessageData(req.params[1]),
from: req.params[0],
requestId: req.id,
...pageMeta,
Expand Down
Loading