Check specified chain ID against endpoint return value (#9491)

Adds additional validation to chainId values in the network form, by comparing the specified value against the value returned by the endpoint.

app/_locales/en/messages.json

@@ -574,6 +574,10 @@
   "endOfFlowMessage9": {
     "message": "Learn more."
   },
+  "endpointReturnedDifferentChainId": {
+    "message": "The endpoint returned a different chain ID: $1",
+    "description": "$1 is the return value of eth_chainId from an RPC endpoint"
+  },
   "ensNotFoundOnCurrentNetwork": {
     "message": "ENS name not found on the current network. Try switching to Ethereum Mainnet."
   },
@@ -653,6 +657,9 @@
   "failed": {
     "message": "Failed"
   },
+  "failedToFetchChainId": {
+    "message": "Could not fetch chain ID. Is your RPC URL correct?"
+  },
   "failureMessage": {
     "message": "Something went wrong, and we were unable to complete the action"
   },
@@ -989,7 +996,7 @@
     "message": "Network Name"
   },
   "networkSettingsChainIdDescription": {
-    "message": "The chain ID is used for signing transactions. Enter a decimal or hexadecimal number starting with '0x'."
+    "message": "The chain ID is used for signing transactions. It must match the chain ID returned by the network. Enter a decimal or hexadecimal number starting with '0x'."
   },
   "networkSettingsDescription": {
     "message": "Add and edit custom RPC networks"

test/e2e/metamask-ui.spec.js

@@ -1258,36 +1258,60 @@ describe('MetaMask', function () {
   })
 
   describe('Stores custom RPC history', function () {
-    const customRpcInfo = [
+    it(`creates first custom RPC entry`, async function () {
-      { rpcUrl: 'http://127.0.0.1:8545/1', chainId: '0x1' },
+      const rpcUrl = 'http://127.0.0.1:8545/1'
-      { rpcUrl: 'http://127.0.0.1:8545/2', chainId: '0x2' },
+      const chainId = '0x539' // Ganache default, decimal 1337
-      { rpcUrl: 'http://127.0.0.1:8545/3', chainId: '0x3' },
-      { rpcUrl: 'http://127.0.0.1:8545/4', chainId: '0x4' },
-    ]
 
-    customRpcInfo.forEach(({ rpcUrl, chainId }) => {
+      await driver.clickElement(By.css('.network-name'))
-      it(`creates custom RPC: '${rpcUrl}' with chainId '${chainId}'`, async function () {
+      await driver.delay(regularDelayMs)
-        await driver.clickElement(By.css('.network-name'))
-        await driver.delay(regularDelayMs)
 
-        await driver.clickElement(By.xpath(`//span[contains(text(), 'Custom RPC')]`))
+      await driver.clickElement(By.xpath(`//span[contains(text(), 'Custom RPC')]`))
-        await driver.delay(regularDelayMs)
+      await driver.delay(regularDelayMs)
 
-        await driver.findElement(By.css('.settings-page__sub-header-text'))
+      await driver.findElement(By.css('.settings-page__sub-header-text'))
 
-        const customRpcInputs = await driver.findElements(By.css('input[type="text"]'))
+      const customRpcInputs = await driver.findElements(By.css('input[type="text"]'))
-        const rpcUrlInput = customRpcInputs[1]
+      const rpcUrlInput = customRpcInputs[1]
-        const chainIdInput = customRpcInputs[2]
+      const chainIdInput = customRpcInputs[2]
 
-        await rpcUrlInput.clear()
+      await rpcUrlInput.clear()
-        await rpcUrlInput.sendKeys(rpcUrl)
+      await rpcUrlInput.sendKeys(rpcUrl)
 
-        await chainIdInput.clear()
+      await chainIdInput.clear()
-        await chainIdInput.sendKeys(chainId)
+      await chainIdInput.sendKeys(chainId)
 
-        await driver.clickElement(By.css('.network-form__footer .btn-secondary'))
+      await driver.clickElement(By.css('.network-form__footer .btn-secondary'))
-        await driver.delay(largeDelayMs * 2)
+      await driver.findElement(
-      })
+        By.xpath(`//div[contains(text(), '${rpcUrl}')]`),
+      )
+    })
+
+    it(`creates second custom RPC entry`, async function () {
+      const rpcUrl = 'http://127.0.0.1:8545/2'
+      const chainId = '0x539' // Ganache default, decimal 1337
+
+      await driver.clickElement(By.css('.network-name'))
+      await driver.delay(regularDelayMs)
+
+      await driver.clickElement(By.xpath(`//span[contains(text(), 'Custom RPC')]`))
+      await driver.delay(regularDelayMs)
+
+      await driver.findElement(By.css('.settings-page__sub-header-text'))
+
+      const customRpcInputs = await driver.findElements(By.css('input[type="text"]'))
+      const rpcUrlInput = customRpcInputs[1]
+      const chainIdInput = customRpcInputs[2]
+
+      await rpcUrlInput.clear()
+      await rpcUrlInput.sendKeys(rpcUrl)
+
+      await chainIdInput.clear()
+      await chainIdInput.sendKeys(chainId)
+
+      await driver.clickElement(By.css('.network-form__footer .btn-secondary'))
+      await driver.findElement(
+        By.xpath(`//div[contains(text(), '${rpcUrl}')]`),
+      )
     })
 
     it('selects another provider', async function () {
@@ -1305,7 +1329,7 @@ describe('MetaMask', function () {
       // only recent 3 are found and in correct order (most recent at the top)
       const customRpcs = await driver.findElements(By.xpath(`//span[contains(text(), 'http://127.0.0.1:8545/')]`))
 
-      assert.equal(customRpcs.length, customRpcInfo.length)
+      assert.equal(customRpcs.length, 2)
     })
 
     it('deletes a custom RPC', async function () {

ui/app/helpers/utils/util.js

@@ -407,3 +407,43 @@ export function constructTxParams ({ sendToken, data, to, amount, from, gas, gas
   }
   return addHexPrefixToObjectValues(txParams)
 }
+
+/**
+ * Makes a JSON RPC request to the given URL, with the given RPC method and params.
+ *
+ * @param {string} rpcUrl - The RPC endpoint URL to target.
+ * @param {string} rpcMethod - The RPC method to request.
+ * @param {Array<unknown>} [rpcParams] - The RPC method params.
+ * @returns {Promise<unknown|undefined>} Returns the result of the RPC method call,
+ * or throws an error in case of failure.
+ */
+export async function jsonRpcRequest (rpcUrl, rpcMethod, rpcParams = []) {
+  const jsonRpcResponse = await window.fetch(rpcUrl, {
+    method: 'POST',
+    body: JSON.stringify({
+      id: Date.now().toString(),
+      jsonrpc: '2.0',
+      method: rpcMethod,
+      params: rpcParams,
+    }),
+    headers: {
+      'Content-Type': 'application/json',
+    },
+    cache: 'default',
+  })
+    .then((httpResponse) => httpResponse.json())
+
+  if (
+    !jsonRpcResponse ||
+    Array.isArray(jsonRpcResponse) ||
+    typeof jsonRpcResponse !== 'object'
+  ) {
+    throw new Error(`RPC endpoint ${rpcUrl} returned non-object response.`)
+  }
+  const { error, result } = jsonRpcResponse
+
+  if (error) {
+    throw new Error(error?.message || error)
+  }
+  return result
+}

ui/app/pages/settings/networks-tab/network-form/network-form.component.js

@@ -2,10 +2,12 @@ import React, { PureComponent } from 'react'
 import PropTypes from 'prop-types'
 import validUrl from 'valid-url'
 import BigNumber from 'bignumber.js'
+import log from 'loglevel'
 import TextField from '../../../../components/ui/text-field'
 import Button from '../../../../components/ui/button'
 import Tooltip from '../../../../components/ui/tooltip'
 import { isPrefixedFormattedHexString } from '../../../../../../app/scripts/lib/util'
+import { jsonRpcRequest } from '../../../../helpers/utils/util'
 
 export default class NetworkForm extends PureComponent {
   static contextTypes = {
@@ -88,7 +90,7 @@ export default class NetworkForm extends PureComponent {
     this.setState({ rpcUrl, chainId, ticker, networkName, blockExplorerUrl, errors: {} })
   }
 
-  onSubmit = () => {
+  onSubmit = async () => {
     const {
       setRpcTarget,
       rpcUrl: propsRpcUrl,
@@ -111,13 +113,17 @@ export default class NetworkForm extends PureComponent {
       chainId = `0x${(new BigNumber(chainId, 10)).toString(16)}`
     }
 
+    if (!(await this.validateChainIdOnSubmit(chainId, rpcUrl))) {
+      return
+    }
+
     if (propsRpcUrl && rpcUrl !== propsRpcUrl) {
-      editRpc(propsRpcUrl, rpcUrl, chainId, ticker, networkName, {
+      await editRpc(propsRpcUrl, rpcUrl, chainId, ticker, networkName, {
         blockExplorerUrl: blockExplorerUrl || rpcPrefs.blockExplorerUrl,
         ...rpcPrefs,
       })
     } else {
-      setRpcTarget(rpcUrl, chainId, ticker, networkName, {
+      await setRpcTarget(rpcUrl, chainId, ticker, networkName, {
         blockExplorerUrl: blockExplorerUrl || rpcPrefs.blockExplorerUrl,
         ...rpcPrefs,
       })
@@ -251,6 +257,36 @@ export default class NetworkForm extends PureComponent {
     this.setErrorTo('chainId', errorMessage)
   }
 
+  validateChainIdOnSubmit = async (chainId, rpcUrl) => {
+    const { t } = this.context
+    let errorMessage
+    let endpointChainId
+    let providerError
+
+    try {
+      endpointChainId = await jsonRpcRequest(rpcUrl, 'eth_chainId')
+    } catch (err) {
+      log.warn('Failed to fetch the chainId from the endpoint.', err)
+      providerError = err
+    }
+
+    if (providerError || typeof endpointChainId !== 'string') {
+      errorMessage = t('failedToFetchChainId')
+    } else if (chainId !== endpointChainId) {
+      errorMessage = t('endpointReturnedDifferentChainId', [
+        endpointChainId.length <= 12
+          ? endpointChainId
+          : `${endpointChainId.slice(0, 9)}...`,
+      ])
+    }
+
+    if (errorMessage) {
+      this.setErrorTo('chainId', errorMessage)
+      return false
+    }
+    return true
+  }
+
   isValidWhenAppended = (url) => {
     const appendedRpc = `http://${url}`
     return validUrl.isWebUri(appendedRpc) && !url.match(/^https?:\/\/$/u)

ui/app/pages/settings/networks-tab/networks-tab.container.js

@@ -63,7 +63,7 @@ const mapDispatchToProps = (dispatch) => {
   return {
     setSelectedSettingsRpcUrl: (newRpcUrl) => dispatch(setSelectedSettingsRpcUrl(newRpcUrl)),
     setRpcTarget: (newRpc, chainId, ticker, nickname, rpcPrefs) => {
-      dispatch(updateAndSetCustomRpc(newRpc, chainId, ticker, nickname, rpcPrefs))
+      return dispatch(updateAndSetCustomRpc(newRpc, chainId, ticker, nickname, rpcPrefs))
     },
     showConfirmDeleteNetworkModal: ({ target, onConfirm }) => {
       return dispatch(showModal({ name: 'CONFIRM_DELETE_NETWORK', target, onConfirm }))
@@ -71,7 +71,7 @@ const mapDispatchToProps = (dispatch) => {
     displayWarning: (warning) => dispatch(displayWarning(warning)),
     setNetworksTabAddMode: (isInAddMode) => dispatch(setNetworksTabAddMode(isInAddMode)),
     editRpc: (oldRpc, newRpc, chainId, ticker, nickname, rpcPrefs) => {
-      dispatch(editRpc(oldRpc, newRpc, chainId, ticker, nickname, rpcPrefs))
+      return dispatch(editRpc(oldRpc, newRpc, chainId, ticker, nickname, rpcPrefs))
     },
   }
 }