From 1d65687ce48bc7f35ee0167c94813f8b3cb3a6ee Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 24 Oct 2018 20:03:55 -0700 Subject: [PATCH 1/4] Validate signTypedData in eth-json-rpc-middleware --- .../network/createMetamaskMiddleware.js | 2 + app/scripts/metamask-controller.js | 58 ++++++++++--------- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/app/scripts/controllers/network/createMetamaskMiddleware.js b/app/scripts/controllers/network/createMetamaskMiddleware.js index 9e6a45888..319c5bf3e 100644 --- a/app/scripts/controllers/network/createMetamaskMiddleware.js +++ b/app/scripts/controllers/network/createMetamaskMiddleware.js @@ -11,6 +11,7 @@ function createMetamaskMiddleware ({ processTransaction, processEthSignMessage, processTypedMessage, + processTypedMessageV3, processPersonalMessage, getPendingNonce, }) { @@ -25,6 +26,7 @@ function createMetamaskMiddleware ({ processTransaction, processEthSignMessage, processTypedMessage, + processTypedMessageV3, processPersonalMessage, }), createPendingNonceMiddleware({ getPendingNonce }), diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 7913662d4..1e02d8488 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -138,12 +138,12 @@ module.exports = class MetamaskController extends EventEmitter { this.accountTracker.stop() } }) - + // ensure accountTracker updates balances after network change this.networkController.on('networkDidChange', () => { this.accountTracker._updateAccounts() }) - + // key mgmt const additionalKeyrings = [TrezorKeyring, LedgerBridgeKeyring] this.keyringController = new KeyringController({ @@ -275,6 +275,8 @@ module.exports = class MetamaskController extends EventEmitter { processTransaction: this.newUnapprovedTransaction.bind(this), // msg signing processEthSignMessage: this.newUnsignedMessage.bind(this), + processTypedMessage: this.newUnsignedTypedMessage.bind(this), + processTypedMessageV3: this.newUnsignedTypedMessage.bind(this), processPersonalMessage: this.newUnsignedPersonalMessage.bind(this), getPendingNonce: this.getPendingNonce.bind(this), } @@ -978,8 +980,8 @@ module.exports = class MetamaskController extends EventEmitter { * @param {Object} msgParams - The params passed to eth_signTypedData. * @param {Function} cb - The callback function, called with the signature. */ - newUnsignedTypedMessage (msgParams, req) { - const promise = this.typedMessageManager.addUnapprovedMessageAsync(msgParams, req) + newUnsignedTypedMessage (msgParams, req, version) { + const promise = this.typedMessageManager.addUnapprovedMessageAsync(msgParams, req, version) this.sendUpdate() this.opts.showUnconfirmedMessage() return promise @@ -1274,9 +1276,9 @@ module.exports = class MetamaskController extends EventEmitter { // watch asset engine.push(this.preferencesController.requestWatchAsset.bind(this.preferencesController)) // sign typed data middleware - engine.push(this.createTypedDataMiddleware('eth_signTypedData', 'V1').bind(this)) - engine.push(this.createTypedDataMiddleware('eth_signTypedData_v1', 'V1').bind(this)) - engine.push(this.createTypedDataMiddleware('eth_signTypedData_v3', 'V3', true).bind(this)) + // engine.push(this.createTypedDataMiddleware('eth_signTypedData', 'V1').bind(this)) + // engine.push(this.createTypedDataMiddleware('eth_signTypedData_v1', 'V1').bind(this)) + // engine.push(this.createTypedDataMiddleware('eth_signTypedData_v3', 'V3', true).bind(this)) // forward to metamask primary provider engine.push(createProviderMiddleware({ provider })) @@ -1542,27 +1544,27 @@ module.exports = class MetamaskController extends EventEmitter { * @param {Function} - next * @param {Function} - end */ - createTypedDataMiddleware (methodName, version, reverse) { - return async (req, res, next, end) => { - const { method, params } = req - if (method === methodName) { - const promise = this.typedMessageManager.addUnapprovedMessageAsync({ - data: reverse ? params[1] : params[0], - from: reverse ? params[0] : params[1], - }, req, version) - this.sendUpdate() - this.opts.showUnconfirmedMessage() - try { - res.result = await promise - end() - } catch (error) { - end(error) - } - } else { - next() - } - } - } + // createTypedDataMiddleware (methodName, version, reverse) { + // return async (req, res, next, end) => { + // const { method, params } = req + // if (method === methodName) { + // const promise = this.typedMessageManager.addUnapprovedMessageAsync({ + // data: reverse ? params[1] : params[0], + // from: reverse ? params[0] : params[1], + // }, req, version) + // this.sendUpdate() + // this.opts.showUnconfirmedMessage() + // try { + // res.result = await promise + // end() + // } catch (error) { + // end(error) + // } + // } else { + // next() + // } + // } + // } /** * Adds a domain to the {@link BlacklistController} whitelist From 95b92a1ddcaf478fe612f46c94c8e0600826cfd6 Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 24 Oct 2018 20:13:40 -0700 Subject: [PATCH 2/4] Remove commented out/unused methods --- app/scripts/metamask-controller.js | 29 ++--------------------------- 1 file changed, 2 insertions(+), 27 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 1e02d8488..5a182d3f0 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -980,8 +980,8 @@ module.exports = class MetamaskController extends EventEmitter { * @param {Object} msgParams - The params passed to eth_signTypedData. * @param {Function} cb - The callback function, called with the signature. */ - newUnsignedTypedMessage (msgParams, req, version) { - const promise = this.typedMessageManager.addUnapprovedMessageAsync(msgParams, req, version) + newUnsignedTypedMessage (msgParams, req) { + const promise = this.typedMessageManager.addUnapprovedMessageAsync(msgParams, req) this.sendUpdate() this.opts.showUnconfirmedMessage() return promise @@ -1275,10 +1275,6 @@ module.exports = class MetamaskController extends EventEmitter { engine.push(subscriptionManager.middleware) // watch asset engine.push(this.preferencesController.requestWatchAsset.bind(this.preferencesController)) - // sign typed data middleware - // engine.push(this.createTypedDataMiddleware('eth_signTypedData', 'V1').bind(this)) - // engine.push(this.createTypedDataMiddleware('eth_signTypedData_v1', 'V1').bind(this)) - // engine.push(this.createTypedDataMiddleware('eth_signTypedData_v3', 'V3', true).bind(this)) // forward to metamask primary provider engine.push(createProviderMiddleware({ provider })) @@ -1544,27 +1540,6 @@ module.exports = class MetamaskController extends EventEmitter { * @param {Function} - next * @param {Function} - end */ - // createTypedDataMiddleware (methodName, version, reverse) { - // return async (req, res, next, end) => { - // const { method, params } = req - // if (method === methodName) { - // const promise = this.typedMessageManager.addUnapprovedMessageAsync({ - // data: reverse ? params[1] : params[0], - // from: reverse ? params[0] : params[1], - // }, req, version) - // this.sendUpdate() - // this.opts.showUnconfirmedMessage() - // try { - // res.result = await promise - // end() - // } catch (error) { - // end(error) - // } - // } else { - // next() - // } - // } - // } /** * Adds a domain to the {@link BlacklistController} whitelist From 8c0e5e97e5ee50122e4ff120e1076e71394be378 Mon Sep 17 00:00:00 2001 From: Thomas Date: Wed, 24 Oct 2018 20:24:42 -0700 Subject: [PATCH 3/4] Add version to unapprovedMessage --- app/scripts/metamask-controller.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 5a182d3f0..bbff95618 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -980,8 +980,8 @@ module.exports = class MetamaskController extends EventEmitter { * @param {Object} msgParams - The params passed to eth_signTypedData. * @param {Function} cb - The callback function, called with the signature. */ - newUnsignedTypedMessage (msgParams, req) { - const promise = this.typedMessageManager.addUnapprovedMessageAsync(msgParams, req) + newUnsignedTypedMessage (msgParams, req, version) { + const promise = this.typedMessageManager.addUnapprovedMessageAsync(msgParams, req, version) this.sendUpdate() this.opts.showUnconfirmedMessage() return promise From 715c309d68009b6566958c34b5a23b8919d59ada Mon Sep 17 00:00:00 2001 From: kumavis Date: Mon, 29 Oct 2018 19:20:35 -0400 Subject: [PATCH 4/4] deps - bump eth-json-rpc-middleware for signTypedData version bump --- package-lock.json | 84 +++++++++++++++++++++++++++-------------------- package.json | 2 +- 2 files changed, 49 insertions(+), 37 deletions(-) diff --git a/package-lock.json b/package-lock.json index 22f0f3ab9..7a28cfdf3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9899,29 +9899,26 @@ } }, "eth-json-rpc-middleware": { - "version": "3.1.3", - "resolved": "https://registry.npmjs.org/eth-json-rpc-middleware/-/eth-json-rpc-middleware-3.1.3.tgz", - "integrity": "sha512-glp/mCefhsqrgVOTTuYlHYiTL+9mMPfaZsuQv4vnRg3kqNigblS1nqARaMeVW9WOM8ssh9TqIFpuUr7JDgNmKQ==", + "version": "3.1.6", + "resolved": "https://registry.npmjs.org/eth-json-rpc-middleware/-/eth-json-rpc-middleware-3.1.6.tgz", + "integrity": "sha512-yf17/rAM4ElKMul8oSvuK7JuYIYEFFdy2YGPo2EZbuOEv2Wq1bteMlppgqZ9NYHriXLAOWV+ojY9kWHGbcU4xA==", "dev": true, "requires": { - "async": "^2.5.0", "btoa": "^1.2.1", "clone": "^2.1.1", "eth-query": "^2.1.2", "eth-sig-util": "^1.4.2", - "eth-tx-summary": "^3.1.2", + "eth-tx-summary": "^3.2.3", "ethereumjs-block": "^1.6.0", "ethereumjs-tx": "^1.3.3", "ethereumjs-util": "^5.1.2", - "ethereumjs-vm": "^2.1.0", + "ethereumjs-vm": "^2.4.0", "fetch-ponyfill": "^4.0.0", - "json-rpc-engine": "^3.6.3", + "json-rpc-engine": "^3.8.0", "json-rpc-error": "^2.0.0", "json-stable-stringify": "^1.0.1", "pify": "^3.0.0", - "promise-to-callback": "^1.0.0", - "safe-event-emitter": "^1.0.1", - "tape": "^4.6.3" + "safe-event-emitter": "^1.0.1" }, "dependencies": { "eth-sig-util": { @@ -9930,24 +9927,14 @@ "integrity": "sha1-jZWCAsftuq6Dlwf7pvCf8ydgYhA=", "dev": true, "requires": { - "ethereumjs-abi": "git+https://github.com/ethereumjs/ethereumjs-abi.git#00ba8463a7f7a67fcad737ff9c2ebd95643427f7", + "ethereumjs-abi": "git+https://github.com/ethereumjs/ethereumjs-abi.git#2863c40e0982acfc0b7163f0285d4c56427c7799", "ethereumjs-util": "^5.1.1" - }, - "dependencies": { - "ethereumjs-abi": { - "version": "git+https://github.com/ethereumjs/ethereumjs-abi.git#00ba8463a7f7a67fcad737ff9c2ebd95643427f7", - "from": "git+https://github.com/ethereumjs/ethereumjs-abi.git#00ba8463a7f7a67fcad737ff9c2ebd95643427f7", - "dev": true, - "requires": { - "bn.js": "^4.10.0", - "ethereumjs-util": "^5.0.0" - } - } } }, "ethereumjs-abi": { - "version": "git+https://github.com/ethereumjs/ethereumjs-abi.git#00ba8463a7f7a67fcad737ff9c2ebd95643427f7", + "version": "git+https://github.com/ethereumjs/ethereumjs-abi.git#2863c40e0982acfc0b7163f0285d4c56427c7799", "from": "git+https://github.com/ethereumjs/ethereumjs-abi.git", + "dev": true, "requires": { "bn.js": "^4.10.0", "ethereumjs-util": "^5.0.0" @@ -9957,6 +9944,7 @@ "version": "5.2.0", "resolved": "https://registry.npmjs.org/ethereumjs-util/-/ethereumjs-util-5.2.0.tgz", "integrity": "sha512-CJAKdI0wgMbQFLlLRtZKGcy/L6pzVRgelIZqRqNbuVFM3K9VEnyfbcvz0ncWMRNCe4kaHWjwRYQcYMucmwsnWA==", + "dev": true, "requires": { "bn.js": "^4.11.0", "create-hash": "^1.1.2", @@ -9966,6 +9954,31 @@ "safe-buffer": "^5.1.1", "secp256k1": "^3.0.1" } + }, + "ethereumjs-vm": { + "version": "2.4.0", + "resolved": "https://registry.npmjs.org/ethereumjs-vm/-/ethereumjs-vm-2.4.0.tgz", + "integrity": "sha512-MJ4lCWa5c6LhahhhvoDKW+YGjK00ZQn0RHHLh4L+WaH1k6Qv7/q3uTluew6sJGNCZdlO0yYMDXYW9qyxLHKlgQ==", + "dev": true, + "requires": { + "async": "^2.1.2", + "async-eventemitter": "^0.2.2", + "ethereumjs-account": "^2.0.3", + "ethereumjs-block": "~1.7.0", + "ethereumjs-common": "~0.4.0", + "ethereumjs-util": "^5.2.0", + "fake-merkle-patricia-tree": "^1.0.1", + "functional-red-black-tree": "^1.0.1", + "merkle-patricia-tree": "^2.1.2", + "rustbn.js": "~0.2.0", + "safe-buffer": "^5.1.1" + } + }, + "rustbn.js": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/rustbn.js/-/rustbn.js-0.2.0.tgz", + "integrity": "sha512-4VlvkRUuCJvr2J6Y0ImW7NvTCriMi7ErOAqWk1y69vAdoNIzCF3yPmgeNzx+RQTLEDFq5sHfscn1MwHxP9hNfA==", + "dev": true } } }, @@ -10916,6 +10929,12 @@ } } }, + "ethereumjs-common": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/ethereumjs-common/-/ethereumjs-common-0.4.1.tgz", + "integrity": "sha512-ywYGsOeGCsMNWso5Y4GhjWI24FJv9FK7+VyVKiQgXg8ZRDPXJ7F/kJ1CnjtkjTvDF4e0yqU+FWswlqR3bmZQ9Q==", + "dev": true + }, "ethereumjs-tx": { "version": "1.3.3", "resolved": "https://registry.npmjs.org/ethereumjs-tx/-/ethereumjs-tx-1.3.3.tgz", @@ -12788,8 +12807,7 @@ }, "code-point-at": { "version": "1.1.0", - "bundled": true, - "optional": true + "bundled": true }, "concat-map": { "version": "0.0.1", @@ -12797,8 +12815,7 @@ }, "console-control-strings": { "version": "1.1.0", - "bundled": true, - "optional": true + "bundled": true }, "core-util-is": { "version": "1.0.2", @@ -12901,8 +12918,7 @@ }, "inherits": { "version": "2.0.3", - "bundled": true, - "optional": true + "bundled": true }, "ini": { "version": "1.3.5", @@ -12912,7 +12928,6 @@ "is-fullwidth-code-point": { "version": "1.0.0", "bundled": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -13030,8 +13045,7 @@ }, "number-is-nan": { "version": "1.0.1", - "bundled": true, - "optional": true + "bundled": true }, "object-assign": { "version": "4.1.1", @@ -13041,7 +13055,6 @@ "once": { "version": "1.4.0", "bundled": true, - "optional": true, "requires": { "wrappy": "1" } @@ -13147,7 +13160,6 @@ "string-width": { "version": "1.0.2", "bundled": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -26768,7 +26780,7 @@ }, "pretty-hrtime": { "version": "1.0.3", - "resolved": "http://registry.npmjs.org/pretty-hrtime/-/pretty-hrtime-1.0.3.tgz", + "resolved": "https://registry.npmjs.org/pretty-hrtime/-/pretty-hrtime-1.0.3.tgz", "integrity": "sha1-t+PqQkNaTJsnWdmeDyAesZWALuE=" }, "printf": { @@ -35267,4 +35279,4 @@ "dev": true } } -} \ No newline at end of file +} diff --git a/package.json b/package.json index ef72d7bfb..0341e25af 100644 --- a/package.json +++ b/package.json @@ -261,7 +261,7 @@ "eslint-plugin-json": "^1.2.0", "eslint-plugin-mocha": "^5.0.0", "eslint-plugin-react": "^7.4.0", - "eth-json-rpc-middleware": "^3.1.3", + "eth-json-rpc-middleware": "^3.1.6", "eth-keyring-controller": "^3.3.1", "fetch-mock": "^6.5.2", "file-loader": "^1.1.11",