From bf1bb6ca7e37aacdd6f4bc33bb457766bed05cb2 Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Wed, 7 Oct 2020 07:28:22 -0700 Subject: [PATCH] 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 | 9 ++- test/e2e/metamask-ui.spec.js | 72 ++++++++++++------- ui/app/helpers/utils/util.js | 40 +++++++++++ .../network-form/network-form.component.js | 42 ++++++++++- .../networks-tab/networks-tab.container.js | 4 +- 5 files changed, 137 insertions(+), 30 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index a2fc5173c..6969c2431 100644 --- a/app/_locales/en/messages.json +++ b/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" diff --git a/test/e2e/metamask-ui.spec.js b/test/e2e/metamask-ui.spec.js index 12fcff05a..fd2acaf98 100644 --- a/test/e2e/metamask-ui.spec.js +++ b/test/e2e/metamask-ui.spec.js @@ -1258,36 +1258,60 @@ describe('MetaMask', function () { }) describe('Stores custom RPC history', function () { - const customRpcInfo = [ - { rpcUrl: 'http://127.0.0.1:8545/1', chainId: '0x1' }, - { rpcUrl: 'http://127.0.0.1:8545/2', chainId: '0x2' }, - { rpcUrl: 'http://127.0.0.1:8545/3', chainId: '0x3' }, - { rpcUrl: 'http://127.0.0.1:8545/4', chainId: '0x4' }, - ] + it(`creates first custom RPC entry`, async function () { + const rpcUrl = 'http://127.0.0.1:8545/1' + const chainId = '0x539' // Ganache default, decimal 1337 - customRpcInfo.forEach(({ rpcUrl, chainId }) => { - it(`creates custom RPC: '${rpcUrl}' with chainId '${chainId}'`, async function () { - await driver.clickElement(By.css('.network-name')) - 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.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.xpath(`//span[contains(text(), 'Custom RPC')]`)) - await driver.delay(regularDelayMs) + await driver.clickElement(By.css('.network-form__footer .btn-secondary')) + 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')) + 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] + 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 rpcUrlInput.clear() + await rpcUrlInput.sendKeys(rpcUrl) - await chainIdInput.clear() - await chainIdInput.sendKeys(chainId) + await chainIdInput.clear() + await chainIdInput.sendKeys(chainId) - await driver.clickElement(By.css('.network-form__footer .btn-secondary')) - await driver.delay(largeDelayMs * 2) - }) + 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 () { diff --git a/ui/app/helpers/utils/util.js b/ui/app/helpers/utils/util.js index 7d41a6c06..990cd3f6d 100644 --- a/ui/app/helpers/utils/util.js +++ b/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} [rpcParams] - The RPC method params. + * @returns {Promise} 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 +} diff --git a/ui/app/pages/settings/networks-tab/network-form/network-form.component.js b/ui/app/pages/settings/networks-tab/network-form/network-form.component.js index ea1014afa..f689c8ace 100644 --- a/ui/app/pages/settings/networks-tab/network-form/network-form.component.js +++ b/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) diff --git a/ui/app/pages/settings/networks-tab/networks-tab.container.js b/ui/app/pages/settings/networks-tab/networks-tab.container.js index 4851bf129..7e61c5cff 100644 --- a/ui/app/pages/settings/networks-tab/networks-tab.container.js +++ b/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)) }, } }