create safer isValidAddress method

app/scripts/controllers/preferences.js

@@ -2,12 +2,12 @@ import { strict as assert } from 'assert';
 import { ObservableStore } from '@metamask/obs-store';
 import { ethErrors } from 'eth-rpc-errors';
 import { normalize as normalizeAddress } from 'eth-sig-util';
-import { isValidAddress } from 'ethereumjs-util';
 import ethers from 'ethers';
 import log from 'loglevel';
 import { LISTED_CONTRACT_ADDRESSES } from '../../../shared/constants/tokens';
 import { NETWORK_TYPE_TO_ID_MAP } from '../../../shared/constants/network';
 import { isPrefixedFormattedHexString } from '../../../shared/modules/network.utils';
+import { isValidHexAddress } from '../../../shared/modules/hexstring-utils';
 import { NETWORK_EVENTS } from './network';
 
 export default class PreferencesController {
@@ -867,7 +867,7 @@ export default class PreferencesController {
         `Invalid decimals "${decimals}": must be 0 <= 36.`,
       );
     }
-    if (!isValidAddress(address)) {
+    if (!isValidHexAddress(address, { allowNonPrefixed: false })) {
       throw ethErrors.rpc.invalidParams(`Invalid address "${address}".`);
     }
   }

app/scripts/controllers/transactions/lib/util.js

@@ -1,7 +1,7 @@
-import { isValidAddress } from 'ethereumjs-util';
 import { ethErrors } from 'eth-rpc-errors';
 import { addHexPrefix } from '../../../lib/util';
 import { TRANSACTION_STATUSES } from '../../../../../shared/constants/transaction';
+import { isValidHexAddress } from '../../../../../shared/modules/hexstring-utils';
 
 const normalizers = {
   from: (from) => addHexPrefix(from),
@@ -110,7 +110,7 @@ export function validateFrom(txParams) {
       `Invalid "from" address "${txParams.from}": not a string.`,
     );
   }
-  if (!isValidAddress(txParams.from)) {
+  if (!isValidHexAddress(txParams.from, { allowNonPrefixed: false })) {
     throw ethErrors.rpc.invalidParams('Invalid "from" address.');
   }
 }
@@ -128,7 +128,10 @@ export function validateRecipient(txParams) {
     } else {
       throw ethErrors.rpc.invalidParams('Invalid "to" address.');
     }
-  } else if (txParams.to !== undefined && !isValidAddress(txParams.to)) {
+  } else if (
+    txParams.to !== undefined &&
+    !isValidHexAddress(txParams.to, { allowNonPrefixed: false })
+  ) {
     throw ethErrors.rpc.invalidParams('Invalid "to" address.');
   }
   return txParams;

app/scripts/lib/typed-message-manager.js

@@ -3,12 +3,12 @@ import { strict as assert } from 'assert';
 import { ObservableStore } from '@metamask/obs-store';
 import { ethErrors } from 'eth-rpc-errors';
 import { typedSignatureHash, TYPED_MESSAGE_SCHEMA } from 'eth-sig-util';
-import { isValidAddress } from 'ethereumjs-util';
 import log from 'loglevel';
 import jsonschema from 'jsonschema';
 import { MESSAGE_TYPE } from '../../../shared/constants/app';
 import { METAMASK_CONTROLLER_EVENTS } from '../metamask-controller';
 import createId from '../../../shared/modules/random-id';
+import { isValidHexAddress } from '../../../shared/modules/hexstring-utils';
 
 /**
  * Represents, and contains data about, an 'eth_signTypedData' type signature request. These are created when a
@@ -160,7 +160,8 @@ export default class TypedMessageManager extends EventEmitter {
     assert.ok('data' in params, 'Params must include a "data" field.');
     assert.ok('from' in params, 'Params must include a "from" field.');
     assert.ok(
-      typeof params.from === 'string' && isValidAddress(params.from),
+      typeof params.from === 'string' &&
+        isValidHexAddress(params.from, { allowNonPrefixed: false }),
       '"from" field must be a valid, lowercase, hexadecimal Ethereum address string.',
     );
 

shared/modules/hexstring-utils.js

@@ -0,0 +1,53 @@
+import {
+  isHexString,
+  isValidAddress,
+  isValidChecksumAddress,
+  addHexPrefix,
+} from 'ethereumjs-util';
+
+export const BURN_ADDRESS = '0x0000000000000000000000000000000000000000';
+
+export function isBurnAddress(address) {
+  return address === BURN_ADDRESS;
+}
+
+/**
+ * Validates that the input is a hex address. This utility method is a thin
+ * wrapper around ethereumjs-util.isValidAddress, with the exception that it
+ * does not throw an error when provided values that are not hex strings. In
+ * addition, and by default, this method will return true for hex strings that
+ * meet the length requirement of a hex address, but are not prefixed with `0x`
+ * Finally, if the mixedCaseUseChecksum flag is true and a mixed case string is
+ * provided this method will validate it has the proper checksum formatting.
+ * @param {string} possibleAddress - Input parameter to check against
+ * @param {Object} [options] - options bag
+ * @param {boolean} [options.allowNonPrefixed] - If true will first ensure '0x'
+ *  is prepended to the string
+ * @param {boolean} [options.mixedCaseUseChecksum] - If true will treat mixed
+ *  case addresses as checksum addresses and validate that proper checksum
+ *  format is used
+ * @returns {boolean} whether or not the input is a valid hex address
+ */
+export function isValidHexAddress(
+  possibleAddress,
+  { allowNonPrefixed = true, mixedCaseUseChecksum = false } = {},
+) {
+  const addressToCheck = allowNonPrefixed
+    ? addHexPrefix(possibleAddress)
+    : possibleAddress;
+  if (!isHexString(addressToCheck)) {
+    return false;
+  }
+
+  if (mixedCaseUseChecksum) {
+    const prefixRemoved = addressToCheck.slice(2);
+    const lower = prefixRemoved.toLowerCase();
+    const upper = prefixRemoved.toUpperCase();
+    const allOneCase = prefixRemoved === lower || prefixRemoved === upper;
+    if (!allOneCase) {
+      return isValidChecksumAddress(addressToCheck);
+    }
+  }
+
+  return isValidAddress(addressToCheck);
+}

shared/modules/hexstring-utils.test.js

@@ -0,0 +1,57 @@
+import { strict as assert } from 'assert';
+import { toChecksumAddress } from 'ethereumjs-util';
+import { isValidHexAddress } from './hexstring-utils';
+
+describe('hexstring utils', function () {
+  describe('isValidHexAddress', function () {
+    it('should allow 40-char non-prefixed hex', function () {
+      const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b825';
+      const result = isValidHexAddress(address);
+      assert.equal(result, true);
+    });
+
+    it('should allow 42-char prefixed hex', function () {
+      const address = '0xfdea65c8e26263f6d9a1b5de9555d2931a33b825';
+      const result = isValidHexAddress(address);
+      assert.equal(result, true);
+    });
+
+    it('should NOT allow 40-char non-prefixed hex when allowNonPrefixed is false', function () {
+      const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b825';
+      const result = isValidHexAddress(address, { allowNonPrefixed: false });
+      assert.equal(result, false);
+    });
+
+    it('should NOT allow any length of non hex-prefixed string', function () {
+      const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b85';
+      const result = isValidHexAddress(address);
+      assert.equal(result, false);
+    });
+
+    it('should NOT allow less than 42 character hex-prefixed string', function () {
+      const address = '0xfdea65ce26263f6d9a1b5de9555d2931a33b85';
+      const result = isValidHexAddress(address);
+      assert.equal(result, false);
+    });
+
+    it('should recognize correct capitalized checksum', function () {
+      const address = '0xFDEa65C8e26263F6d9A1B5de9555D2931A33b825';
+      const result = isValidHexAddress(address, { mixedCaseUseChecksum: true });
+      assert.equal(result, true);
+    });
+
+    it('should recognize incorrect capitalized checksum', function () {
+      const address = '0xFDea65C8e26263F6d9A1B5de9555D2931A33b825';
+      const result = isValidHexAddress(address, { mixedCaseUseChecksum: true });
+      assert.equal(result, false);
+    });
+
+    it('should recognize this sample hashed address', function () {
+      const address = '0x5Fda30Bb72B8Dfe20e48A00dFc108d0915BE9Bb0';
+      const result = isValidHexAddress(address, { mixedCaseUseChecksum: true });
+      const hashed = toChecksumAddress(address.toLowerCase());
+      assert.equal(hashed, address);
+      assert.equal(result, true);
+    });
+  });
+});

ui/components/ui/identicon/identicon.component.js

@@ -2,8 +2,9 @@ import React, { PureComponent } from 'react';
 import PropTypes from 'prop-types';
 import classnames from 'classnames';
 import contractMap from '@metamask/contract-metadata';
+import { isHexString, addHexPrefix } from 'ethereumjs-util';
 
-import { checksumAddress, isHex } from '../../../helpers/utils/util';
+import { checksumAddress } from '../../../helpers/utils/util';
 import Jazzicon from '../jazzicon';
 import BlockieIdenticon from './blockieIdenticon';
 
@@ -85,12 +86,12 @@ export default class Identicon extends PureComponent {
     }
 
     if (address) {
-      if (isHex(address)) {
-        const checksummedAddress = checksumAddress(address);
+      const checksummedAddress =
+        isHexString(addHexPrefix(address)) &&
+        checksumAddress(addHexPrefix(address));
 
-        if (contractMap[checksummedAddress]?.logo) {
-          return this.renderJazzicon();
-        }
+      if (contractMap[checksummedAddress]?.logo) {
+        return this.renderJazzicon();
       }
 
       return (

ui/components/ui/identicon/identicon.component.test.js

@@ -38,7 +38,11 @@ describe('Identicon', () => {
 
   it('renders div with address prop', () => {
     const wrapper = mount(
-      <Identicon store={store} className="test-address" address="0x0" />,
+      <Identicon
+        store={store}
+        className="test-address"
+        address="0x0000000000000000000000000000000000000000"
+      />,
     );
 
     expect(wrapper.find('div.test-address').prop('className')).toStrictEqual(

ui/helpers/utils/icon-factory.js

@@ -1,5 +1,7 @@
 import contractMap from '@metamask/contract-metadata';
-import { isValidAddress, checksumAddress, isHex } from './util';
+import { isHexString, addHexPrefix } from 'ethereumjs-util';
+import { isValidHexAddress } from '../../../shared/modules/hexstring-utils';
+import { checksumAddress } from './util';
 
 let iconFactory;
 
@@ -18,8 +20,8 @@ function IconFactory(jazzicon) {
 IconFactory.prototype.iconForAddress = function (address, diameter) {
   let addr = address;
 
-  if (isHex(address)) {
-    addr = checksumAddress(address);
+  if (isHexString(addHexPrefix(address))) {
+    addr = checksumAddress(addHexPrefix(address));
   }
 
   if (iconExistsFor(addr)) {
@@ -52,7 +54,9 @@ IconFactory.prototype.generateNewIdenticon = function (address, diameter) {
 
 function iconExistsFor(address) {
   return (
-    contractMap[address] && isValidAddress(address) && contractMap[address].logo
+    contractMap[address] &&
+    isValidHexAddress(address, { allowNonPrefixed: false }) &&
+    contractMap[address].logo
   );
 }
 

ui/helpers/utils/util.js

@@ -78,20 +78,6 @@ export function addressSummary(
     : '...';
 }
 
-export function isValidAddress(address) {
-  if (!address || address === '0x0000000000000000000000000000000000000000') {
-    return false;
-  }
-  const prefixed = addHexPrefix(address);
-  if (!isHex(prefixed)) {
-    return false;
-  }
-  return (
-    (isAllOneCase(prefixed.slice(2)) && ethUtil.isValidAddress(prefixed)) ||
-    ethUtil.isValidChecksumAddress(prefixed)
-  );
-}
-
 export function isValidDomainName(address) {
   const match = punycode
     .toASCII(address)
@@ -112,15 +98,6 @@ export function isOriginContractAddress(to, sendTokenAddress) {
   return to.toLowerCase() === sendTokenAddress.toLowerCase();
 }
 
-export function isAllOneCase(address) {
-  if (!address) {
-    return true;
-  }
-  const lower = address.toLowerCase();
-  const upper = address.toUpperCase();
-  return address === lower || address === upper;
-}
-
 // Takes wei Hex, returns wei BN, even if input is null
 export function numericBalance(balance) {
   if (!balance) {
@@ -182,10 +159,6 @@ export function formatBalance(
   return formatted;
 }
 
-export function isHex(str) {
-  return Boolean(str.match(/^(0x)?[0-9a-fA-F]+$/u));
-}
-
 export function getContractAtAddress(tokenAddress) {
   return global.eth.contract(abi).at(tokenAddress);
 }
@@ -253,13 +226,6 @@ export function shortenAddress(address = '') {
   return `${address.slice(0, 6)}...${address.slice(-4)}`;
 }
 
-export function isValidAddressHead(address) {
-  const addressLengthIsLessThanFull = address.length < 42;
-  const addressIsHex = isHex(address);
-
-  return addressLengthIsLessThanFull && addressIsHex;
-}
-
 export function getAccountByAddress(accounts = [], targetAddress) {
   return accounts.find(({ address }) => address === targetAddress);
 }