diff options
| -rw-r--r-- | CHANGELOG.md | 9 | ||||
| -rw-r--r-- | app/manifest.json | 2 | ||||
| -rw-r--r-- | app/scripts/contentscript.js | 6 | ||||
| -rw-r--r-- | app/scripts/controllers/transactions.js | 59 | ||||
| -rw-r--r-- | app/scripts/lib/tx-gas-utils.js | 35 | ||||
| -rw-r--r-- | app/scripts/lib/tx-state-manager.js | 2 | ||||
| -rw-r--r-- | app/scripts/migrations/024.js | 45 | ||||
| -rw-r--r-- | app/scripts/migrations/index.js | 1 | ||||
| -rw-r--r-- | test/unit/migrations/024-test.js | 37 | ||||
| -rw-r--r-- | test/unit/tx-controller-test.js | 89 | ||||
| -rw-r--r-- | test/unit/tx-gas-util-test.js | 42 | ||||
| -rw-r--r-- | ui/app/components/ens-input.js | 4 | ||||
| -rw-r--r-- | ui/app/components/pending-tx/confirm-send-ether.js | 3 |
13 files changed, 239 insertions, 95 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 479e422f2..f427d1e1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## Current Master +## 4.5.3 Wed Apr 04 2018 + +- Fix bug where checksum address are messing with balance issue [#3843](https://github.com/MetaMask/metamask-extension/issues/3843) +- new ui: fix the confirm transaction screen + +## 4.5.2 Wed Apr 04 2018 + +- Fix overly strict validation where transactions were rejected with hex encoded "chainId" + ## 4.5.1 Tue Apr 03 2018 - Fix default network (should be mainnet not Rinkeby) diff --git a/app/manifest.json b/app/manifest.json index 99305f20e..61d2c5b5e 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "__MSG_appName__", "short_name": "__MSG_appName__", - "version": "4.5.1", + "version": "4.5.3", "manifest_version": 2, "author": "https://metamask.io", "description": "__MSG_appDescription__", diff --git a/app/scripts/contentscript.js b/app/scripts/contentscript.js index 2098fae27..fe1766273 100644 --- a/app/scripts/contentscript.js +++ b/app/scripts/contentscript.js @@ -131,7 +131,11 @@ function documentElementCheck () { } function blacklistedDomainCheck () { - var blacklistedDomains = ['uscourts.gov', 'dropbox.com'] + var blacklistedDomains = [ + 'uscourts.gov', + 'dropbox.com', + 'webbyawards.com', + ] var currentUrl = window.location.href var currentRegex for (let i = 0; i < blacklistedDomains.length; i++) { diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 31e53554d..a73a8b36d 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -185,7 +185,8 @@ module.exports = class TransactionController extends EventEmitter { async addUnapprovedTransaction (txParams) { // validate - await this.txGasUtil.validateTxParams(txParams) + this._validateTxParams(txParams) + this._normalizeTxParams(txParams) // construct txMeta let txMeta = this.txStateManager.generateTxMeta({txParams}) this.addTx(txMeta) @@ -215,7 +216,6 @@ module.exports = class TransactionController extends EventEmitter { } txParams.gasPrice = ethUtil.addHexPrefix(gasPrice.toString(16)) txParams.value = txParams.value || '0x0' - if (txParams.to === null) delete txParams.to // set gasLimit return await this.txGasUtil.analyzeGasUsage(txMeta) } @@ -314,6 +314,61 @@ module.exports = class TransactionController extends EventEmitter { // PRIVATE METHODS // + _normalizeTxParams (txParams) { + delete txParams.chainId + + if ( !txParams.to ) { + delete txParams.to + } else { + txParams.to = ethUtil.addHexPrefix(txParams.to) + } + txParams.from = ethUtil.addHexPrefix(txParams.from).toLowerCase() + + if (!txParams.data) { + delete txParams.data + } else { + txParams.data = ethUtil.addHexPrefix(txParams.data) + } + + if (txParams.value) txParams.value = ethUtil.addHexPrefix(txParams.value) + + if (txParams.gas) txParams.gas = ethUtil.addHexPrefix(txParams.gas) + if (txParams.gasPrice) txParams.gas = ethUtil.addHexPrefix(txParams.gas) + } + + _validateTxParams (txParams) { + this._validateFrom(txParams) + this._validateRecipient(txParams) + if ('value' in txParams) { + const value = txParams.value.toString() + if (value.includes('-')) { + throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) + } + + if (value.includes('.')) { + throw new Error(`Invalid transaction value of ${txParams.value} number must be in wei`) + } + } + } + + _validateFrom (txParams) { + if ( !(typeof txParams.from === 'string') ) throw new Error(`Invalid from address ${txParams.from} not a string`) + if (!ethUtil.isValidAddress(txParams.from)) throw new Error('Invalid from address') + } + + _validateRecipient (txParams) { + if (txParams.to === '0x' || txParams.to === null ) { + if (txParams.data) { + delete txParams.to + } else { + throw new Error('Invalid recipient address') + } + } else if ( txParams.to !== undefined && !ethUtil.isValidAddress(txParams.to) ) { + throw new Error('Invalid recipient address') + } + return txParams + } + _markNonceDuplicatesDropped (txId) { this.txStateManager.setTxStatusConfirmed(txId) // get the confirmed transactions nonce and from address diff --git a/app/scripts/lib/tx-gas-utils.js b/app/scripts/lib/tx-gas-utils.js index 829b4c421..c579e462a 100644 --- a/app/scripts/lib/tx-gas-utils.js +++ b/app/scripts/lib/tx-gas-utils.js @@ -4,7 +4,7 @@ const { BnMultiplyByFraction, bnToHex, } = require('./util') -const { addHexPrefix, isValidAddress } = require('ethereumjs-util') +const { addHexPrefix } = require('ethereumjs-util') const SIMPLE_GAS_COST = '0x5208' // Hex for 21000, cost of a simple send. /* @@ -100,37 +100,4 @@ module.exports = class TxGasUtil { // otherwise use blockGasLimit return bnToHex(upperGasLimitBn) } - - async validateTxParams (txParams) { - this.validateFrom(txParams) - this.validateRecipient(txParams) - if ('value' in txParams) { - const value = txParams.value.toString() - if (value.includes('-')) { - throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`) - } - - if (value.includes('.')) { - throw new Error(`Invalid transaction value of ${txParams.value} number must be in wei`) - } - } - } - - validateFrom (txParams) { - if ( !(typeof txParams.from === 'string') ) throw new Error(`Invalid from address ${txParams.from} not a string`) - if (!isValidAddress(txParams.from)) throw new Error('Invalid from address') - } - - validateRecipient (txParams) { - if (txParams.to === '0x' || txParams.to === null ) { - if (txParams.data) { - delete txParams.to - } else { - throw new Error('Invalid recipient address') - } - } else if ( txParams.to !== undefined && !isValidAddress(txParams.to) ) { - throw new Error('Invalid recipient address') - } - return txParams - } }
\ No newline at end of file diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index 9e597ef37..2ab24d6a0 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -143,7 +143,7 @@ module.exports = class TransactionStateManager extends EventEmitter { // validate types switch (key) { case 'chainId': - if (typeof value !== 'number') throw new Error(`${key} in txParams is not a Number. got: (${value})`) + if (typeof value !== 'number' && typeof value !== 'string') throw new Error(`${key} in txParams is not a Number or hex string. got: (${value})`) break default: if (typeof value !== 'string') throw new Error(`${key} in txParams is not a string. got: (${value})`) diff --git a/app/scripts/migrations/024.js b/app/scripts/migrations/024.js new file mode 100644 index 000000000..7a0391805 --- /dev/null +++ b/app/scripts/migrations/024.js @@ -0,0 +1,45 @@ + +const version = 24 + +/* + +This migration ensures that the from address in txParams is to lower case for +all unapproved transactions + +*/ + +const clone = require('clone') + +module.exports = { + version, + + migrate: function (originalVersionedData) { + const versionedData = clone(originalVersionedData) + versionedData.meta.version = version + try { + const state = versionedData.data + const newState = transformState(state) + versionedData.data = newState + } catch (err) { + console.warn(`MetaMask Migration #${version}` + err.stack) + } + return Promise.resolve(versionedData) + }, +} + +function transformState (state) { + const newState = state + const transactions = newState.TransactionController.transactions + + newState.TransactionController.transactions = transactions.map((txMeta, _, txList) => { + if ( + txMeta.status === 'unapproved' && + txMeta.txParams && + txMeta.txParams.from + ) { + txMeta.txParams.from = txMeta.txParams.from.toLowerCase() + } + return txMeta + }) + return newState +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 811e06b6b..7e4542740 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -34,4 +34,5 @@ module.exports = [ require('./021'), require('./022'), require('./023'), + require('./024'), ] diff --git a/test/unit/migrations/024-test.js b/test/unit/migrations/024-test.js new file mode 100644 index 000000000..dab77d4e4 --- /dev/null +++ b/test/unit/migrations/024-test.js @@ -0,0 +1,37 @@ +const assert = require('assert') +const migration24 = require('../../../app/scripts/migrations/024') +const properTime = (new Date()).getTime() +const storage = { + "meta": {}, + "data": { + "TransactionController": { + "transactions": [ + ] + }, + }, +} + +const transactions = [] + + +while (transactions.length <= 10) { + transactions.push({ txParams: { from: '0x8aCce2391c0d510a6c5E5d8f819a678f79b7e675' }, status: 'unapproved' }) + transactions.push({ txParams: { from: '0x8aCce2391c0d510a6c5E5d8f819a678f79b7e675' }, status: 'confirmed' }) +} + + +storage.data.TransactionController.transactions = transactions + +describe('storage is migrated successfully and the txParams.from are lowercase', () => { + it('should lowercase the from for unapproved txs', (done) => { + migration24.migrate(storage) + .then((migratedData) => { + const migratedTransactions = migratedData.data.TransactionController.transactions + migratedTransactions.forEach((tx) => { + if (tx.status === 'unapproved') assert.equal(tx.txParams.from, '0x8acce2391c0d510a6c5e5d8f819a678f79b7e675') + else assert.equal(tx.txParams.from, '0x8aCce2391c0d510a6c5E5d8f819a678f79b7e675') + }) + done() + }).catch(done) + }) +}) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index 6bd010e7a..3fec9758f 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -210,29 +210,96 @@ describe('Transaction Controller', function () { }) }) - describe('#validateTxParams', function () { - it('does not throw for positive values', function (done) { + describe('#_validateTxParams', function () { + it('does not throw for positive values', function () { var sample = { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', value: '0x01', } - txController.txGasUtil.validateTxParams(sample).then(() => { - done() - }).catch(done) + txController._validateTxParams(sample) }) - it('returns error for negative values', function (done) { + it('returns error for negative values', function () { var sample = { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', value: '-0x01', } - txController.txGasUtil.validateTxParams(sample) - .then(() => done('expected to thrown on negativity values but didn\'t')) - .catch((err) => { + try { + txController._validateTxParams(sample) + } catch (err) { assert.ok(err, 'error') - done() - }) + } + }) + }) + + describe('#_normalizeTxParams', () => { + it('should normalize txParams', () => { + let txParams = { + chainId: '0x1', + from: 'a7df1beDBF813f57096dF77FCd515f0B3900e402', + to: null, + data: '68656c6c6f20776f726c64', + } + + txController._normalizeTxParams(txParams) + + assert(!txParams.chainId, 'their should be no chainId') + assert(!txParams.to, 'their should be no to address if null') + assert.equal(txParams.from.slice(0, 2), '0x', 'from should be hexPrefixd') + assert.equal(txParams.data.slice(0, 2), '0x', 'data should be hexPrefixd') + + txParams.to = 'a7df1beDBF813f57096dF77FCd515f0B3900e402' + + txController._normalizeTxParams(txParams) + assert.equal(txParams.to.slice(0, 2), '0x', 'to should be hexPrefixd') + + }) + }) + + describe('#_validateRecipient', () => { + it('removes recipient for txParams with 0x when contract data is provided', function () { + const zeroRecipientandDataTxParams = { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + to: '0x', + data: 'bytecode', + } + const sanitizedTxParams = txController._validateRecipient(zeroRecipientandDataTxParams) + assert.deepEqual(sanitizedTxParams, { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', data: 'bytecode' }, 'no recipient with 0x') }) + + it('should error when recipient is 0x', function () { + const zeroRecipientTxParams = { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + to: '0x', + } + assert.throws(() => { txController._validateRecipient(zeroRecipientTxParams) }, Error, 'Invalid recipient address') + }) + }) + + + describe('#_validateFrom', () => { + it('should error when from is not a hex string', function () { + + // where from is undefined + const txParams = {} + assert.throws(() => { txController._validateFrom(txParams) }, Error, `Invalid from address ${txParams.from} not a string`) + + // where from is array + txParams.from = [] + assert.throws(() => { txController._validateFrom(txParams) }, Error, `Invalid from address ${txParams.from} not a string`) + + // where from is a object + txParams.from = {} + assert.throws(() => { txController._validateFrom(txParams) }, Error, `Invalid from address ${txParams.from} not a string`) + + // where from is a invalid address + txParams.from = 'im going to fail' + assert.throws(() => { txController._validateFrom(txParams) }, Error, `Invalid from address`) + + // should run + txParams.from ='0x1678a085c290ebd122dc42cba69373b5953b831d' + txController._validateFrom(txParams) + }) }) describe('#addTx', function () { diff --git a/test/unit/tx-gas-util-test.js b/test/unit/tx-gas-util-test.js index 15d412c72..40ea8a7d6 100644 --- a/test/unit/tx-gas-util-test.js +++ b/test/unit/tx-gas-util-test.js @@ -11,46 +11,4 @@ describe('Tx Gas Util', function () { provider, }) }) - - it('removes recipient for txParams with 0x when contract data is provided', function () { - const zeroRecipientandDataTxParams = { - from: '0x1678a085c290ebd122dc42cba69373b5953b831d', - to: '0x', - data: 'bytecode', - } - const sanitizedTxParams = txGasUtil.validateRecipient(zeroRecipientandDataTxParams) - assert.deepEqual(sanitizedTxParams, { from: '0x1678a085c290ebd122dc42cba69373b5953b831d', data: 'bytecode' }, 'no recipient with 0x') - }) - - it('should error when recipient is 0x', function () { - const zeroRecipientTxParams = { - from: '0x1678a085c290ebd122dc42cba69373b5953b831d', - to: '0x', - } - assert.throws(() => { txGasUtil.validateRecipient(zeroRecipientTxParams) }, Error, 'Invalid recipient address') - }) - - it('should error when from is not a hex string', function () { - - // where from is undefined - const txParams = {} - assert.throws(() => { txGasUtil.validateFrom(txParams) }, Error, `Invalid from address ${txParams.from} not a string`) - - // where from is array - txParams.from = [] - assert.throws(() => { txGasUtil.validateFrom(txParams) }, Error, `Invalid from address ${txParams.from} not a string`) - - // where from is a object - txParams.from = {} - assert.throws(() => { txGasUtil.validateFrom(txParams) }, Error, `Invalid from address ${txParams.from} not a string`) - - // where from is a invalid address - txParams.from = 'im going to fail' - assert.throws(() => { txGasUtil.validateFrom(txParams) }, Error, `Invalid from address`) - - // should run - txParams.from ='0x1678a085c290ebd122dc42cba69373b5953b831d' - txGasUtil.validateFrom(txParams) - }) - }) diff --git a/ui/app/components/ens-input.js b/ui/app/components/ens-input.js index 1f3946817..feb0a7037 100644 --- a/ui/app/components/ens-input.js +++ b/ui/app/components/ens-input.js @@ -32,10 +32,10 @@ EnsInput.prototype.render = function () { const network = this.props.network const networkHasEnsSupport = getNetworkEnsSupport(network) - if (!networkHasEnsSupport) return - props.onChange(recipient) + if (!networkHasEnsSupport) return + if (recipient.match(ensRE) === null) { return this.setState({ loadingEns: false, diff --git a/ui/app/components/pending-tx/confirm-send-ether.js b/ui/app/components/pending-tx/confirm-send-ether.js index d007e6661..7bf20bced 100644 --- a/ui/app/components/pending-tx/confirm-send-ether.js +++ b/ui/app/components/pending-tx/confirm-send-ether.js @@ -237,6 +237,7 @@ ConfirmSendEther.prototype.getData = function () { const { identities } = this.props const txMeta = this.gatherTxMeta() const txParams = txMeta.txParams || {} + const account = identities ? identities[txParams.from] || {} : {} const { FIAT: gasFeeInFIAT, ETH: gasFeeInETH, gasFeeInHex } = this.getGasFee() const { FIAT: amountInFIAT, ETH: amountInETH } = this.getAmount() @@ -252,7 +253,7 @@ ConfirmSendEther.prototype.getData = function () { return { from: { address: txParams.from, - name: identities[txParams.from].name, + name: account.name, }, to: { address: txParams.to, |