From 9c6c277b8b5d33568d81a81c3e13fc06b0f7241a Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 2 Jan 2017 14:38:04 -0800 Subject: [PATCH 1/4] Add replay protection to Transaction Manager Fixes #897 Needs tests. --- app/scripts/metamask-controller.js | 6 ++--- app/scripts/transaction-manager.js | 43 +++++++++++++++++------------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 5df10672a..31bb4a9c2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -177,8 +177,8 @@ module.exports = class MetamaskController { // tx signing approveTransaction: this.newUnsignedTransaction.bind(this), signTransaction: (...args) => { - this.setupSigningListners(...args) - this.txManager.formatTxForSigining(...args) + this.setupSigningListeners(...args) + this.txManager.formatTxForSigning(...args) this.sendUpdate() }, @@ -257,7 +257,7 @@ module.exports = class MetamaskController { }) } - setupSigningListners (txParams) { + setupSigningListeners (txParams) { var txId = txParams.metamaskId // apply event listeners for signing and formating events this.txManager.once(`${txId}:formatted`, this.keyringController.signTransaction.bind(this.keyringController)) diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index fd136a51b..3cf12b016 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -137,26 +137,33 @@ module.exports = class TransactionManager extends EventEmitter { } // formats txParams so the keyringController can sign it - formatTxForSigining (txParams, cb) { - var address = txParams.from - var metaTx = this.getTx(txParams.metamaskId) - var gasMultiplier = metaTx.gasMultiplier - var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16) - gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10)) - txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber()) + formatTxForSigning (txParams, cb) { + this.getNetwork((err, networkId) => { + if (err) { + return cb(err) + } - // normalize values - txParams.to = normalize(txParams.to) - txParams.from = normalize(txParams.from) - txParams.value = normalize(txParams.value) - txParams.data = normalize(txParams.data) - txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) - txParams.nonce = normalize(txParams.nonce) - const ethTx = new Transaction(txParams) + var address = txParams.from + var metaTx = this.getTx(txParams.metamaskId) + var gasMultiplier = metaTx.gasMultiplier + var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16) + gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10)) + txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber()) - // listener is assigned in metamaskController - this.emit(`${txParams.metamaskId}:formatted`, ethTx, address, txParams.metamaskId, cb) - } + // normalize values + txParams.to = normalize(txParams.to) + txParams.from = normalize(txParams.from) + txParams.value = normalize(txParams.value) + txParams.data = normalize(txParams.data) + txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) + txParams.nonce = normalize(txParams.nonce) + + const ethTx = new Transaction(txParams, parseInt(networkId)) + + // listener is assigned in metamaskController + this.emit(`${txParams.metamaskId}:formatted`, ethTx, address, txParams.metamaskId, cb) + }) + } // receives a signed tx object and updates the tx hash // and pass it to the cb to be sent off From d2fb48f8ba1c717d3360d48551b568b511b7d84d Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 2 Jan 2017 14:41:04 -0800 Subject: [PATCH 2/4] Bump changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 239203553..0a06314e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Current Master - Add a check for when a tx is included in a block. +- Implement replay attack protections allowed by EIP 155. ## 2.14.1 2016-12-20 From 7320095079717b296776ac8e38571020a5d558cf Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 2 Jan 2017 14:54:31 -0800 Subject: [PATCH 3/4] Linted --- app/scripts/transaction-manager.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index 3cf12b016..7fcdbe7ca 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -162,8 +162,8 @@ module.exports = class TransactionManager extends EventEmitter { // listener is assigned in metamaskController this.emit(`${txParams.metamaskId}:formatted`, ethTx, address, txParams.metamaskId, cb) - }) - } + }) + } // receives a signed tx object and updates the tx hash // and pass it to the cb to be sent off From 305cda4265e84d520d21c473a796e8f71febc57f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 2 Jan 2017 15:31:05 -0800 Subject: [PATCH 4/4] Use chainId parameter instead of second argument --- app/scripts/transaction-manager.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index 7fcdbe7ca..d426993a4 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -157,8 +157,9 @@ module.exports = class TransactionManager extends EventEmitter { txParams.data = normalize(txParams.data) txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) txParams.nonce = normalize(txParams.nonce) + txParams.chainId = parseInt(networkId) - const ethTx = new Transaction(txParams, parseInt(networkId)) + const ethTx = new Transaction(txParams) // listener is assigned in metamaskController this.emit(`${txParams.metamaskId}:formatted`, ethTx, address, txParams.metamaskId, cb)