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)) {
       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)) {
     throw ethErrors.rpc.invalidParams('Invalid "from" address.');
   }
 }
@@ -128,7 +128,7 @@ 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)) {
     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,7 @@ 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),
       '"from" field must be a valid, lowercase, hexadecimal Ethereum address string.',
     );
 

shared/modules/hexstring-utils.js

@@ -0,0 +1,27 @@
+import {
+  isHexString,
+  isValidAddress,
+  isValidChecksumAddress,
+} from 'ethereumjs-util';
+
+export const BURN_ADDRESS = '0x0000000000000000000000000000000000000000';
+
+export function isBurnAddress(address) {
+  return address === BURN_ADDRESS;
+}
+
+export function isValidHexAddress(possibleAddress) {
+  if (!isHexString(possibleAddress)) {
+    return false;
+  }
+
+  const prefixRemoved = possibleAddress.slice(2);
+  const lower = prefixRemoved.toLowerCase();
+  const upper = prefixRemoved.toUpperCase();
+  const allOneCase = prefixRemoved === lower || prefixRemoved === upper;
+  if (!allOneCase) {
+    return isValidChecksumAddress(possibleAddress);
+  }
+
+  return isValidAddress(possibleAddress);
+}

shared/modules/hexstring-utils.test.js

@@ -0,0 +1,51 @@
+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 NOT allow 40-char non-prefixed hex', function () {
+      const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b825';
+      const result = isValidHexAddress(address);
+      assert.equal(result, false);
+    });
+
+    it('should allow 42-char prefixed hex', function () {
+      const address = '0xfdea65c8e26263f6d9a1b5de9555d2931a33b825';
+      const result = isValidHexAddress(address);
+      assert.equal(result, true);
+    });
+
+    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);
+      assert.equal(result, true);
+    });
+
+    it('should recognize incorrect capitalized checksum', function () {
+      const address = '0xFDea65C8e26263F6d9A1B5de9555D2931A33b825';
+      const result = isValidHexAddress(address);
+      assert.equal(result, false);
+    });
+
+    it('should recognize this sample hashed address', function () {
+      const address = '0x5Fda30Bb72B8Dfe20e48A00dFc108d0915BE9Bb0';
+      const result = isValidHexAddress(address);
+      const hashed = toChecksumAddress(address.toLowerCase());
+      assert.equal(hashed, address);
+      assert.equal(result, true);
+    });
+  });
+});

ui/helpers/utils/icon-factory.js

@@ -1,5 +1,6 @@
 import contractMap from '@metamask/contract-metadata';
-import { isValidAddress, checksumAddress, isHex } from './util';
+import { isValidHexAddress } from '../../../shared/modules/hexstring-utils';
+import { checksumAddress, isHex } from './util';
 
 let iconFactory;
 
@@ -52,7 +53,9 @@ IconFactory.prototype.generateNewIdenticon = function (address, diameter) {
 
 function iconExistsFor(address) {
   return (
-    contractMap[address] && isValidAddress(address) && contractMap[address].logo
+    contractMap[address] &&
+    isValidHexAddress(address) &&
+    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) {
@@ -253,13 +230,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);
 }

ui/helpers/utils/util.test.js

@@ -1,4 +1,4 @@
-import { BN, toChecksumAddress } from 'ethereumjs-util';
+import { BN } from 'ethereumjs-util';
 import * as util from './util';
 
 describe('util', () => {
@@ -47,52 +47,6 @@ describe('util', () => {
     });
   });
 
-  describe('#isValidAddress', () => {
-    it('should allow 40-char non-prefixed hex', () => {
-      const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b825';
-      const result = util.isValidAddress(address);
-      expect(result).toStrictEqual(true);
-    });
-
-    it('should allow 42-char non-prefixed hex', () => {
-      const address = '0xfdea65c8e26263f6d9a1b5de9555d2931a33b825';
-      const result = util.isValidAddress(address);
-      expect(result).toStrictEqual(true);
-    });
-
-    it('should not allow less non hex-prefixed', () => {
-      const address = 'fdea65c8e26263f6d9a1b5de9555d2931a33b85';
-      const result = util.isValidAddress(address);
-      expect(result).toStrictEqual(false);
-    });
-
-    it('should not allow less hex-prefixed', () => {
-      const address = '0xfdea65ce26263f6d9a1b5de9555d2931a33b85';
-      const result = util.isValidAddress(address);
-      expect(result).toStrictEqual(false);
-    });
-
-    it('should recognize correct capitalized checksum', () => {
-      const address = '0xFDEa65C8e26263F6d9A1B5de9555D2931A33b825';
-      const result = util.isValidAddress(address);
-      expect(result).toStrictEqual(true);
-    });
-
-    it('should recognize incorrect capitalized checksum', () => {
-      const address = '0xFDea65C8e26263F6d9A1B5de9555D2931A33b825';
-      const result = util.isValidAddress(address);
-      expect(result).toStrictEqual(false);
-    });
-
-    it('should recognize this sample hashed address', () => {
-      const address = '0x5Fda30Bb72B8Dfe20e48A00dFc108d0915BE9Bb0';
-      const result = util.isValidAddress(address);
-      const hashed = toChecksumAddress(address.toLowerCase());
-      expect(hashed).toStrictEqual(address);
-      expect(result).toStrictEqual(true);
-    });
-  });
-
   describe('isValidDomainName', () => {
     it('should return true when given a valid domain name', () => {
       expect(util.isValidDomainName('foo.bar')).toStrictEqual(true);

ui/pages/add-token/add-token.component.js

@@ -1,15 +1,13 @@
 import React, { Component } from 'react';
 import PropTypes from 'prop-types';
-import {
-  checkExistingAddresses,
-  isValidAddress,
-} from '../../helpers/utils/util';
+import { checkExistingAddresses } from '../../helpers/utils/util';
 import { tokenInfoGetter } from '../../helpers/utils/token-util';
 import { CONFIRM_ADD_TOKEN_ROUTE } from '../../helpers/constants/routes';
 import TextField from '../../components/ui/text-field';
 import PageContainer from '../../components/ui/page-container';
 import { Tabs, Tab } from '../../components/ui/tabs';
 import { addHexPrefix } from '../../../app/scripts/lib/util';
+import { isValidHexAddress } from '../../../shared/modules/hexstring-utils';
 import TokenList from './token-list';
 import TokenSearch from './token-search';
 
@@ -167,7 +165,7 @@ class AddToken extends Component {
       autoFilled: false,
     });
 
-    const addressIsValid = isValidAddress(customAddress);
+    const addressIsValid = isValidHexAddress(customAddress);
     const standardAddress = addHexPrefix(customAddress).toLowerCase();
 
     switch (true) {

ui/pages/send/send-content/add-recipient/add-recipient.component.js

@@ -2,13 +2,16 @@ import React, { Component } from 'react';
 import PropTypes from 'prop-types';
 import Fuse from 'fuse.js';
 import Identicon from '../../../../components/ui/identicon';
-import { isValidAddress } from '../../../../helpers/utils/util';
 import Dialog from '../../../../components/ui/dialog';
 import ContactList from '../../../../components/app/contact-list';
 import RecipientGroup from '../../../../components/app/contact-list/recipient-group/recipient-group.component';
 import { ellipsify } from '../../send.utils';
 import Button from '../../../../components/ui/button';
 import Confusable from '../../../../components/ui/confusable';
+import {
+  isBurnAddress,
+  isValidHexAddress,
+} from '../../../../../shared/modules/hexstring-utils';
 
 export default class AddRecipient extends Component {
   static propTypes = {
@@ -101,7 +104,7 @@ export default class AddRecipient extends Component {
 
     let content;
 
-    if (isValidAddress(query)) {
+    if (!isBurnAddress(query) && isValidHexAddress(query)) {
       content = this.renderExplicitAddress(query);
     } else if (ensResolution) {
       content = this.renderExplicitAddress(

ui/pages/send/send-content/add-recipient/add-recipient.js

@@ -11,18 +11,24 @@ import {
 } from '../../send.constants';
 
 import {
-  isValidAddress,
   checkExistingAddresses,
   isValidDomainName,
   isOriginContractAddress,
   isDefaultMetaMaskChain,
 } from '../../../../helpers/utils/util';
+import {
+  isBurnAddress,
+  isValidHexAddress,
+} from '../../../../../shared/modules/hexstring-utils';
 
 export function getToErrorObject(to, sendTokenAddress, chainId) {
   let toError = null;
   if (!to) {
     toError = REQUIRED_ERROR;
-  } else if (!isValidAddress(to) && !isValidDomainName(to)) {
+  } else if (
+    isBurnAddress(to) ||
+    (!isValidHexAddress(to) && !isValidDomainName(to))
+  ) {
     toError = isDefaultMetaMaskChain(chainId)
       ? INVALID_RECIPIENT_ADDRESS_ERROR
       : INVALID_RECIPIENT_ADDRESS_NOT_ETH_NETWORK_ERROR;

ui/pages/send/send-content/add-recipient/add-recipient.utils.test.js

@@ -11,13 +11,19 @@ jest.mock('../../../../helpers/utils/util', () => ({
   isDefaultMetaMaskChain: jest.fn().mockReturnValue(true),
   isEthNetwork: jest.fn().mockReturnValue(true),
   checkExistingAddresses: jest.fn().mockReturnValue(true),
-  isValidAddress: jest.fn((to) => Boolean(to.match(/^[0xabcdef123456798]+$/u))),
   isValidDomainName: jest.requireActual('../../../../helpers/utils/util')
     .isValidDomainName,
   isOriginContractAddress: jest.requireActual('../../../../helpers/utils/util')
     .isOriginContractAddress,
 }));
 
+jest.mock('../../../../../shared/modules/hexstring-utils', () => ({
+  isValidHexAddress: jest.fn((to) =>
+    Boolean(to.match(/^[0xabcdef123456798]+$/u)),
+  ),
+  isBurnAddress: jest.fn(() => false),
+}));
+
 describe('add-recipient utils', () => {
   describe('getToErrorObject()', () => {
     it('should return a required error if "to" is falsy', () => {