add sanitizeTypeData function with tests (#1028)

src/core/keychain/index.ts

@@ -26,6 +26,7 @@ import {
   EthereumWalletSeed,
   identifyWalletType,
   normalizeTransactionResponsePayload,
+  sanitizeTypedData,
 } from '../utils/ethereum';
 import { addHexPrefix } from '../utils/hex';
 
@@ -305,15 +306,18 @@ export const signTypedData = async ({
   // v4 => same as v3 but also supports which supports arrays and recursive structs.
   // Because v4 is backwards compatible with v3, we're supporting only v4
 
+  let sanitizedData = parsedData;
+
   let version = 'v1';
   if (
     typeof parsedData === 'object' &&
     (parsedData.types || parsedData.primaryType || parsedData.domain)
   ) {
     version = 'v4';
+    sanitizedData = sanitizeTypedData(parsedData);
   }
   return signTypedDataSigUtil({
-    data: parsedData as unknown as TypedMessage<TypedDataTypes>,
+    data: sanitizedData as unknown as TypedMessage<TypedDataTypes>,
     privateKey: pkeyBuffer,
     version: version.toUpperCase() as SignTypedDataVersion,
   });

src/core/utils/ethereum.test.ts

@@ -0,0 +1,144 @@
+import { expect, test } from 'vitest';
+
+import { sanitizeTypedData } from './ethereum';
+
+const complexMessage = {
+  domain: {
+    chainId: '1337',
+    name: 'Ether Mail',
+    verifyingContract: '0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC',
+    version: '1',
+  },
+  message: {
+    contents: 'Hello, Bob!',
+    from: {
+      name: 'Cow',
+      wallets: [
+        '0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826',
+        '0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF',
+      ],
+    },
+    to: [
+      {
+        name: 'Bob',
+        wallets: [
+          '0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB',
+          '0xB0BdaBea57B0BDABeA57b0bdABEA57b0BDabEa57',
+          '0xB0B0b0b0b0b0B000000000000000000000000000',
+        ],
+      },
+    ],
+  },
+  primaryType: 'Mail',
+  types: {
+    EIP712Domain: [
+      {
+        name: 'name',
+        type: 'string',
+      },
+      {
+        name: 'version',
+        type: 'string',
+      },
+      {
+        name: 'chainId',
+        type: 'uint256',
+      },
+      {
+        name: 'verifyingContract',
+        type: 'address',
+      },
+    ],
+    Group: [
+      {
+        name: 'name',
+        type: 'string',
+      },
+      {
+        name: 'members',
+        type: 'Person[]',
+      },
+    ],
+    Mail: [
+      {
+        name: 'from',
+        type: 'Person',
+      },
+      {
+        name: 'to',
+        type: 'Person[]',
+      },
+      {
+        name: 'contents',
+        type: 'string',
+      },
+    ],
+    Person: [
+      {
+        name: 'name',
+        type: 'string',
+      },
+      {
+        name: 'wallets',
+        type: 'address[]',
+      },
+    ],
+  },
+};
+
+const complexMessageBad = {
+  ...complexMessage,
+  message: {
+    ...complexMessage.message,
+    thisIsBad: 'yeah',
+  },
+};
+
+const phishingMessage = {
+  types: {
+    EIP712Domain: [
+      { name: 'name', type: 'string' },
+      { name: 'version', type: 'string' },
+      { name: 'chainId', type: 'uint256' },
+      { name: 'verifyingContract', type: 'address' },
+    ],
+    Permit: [
+      { name: 'holder', type: 'address' },
+      { name: 'spender', type: 'address' },
+      { name: 'nonce', type: 'uint256' },
+      { name: 'allowed', type: 'bool' },
+      { name: 'expiry', type: 'uint256' },
+    ],
+  },
+  primaryType: 'Permit',
+  domain: {
+    name: 'Dai Stablecoin',
+    version: '1',
+    chainId: 1, // Ethereum mainnet
+    verifyingContract: '0x6b175474e89094c44da98b954eedeac495271d0f',
+  },
+  message: {
+    'TX simulation':
+      'NO RISK FOUND ✅ ⸻⸻⸻⸻⸻⸻⁢ ⁢ ⁢ ⁢ ⁢ ⁢ ⁢ 100 DAI will be transferred to your account',
+    holder: '0x000000000000000000000000deadbeef',
+    spender: '0x00000000000000000000000000deface',
+    nonce: 1,
+    allowed: true,
+    expiry: 12345678,
+  },
+};
+
+test('[utils/ethereum -> sanitizeTypeData] :: valid message should stay identical', async () => {
+  const sanitizedMessage = sanitizeTypedData(complexMessage);
+  expect(sanitizedMessage).toEqual(complexMessage);
+});
+
+test('[utils/ethereum -> sanitizeTypeData] :: bad message should not contain extra types', async () => {
+  const sanitizedMessage = sanitizeTypedData(complexMessageBad);
+  expect(sanitizedMessage).toEqual(complexMessage);
+});
+
+test('[utils/ethereum -> sanitizeTypeData] :: extra fields for phishing attempt should be ignored', async () => {
+  const sanitizedMessage = sanitizeTypedData(phishingMessage);
+  expect(sanitizedMessage.message['TX simulation']).toBe(undefined);
+});

src/core/utils/ethereum.ts

@@ -100,3 +100,39 @@ export const normalizeTransactionResponsePayload = (
   }
   return payload;
 };
+
+// This function removes all the keys from the message that are not present in the types
+// preventing a know phising attack where the signature process could allow malicious DApps
+// to trick users into signing an EIP-712 object different from the one presented
+// in the signature approval preview. Consequently, users were at risk of unknowingly
+// transferring control of their ERC-20 tokens, NFTs, etc to adversaries by signing
+// hidden Permit messages.
+
+// For more info read https://www.coinspect.com/wallet-EIP-712-injection-vulnerability/
+
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export const sanitizeTypedData = (data: any) => {
+  if (data.types[data.primaryType].length > 0) {
+    // Extract all the valid permit types for the primary type
+    const permitPrimaryTypes: string[] = data.types[data.primaryType].map(
+      (type: { name: string; type: string }) => type.name,
+    );
+
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const sanitizedMessage: any = {};
+    // Extract all the message keys that matches the valid permit types
+    Object.keys(data.message).forEach((key) => {
+      if (permitPrimaryTypes.includes(key)) {
+        sanitizedMessage[key] = data.message[key];
+      }
+    });
+
+    const sanitizedData = {
+      ...data,
+      message: sanitizedMessage,
+    };
+
+    return sanitizedData;
+  }
+  return data;
+};

src/core/utils/signMessages.tsx

@@ -7,6 +7,8 @@ import { RainbowError, logger } from '~/logger';
 import { ProviderRequestPayload } from '../transports/providerRequestTransport';
 import { RPCMethod } from '../types/rpcMethods';
 
+import { sanitizeTypedData } from './ethereum';
+
 export const isSignTypedData = (method: RPCMethod) =>
   method.indexOf('signTypedData') !== -1;
 
@@ -60,9 +62,12 @@ export const getSigningRequestDisplayDetails = (
               msgData = JSON.parse(data) as Bytes;
               // eslint-disable-next-line no-empty
             } catch (e) {}
+
+            const sanitizedMessageData = sanitizeTypedData(msgData);
+
             return {
-              message: JSON.stringify(msgData, null, 2),
-              msgData,
+              message: JSON.stringify(sanitizedMessageData, null, 2),
+              msgData: sanitizedMessageData,
               address: getAddress(address),
               typedData: true,
             };