Add contract address validation for token swaps (#10912)

shared/constants/network.js

@@ -33,6 +33,7 @@ export const MAINNET_DISPLAY_NAME = 'Ethereum Mainnet';
 export const GOERLI_DISPLAY_NAME = 'Goerli';
 
 export const ETH_SYMBOL = 'ETH';
+export const WETH_SYMBOL = 'WETH';
 export const TEST_ETH_SYMBOL = 'TESTETH';
 export const BNB_SYMBOL = 'BNB';
 

shared/constants/swaps.js

@@ -51,6 +51,9 @@ const TESTNET_CONTRACT_ADDRESS = '0x881d40237659c251811cec9c364ef91dc08d300c';
 
 const BSC_CONTRACT_ADDRESS = '0x1a1ec25dc08e98e5e93f1104b5e5cdd298707d31';
 
+export const ETH_WETH_CONTRACT_ADDRESS =
+  '0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2';
+
 const METASWAP_ETH_API_HOST = 'https://api.metaswap.codefi.network';
 
 const METASWAP_BSC_API_HOST = 'https://bsc-api.metaswap.codefi.network';

test/jest/setup.js

@@ -1,2 +1,2 @@
-// jest-setup.js is for Jest-specific setup only and runs before our Jest tests.
+// This file is for Jest-specific setup only and runs before our Jest tests.
 import '@testing-library/jest-dom';

ui/app/ducks/swaps/swaps.js

@@ -2,6 +2,8 @@ import { createSlice } from '@reduxjs/toolkit';
 import BigNumber from 'bignumber.js';
 import log from 'loglevel';
 
+import { captureMessage } from '@sentry/browser';
+
 import {
   addToken,
   addUnapprovedTransaction,
@@ -33,6 +35,7 @@ import {
 import {
   fetchSwapsFeatureLiveness,
   fetchSwapsGasPrices,
+  isContractAddressValid,
 } from '../../pages/swaps/swaps.util';
 import { calcGasTotal } from '../../pages/send/send.utils';
 import {
@@ -676,6 +679,19 @@ export const signAndSendTransactions = (history, metaMetricsEvent) => {
       sensitiveProperties: swapMetaData,
     });
 
+    if (!isContractAddressValid(usedTradeTxParams.to, swapMetaData, chainId)) {
+      captureMessage('Invalid contract address', {
+        extra: {
+          token_from: swapMetaData.token_from,
+          token_to: swapMetaData.token_to,
+          contract_address: usedTradeTxParams.to,
+        },
+      });
+      await dispatch(setSwapsErrorKey(SWAP_FAILED_ERROR));
+      history.push(SWAPS_ERROR_ROUTE);
+      return;
+    }
+
     let finalApproveTxMeta;
     const approveTxParams = getApproveTxParams(state);
     if (approveTxParams) {

ui/app/pages/swaps/swaps.util.js

@@ -5,13 +5,18 @@ import { isValidAddress } from 'ethereumjs-util';
 import {
   SWAPS_CHAINID_DEFAULT_TOKEN_MAP,
   METASWAP_CHAINID_API_HOST_MAP,
+  SWAPS_CHAINID_CONTRACT_ADDRESS_MAP,
+  ETH_WETH_CONTRACT_ADDRESS,
 } from '../../../../shared/constants/swaps';
 import {
   isSwapsDefaultTokenAddress,
   isSwapsDefaultTokenSymbol,
 } from '../../../../shared/modules/swaps.utils';
-
+import {
-import { MAINNET_CHAIN_ID } from '../../../../shared/constants/network';
+  ETH_SYMBOL,
+  WETH_SYMBOL,
+  MAINNET_CHAIN_ID,
+} from '../../../../shared/constants/network';
 import {
   calcTokenValue,
   calcTokenAmount,
@@ -676,3 +681,38 @@ export function formatSwapsValueForDisplay(destinationAmount) {
   }
   return amountToDisplay;
 }
+
+/**
+ * Checks whether a contract address is valid before swapping tokens.
+ *
+ * @param {string} contractAddress - E.g. "0x881d40237659c251811cec9c364ef91dc08d300c" for mainnet
+ * @param {object} swapMetaData - We check the following 2 fields, e.g. { token_from: "ETH", token_to: "WETH" }
+ * @param {string} chainId - The hex encoded chain ID to check
+ * @returns {boolean} Whether a contract address is valid or not
+ */
+export const isContractAddressValid = (
+  contractAddress,
+  swapMetaData,
+  chainId = MAINNET_CHAIN_ID,
+) => {
+  if (!contractAddress) {
+    return false;
+  }
+  if (
+    (swapMetaData.token_from === ETH_SYMBOL &&
+      swapMetaData.token_to === WETH_SYMBOL) ||
+    (swapMetaData.token_from === WETH_SYMBOL &&
+      swapMetaData.token_to === ETH_SYMBOL)
+  ) {
+    // Sometimes we get a contract address with a few upper-case chars and since addresses are
+    // case-insensitive, we compare uppercase versions for validity.
+    return (
+      contractAddress.toUpperCase() === ETH_WETH_CONTRACT_ADDRESS.toUpperCase()
+    );
+  }
+  const contractAddressForChainId = SWAPS_CHAINID_CONTRACT_ADDRESS_MAP[chainId];
+  return (
+    contractAddressForChainId &&
+    contractAddressForChainId.toUpperCase() === contractAddress.toUpperCase()
+  );
+};

ui/app/pages/swaps/swaps.util.test.js

@@ -1,5 +1,15 @@
 import nock from 'nock';
-import { MAINNET_CHAIN_ID } from '../../../../shared/constants/network';
+import {
+  ETH_SYMBOL,
+  WETH_SYMBOL,
+  MAINNET_CHAIN_ID,
+  BSC_CHAIN_ID,
+  LOCALHOST_CHAIN_ID,
+} from '../../../../shared/constants/network';
+import {
+  SWAPS_CHAINID_CONTRACT_ADDRESS_MAP,
+  ETH_WETH_CONTRACT_ADDRESS,
+} from '../../../../shared/constants/swaps';
 import {
   TOKENS,
   EXPECTED_TOKENS_RESULT,
@@ -13,6 +23,7 @@ import {
   fetchTokens,
   fetchAggregatorMetadata,
   fetchTopAssets,
+  isContractAddressValid,
 } from './swaps.util';
 
 jest.mock('../../../lib/storage-helpers.js', () => ({
@@ -165,4 +176,166 @@ describe('Swaps Util', () => {
       expect(result).toStrictEqual(expectedResult);
     });
   });
+
+  describe('isContractAddressValid', () => {
+    let swapMetaData;
+    let usedTradeTxParams;
+
+    beforeEach(() => {
+      swapMetaData = {
+        available_quotes: undefined,
+        average_savings: undefined,
+        best_quote_source: 'paraswap',
+        custom_slippage: true,
+        estimated_gas: '134629',
+        fee_savings: undefined,
+        gas_fees: '47.411896',
+        median_metamask_fee: undefined,
+        other_quote_selected: false,
+        other_quote_selected_source: '',
+        performance_savings: undefined,
+        slippage: 5,
+        suggested_gas_price: '164',
+        token_from: ETH_SYMBOL,
+        token_from_amount: '1',
+        token_to: WETH_SYMBOL,
+        token_to_amount: '1.0000000',
+        used_gas_price: '164',
+      };
+      usedTradeTxParams = {
+        data: 'testData',
+        from: '0xe53a5bc256898bfa5673b20aceeb2b2152075d17',
+        gas: '2427c',
+        gasPrice: '27592f5a00',
+        to: ETH_WETH_CONTRACT_ADDRESS,
+        value: '0xde0b6b3a7640000',
+      };
+    });
+
+    it('returns true if "token_from" is ETH, "token_to" is WETH and "to" is ETH_WETH contract address', () => {
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          MAINNET_CHAIN_ID,
+        ),
+      ).toBe(true);
+    });
+
+    it('returns true if "token_from" is WETH, "token_to" is ETH and "to" is ETH_WETH contract address', () => {
+      swapMetaData.token_from = WETH_SYMBOL;
+      swapMetaData.token_to = ETH_SYMBOL;
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          MAINNET_CHAIN_ID,
+        ),
+      ).toBe(true);
+    });
+
+    it('returns true if "token_from" is ETH, "token_to" is WETH and "to" is ETH_WETH contract address with some uppercase chars', () => {
+      usedTradeTxParams.to = '0xc02AAA39B223fe8d0a0e5c4f27ead9083c756cc2';
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          MAINNET_CHAIN_ID,
+        ),
+      ).toBe(true);
+    });
+
+    it('returns false if "token_from" is ETH, "token_to" is WETH and "to" is mainnet contract address', () => {
+      usedTradeTxParams.to =
+        SWAPS_CHAINID_CONTRACT_ADDRESS_MAP[MAINNET_CHAIN_ID];
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          MAINNET_CHAIN_ID,
+        ),
+      ).toBe(false);
+    });
+
+    it('returns false if "token_from" is WETH, "token_to" is ETH and "to" is mainnet contract address', () => {
+      swapMetaData.token_from = WETH_SYMBOL;
+      swapMetaData.token_to = ETH_SYMBOL;
+      usedTradeTxParams.to =
+        SWAPS_CHAINID_CONTRACT_ADDRESS_MAP[MAINNET_CHAIN_ID];
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          MAINNET_CHAIN_ID,
+        ),
+      ).toBe(false);
+    });
+
+    it('returns false if contractAddress is null', () => {
+      expect(
+        isContractAddressValid(null, swapMetaData, LOCALHOST_CHAIN_ID),
+      ).toBe(false);
+    });
+
+    it('returns true if "token_from" is BAT and "to" is mainnet contract address', () => {
+      swapMetaData.token_from = 'BAT';
+      usedTradeTxParams.to =
+        SWAPS_CHAINID_CONTRACT_ADDRESS_MAP[MAINNET_CHAIN_ID];
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          MAINNET_CHAIN_ID,
+        ),
+      ).toBe(true);
+    });
+
+    it('returns true if "token_to" is BAT and "to" is BSC contract address', () => {
+      swapMetaData.token_to = 'BAT';
+      usedTradeTxParams.to = SWAPS_CHAINID_CONTRACT_ADDRESS_MAP[BSC_CHAIN_ID];
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          BSC_CHAIN_ID,
+        ),
+      ).toBe(true);
+    });
+
+    it('returns true if "token_to" is BAT and "to" is testnet contract address', () => {
+      swapMetaData.token_to = 'BAT';
+      usedTradeTxParams.to =
+        SWAPS_CHAINID_CONTRACT_ADDRESS_MAP[LOCALHOST_CHAIN_ID];
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          LOCALHOST_CHAIN_ID,
+        ),
+      ).toBe(true);
+    });
+
+    it('returns true if "token_to" is BAT and "to" is testnet contract address with some uppercase chars', () => {
+      swapMetaData.token_to = 'BAT';
+      usedTradeTxParams.to = '0x881D40237659C251811CEC9c364ef91dC08D300C';
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          LOCALHOST_CHAIN_ID,
+        ),
+      ).toBe(true);
+    });
+
+    it('returns false if "token_to" is BAT and "to" has mismatch with current chainId', () => {
+      swapMetaData.token_to = 'BAT';
+      expect(
+        isContractAddressValid(
+          usedTradeTxParams.to,
+          swapMetaData,
+          LOCALHOST_CHAIN_ID,
+        ),
+      ).toBe(false);
+    });
+  });
 });