From 45ba657ca1333fe55b25bd6e27e2df0822ea04af Mon Sep 17 00:00:00 2001 From: Erik Marks <25517051+rekmarks@users.noreply.github.com> Date: Mon, 12 Oct 2020 12:10:19 -0700 Subject: [PATCH] Fix signTypedData_v4 chainId param validation (#9552) --- app/scripts/lib/typed-message-manager.js | 7 +++---- app/scripts/metamask-controller.js | 4 +++- test/unit/app/typed-message-manager.spec.js | 6 +----- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/app/scripts/lib/typed-message-manager.js b/app/scripts/lib/typed-message-manager.js index c0378b87c..4769f6e4c 100644 --- a/app/scripts/lib/typed-message-manager.js +++ b/app/scripts/lib/typed-message-manager.js @@ -32,9 +32,9 @@ export default class TypedMessageManager extends EventEmitter { /** * Controller in charge of managing - storing, adding, removing, updating - TypedMessage. */ - constructor ({ networkController }) { + constructor ({ getCurrentChainId }) { super() - this.networkController = networkController + this._getCurrentChainId = getCurrentChainId this.memStore = new ObservableStore({ unapprovedTypedMessages: {}, unapprovedTypedMessagesCount: 0, @@ -167,8 +167,7 @@ export default class TypedMessageManager extends EventEmitter { assert.equal(validation.errors.length, 0, 'Signing data must conform to EIP-712 schema. See https://git.io/fNtcx.') const { chainId } = data.domain if (chainId) { - // eslint-disable-next-line radix - const activeChainId = parseInt(this.networkController.getNetworkState()) + const activeChainId = parseInt(this._getCurrentChainId(), 16) assert.ok(!Number.isNaN(activeChainId), `Cannot sign messages for chainId "${chainId}", because MetaMask is switching networks.`) assert.equal(chainId, activeChainId, `Provided chainId "${chainId}" must match the active chainId "${activeChainId}"`) } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 130500dab..81b3665a0 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -290,7 +290,9 @@ export default class MetamaskController extends EventEmitter { this.personalMessageManager = new PersonalMessageManager() this.decryptMessageManager = new DecryptMessageManager() this.encryptionPublicKeyManager = new EncryptionPublicKeyManager() - this.typedMessageManager = new TypedMessageManager({ networkController: this.networkController }) + this.typedMessageManager = new TypedMessageManager({ + getCurrentChainId: this.networkController.getCurrentChainId.bind(this.networkController), + }) this.swapsController = new SwapsController({ getBufferedGasLimit: this.txController.txGasUtil.getBufferedGasLimit.bind(this.txController.txGasUtil), diff --git a/test/unit/app/typed-message-manager.spec.js b/test/unit/app/typed-message-manager.spec.js index 559628dd4..7598ea185 100644 --- a/test/unit/app/typed-message-manager.spec.js +++ b/test/unit/app/typed-message-manager.spec.js @@ -1,6 +1,5 @@ import assert from 'assert' import sinon from 'sinon' -import NetworkController from '../../../app/scripts/controllers/network' import TypedMessageManager from '../../../app/scripts/lib/typed-message-manager' describe('Typed Message Manager', function () { @@ -9,11 +8,8 @@ describe('Typed Message Manager', function () { const address = '0xc42edfcc21ed14dda456aa0756c153f7985d8813' beforeEach(async function () { - const networkController = new NetworkController() - sinon.stub(networkController, 'getNetworkState').returns('0x1') - typedMessageManager = new TypedMessageManager({ - networkController, + getCurrentChainId: sinon.fake.returns('0x1'), }) msgParamsV1 = {