Add MAX_SAFE_CHAIN_ID and refactor chain ID validation (#10224)

* Add MAX_SAFE_CHAIN_ID constant
* Add isSafeChainId to shared utils module
* Move isPrefixedFormattedHexString to shared utils module
* Validate custom RPC chain IDs in network controller
* Update some network controller error messages.
* Add isSafeChainId validation to UI

app/_locales/en/messages.json

@@ -845,6 +845,9 @@
   "invalidBlockExplorerURL": {
     "message": "Invalid Block Explorer URL"
   },
+  "invalidChainIdTooBig": {
+    "message": "Invalid chain ID. The chain ID is too big."
+  },
   "invalidCustomNetworkAlertContent1": {
     "message": "The chain ID for custom network '$1' has to be re-entered.",
     "description": "$1 is the name/identifier of the network."

app/scripts/controllers/network/network.js

@@ -18,6 +18,10 @@ import {
   MAINNET_CHAIN_ID,
   RINKEBY_CHAIN_ID,
 } from '../../../../shared/constants/network'
+import {
+  isPrefixedFormattedHexString,
+  isSafeChainId,
+} from '../../../../shared/modules/utils'
 import createMetamaskMiddleware from './createMetamaskMiddleware'
 import createInfuraClient from './createInfuraClient'
 import createJsonRpcClient from './createJsonRpcClient'
@@ -160,6 +164,14 @@ export default class NetworkController extends EventEmitter {
   }
 
   setRpcTarget(rpcUrl, chainId, ticker = 'ETH', nickname = '', rpcPrefs) {
+    assert.ok(
+      isPrefixedFormattedHexString(chainId),
+      `Invalid chain ID "${chainId}": invalid hex string.`,
+    )
+    assert.ok(
+      isSafeChainId(parseInt(chainId, 16)),
+      `Invalid chain ID "${chainId}": numerical value greater than max safe value.`,
+    )
     this.setProviderConfig({
       type: NETWORK_TYPE_RPC,
       rpcUrl,
@@ -171,14 +183,14 @@ export default class NetworkController extends EventEmitter {
   }
 
   async setProviderType(type, rpcUrl = '', ticker = 'ETH', nickname = '') {
-    assert.notEqual(
+    assert.notStrictEqual(
       type,
       NETWORK_TYPE_RPC,
       `NetworkController - cannot call "setProviderType" with type "${NETWORK_TYPE_RPC}". Use "setRpcTarget"`,
     )
-    assert(
+    assert.ok(
       INFURA_PROVIDER_TYPES.includes(type),
-      `NetworkController - Unknown rpc type "${type}"`,
+      `Unknown Infura provider type "${type}".`,
     )
     const { chainId } = NETWORK_TYPE_TO_ID_MAP[type]
     this.setProviderConfig({ type, rpcUrl, chainId, ticker, nickname })

app/scripts/controllers/preferences.js

@@ -5,9 +5,9 @@ import { normalize as normalizeAddress } from 'eth-sig-util'
 import { isValidAddress } from 'ethereumjs-util'
 import ethers from 'ethers'
 import log from 'loglevel'
-import { isPrefixedFormattedHexString } from '../lib/util'
 import { LISTED_CONTRACT_ADDRESSES } from '../../../shared/constants/tokens'
 import { NETWORK_TYPE_TO_ID_MAP } from '../../../shared/constants/network'
+import { isPrefixedFormattedHexString } from '../../../shared/modules/utils'
 
 export default class PreferencesController {
   /**

app/scripts/lib/util.js

@@ -147,21 +147,6 @@ function checkForError() {
   return new Error(lastError.message)
 }
 
-/**
- * Checks whether the given value is a 0x-prefixed, non-zero, non-zero-padded,
- * hexadecimal string.
- *
- * @param {any} value - The value to check.
- * @returns {boolean} True if the value is a correctly formatted hex string,
- * false otherwise.
- */
-function isPrefixedFormattedHexString(value) {
-  if (typeof value !== 'string') {
-    return false
-  }
-  return /^0x[1-9a-f]+[0-9a-f]*$/iu.test(value)
-}
-
 /**
  * Prefixes a hex string with '0x' or '-0x' and returns it. Idempotent.
  *
@@ -202,7 +187,6 @@ export {
   hexToBn,
   BnMultiplyByFraction,
   checkForError,
-  isPrefixedFormattedHexString,
   addHexPrefix,
   bnToHex,
 }

app/scripts/metamask-controller.js

@@ -2402,13 +2402,6 @@ export default class MetamaskController extends EventEmitter {
     nickname,
     rpcPrefs,
   ) {
-    await this.preferencesController.updateRpc({
-      rpcUrl,
-      chainId,
-      ticker,
-      nickname,
-      rpcPrefs,
-    })
     this.networkController.setRpcTarget(
       rpcUrl,
       chainId,
@@ -2416,6 +2409,13 @@ export default class MetamaskController extends EventEmitter {
       nickname,
       rpcPrefs,
     )
+    await this.preferencesController.updateRpc({
+      rpcUrl,
+      chainId,
+      ticker,
+      nickname,
+      rpcPrefs,
+    })
     return rpcUrl
   }
 

shared/constants/network.js

@@ -17,6 +17,12 @@ export const RINKEBY_CHAIN_ID = '0x4'
 export const GOERLI_CHAIN_ID = '0x5'
 export const KOVAN_CHAIN_ID = '0x2a'
 
+/**
+ * The largest possible chain ID we can handle.
+ * Explanation: https://gist.github.com/rekmarks/a47bd5f2525936c4b8eee31a16345553
+ */
+export const MAX_SAFE_CHAIN_ID = 4503599627370476
+
 export const ROPSTEN_DISPLAY_NAME = 'Ropsten'
 export const RINKEBY_DISPLAY_NAME = 'Rinkeby'
 export const KOVAN_DISPLAY_NAME = 'Kovan'

shared/modules/README.md

@@ -1,3 +0,0 @@
-### Shared Modules
-
-This folder is reserved for modules that can be used globally within both the background and ui applications.

shared/modules/utils.js

@@ -0,0 +1,30 @@
+import { MAX_SAFE_CHAIN_ID } from '../constants/network'
+
+/**
+ * Checks whether the given number primitive chain ID is safe.
+ * Because some cryptographic libraries we use expect the chain ID to be a
+ * number primitive, it must not exceed a certain size.
+ *
+ * @param {number} chainId - The chain ID to check for safety.
+ * @returns {boolean} Whether the given chain ID is safe.
+ */
+export function isSafeChainId(chainId) {
+  return (
+    Number.isSafeInteger(chainId) && chainId > 0 && chainId <= MAX_SAFE_CHAIN_ID
+  )
+}
+
+/**
+ * Checks whether the given value is a 0x-prefixed, non-zero, non-zero-padded,
+ * hexadecimal string.
+ *
+ * @param {any} value - The value to check.
+ * @returns {boolean} True if the value is a correctly formatted hex string,
+ * false otherwise.
+ */
+export function isPrefixedFormattedHexString(value) {
+  if (typeof value !== 'string') {
+    return false
+  }
+  return /^0x[1-9a-f]+[0-9a-f]*$/iu.test(value)
+}

test/unit/app/util-test.js

@@ -2,8 +2,8 @@ import { strict as assert } from 'assert'
 import {
   getEnvironmentType,
   sufficientBalance,
-  isPrefixedFormattedHexString,
 } from '../../../app/scripts/lib/util'
+import { isPrefixedFormattedHexString } from '../../../shared/modules/utils'
 
 import {
   ENVIRONMENT_TYPE_POPUP,

ui/app/components/app/dropdowns/network-dropdown.js

@@ -11,10 +11,8 @@ import {
 } from '../../../helpers/constants/routes'
 import { ENVIRONMENT_TYPE_FULLSCREEN } from '../../../../../shared/constants/app'
 import { NETWORK_TYPE_RPC } from '../../../../../shared/constants/network'
-import {
-  getEnvironmentType,
-  isPrefixedFormattedHexString,
-} from '../../../../../app/scripts/lib/util'
+import { isPrefixedFormattedHexString } from '../../../../../shared/modules/utils'
+import { getEnvironmentType } from '../../../../../app/scripts/lib/util'
 
 import { Dropdown, DropdownMenuItem } from './components/dropdown'
 import NetworkDropdownIcon from './components/network-dropdown-icon'

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

@@ -1,12 +1,14 @@
 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 {
+  isPrefixedFormattedHexString,
+  isSafeChainId,
+} from '../../../../../../shared/modules/utils'
 import { jsonRpcRequest } from '../../../../helpers/utils/util'
 
 const FORM_STATE_KEYS = [
@@ -126,7 +128,7 @@ export default class NetworkForm extends PureComponent {
     if (!chainId || typeof chainId !== 'string' || !chainId.startsWith('0x')) {
       return chainId
     }
-    return new BigNumber(chainId, 16).toString(10)
+    return parseInt(chainId, 16).toString(10)
   }
 
   onSubmit = async () => {
@@ -155,7 +157,7 @@ export default class NetworkForm extends PureComponent {
       // Ensure chainId is a 0x-prefixed, lowercase hex string
       let chainId = formChainId
       if (!chainId.startsWith('0x')) {
-        chainId = `0x${new BigNumber(chainId, 10).toString(16)}`
+        chainId = `0x${parseInt(chainId, 10).toString(16)}`
       }
 
       if (!(await this.validateChainIdOnSubmit(formChainId, chainId, rpcUrl))) {
@@ -308,8 +310,10 @@ export default class NetworkForm extends PureComponent {
   validateChainIdOnChange = (chainIdArg = '') => {
     const chainId = chainIdArg.trim()
     let errorMessage = ''
+    let radix = 10
 
     if (chainId.startsWith('0x')) {
+      radix = 16
       if (!/^0x[0-9a-f]+$/iu.test(chainId)) {
         errorMessage = this.context.t('invalidHexNumber')
       } else if (!isPrefixedFormattedHexString(chainId)) {
@@ -319,6 +323,8 @@ export default class NetworkForm extends PureComponent {
       errorMessage = this.context.t('invalidNumber')
     } else if (chainId.startsWith('0')) {
       errorMessage = this.context.t('invalidNumberLeadingZeros')
+    } else if (!isSafeChainId(parseInt(chainId, radix))) {
+      errorMessage = this.context.t('invalidChainIdTooBig')
     }
 
     this.setErrorTo('chainId', errorMessage)
@@ -356,7 +362,7 @@ export default class NetworkForm extends PureComponent {
       // in an error message in the form.
       if (!formChainId.startsWith('0x')) {
         try {
-          endpointChainId = new BigNumber(endpointChainId, 16).toString(10)
+          endpointChainId = parseInt(endpointChainId, 16).toString(10)
         } catch (err) {
           log.warn(
             'Failed to convert endpoint chain ID to decimal',