From b9372ba50a135507ef6fc956a9a0f7a4867d9166 Mon Sep 17 00:00:00 2001 From: Brad Decker Date: Fri, 21 May 2021 15:07:06 -0500 Subject: [PATCH 1/8] implement safer to buffer (#11159) --- shared/modules/buffer-utils.js | 16 +++++ shared/modules/buffer-utils.test.js | 69 +++++++++++++++++++ .../confirm-deploy-contract.component.js | 2 +- .../confirm-transaction-base.component.js | 3 +- 4 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 shared/modules/buffer-utils.js create mode 100644 shared/modules/buffer-utils.test.js diff --git a/shared/modules/buffer-utils.js b/shared/modules/buffer-utils.js new file mode 100644 index 000000000..38a10b094 --- /dev/null +++ b/shared/modules/buffer-utils.js @@ -0,0 +1,16 @@ +import { toBuffer as ethUtilToBuffer, isHexString } from 'ethereumjs-util'; + +/** + * Returns a buffer from the provided input, via ethereumjs-util.toBuffer but + * additionally handling non hex strings. This is a failsafe as in most cases + * we should be primarily dealing with hex prefixed strings with this utility + * but we do not want to break the extension for users. + * @param {import('ethereumjs-util').ToBufferInputTypes | string} input + * @returns {Buffer} + */ +export function toBuffer(input) { + if (typeof input === 'string' && isHexString(input) === false) { + return Buffer.from(input); + } + return ethUtilToBuffer(input); +} diff --git a/shared/modules/buffer-utils.test.js b/shared/modules/buffer-utils.test.js new file mode 100644 index 000000000..2f5290cca --- /dev/null +++ b/shared/modules/buffer-utils.test.js @@ -0,0 +1,69 @@ +import { strict as assert } from 'assert'; +import BN from 'bn.js'; +import { toBuffer } from './buffer-utils'; + +describe('buffer utils', function () { + describe('toBuffer', function () { + it('should work with prefixed hex strings', function () { + const result = toBuffer('0xe'); + assert.equal(result.length, 1); + }); + + it('should work with non prefixed hex strings', function () { + const result = toBuffer('e'); + assert.equal(result.length, 1); + }); + + it('should work with weirdly 0x prefixed non-hex strings', function () { + const result = toBuffer('0xtest'); + assert.equal(result.length, 6); + }); + + it('should work with regular strings', function () { + const result = toBuffer('test'); + assert.equal(result.length, 4); + }); + + it('should work with BN', function () { + const result = toBuffer(new BN(100)); + assert.equal(result.length, 1); + }); + + it('should work with Buffer', function () { + const result = toBuffer(Buffer.from('test')); + assert.equal(result.length, 4); + }); + + it('should work with a number', function () { + const result = toBuffer(100); + assert.equal(result.length, 1); + }); + + it('should work with null or undefined', function () { + const result = toBuffer(null); + const result2 = toBuffer(undefined); + assert.equal(result.length, 0); + assert.equal(result2.length, 0); + }); + + it('should work with UInt8Array', function () { + const uint8 = new Uint8Array(2); + const result = toBuffer(uint8); + assert.equal(result.length, 2); + }); + + it('should work with objects that have a toBuffer property', function () { + const result = toBuffer({ + toBuffer: () => Buffer.from('hi'), + }); + assert.equal(result.length, 2); + }); + + it('should work with objects that have a toArray property', function () { + const result = toBuffer({ + toArray: () => ['hi'], + }); + assert.equal(result.length, 1); + }); + }); +}); diff --git a/ui/app/pages/confirm-deploy-contract/confirm-deploy-contract.component.js b/ui/app/pages/confirm-deploy-contract/confirm-deploy-contract.component.js index 0c37ca083..63026bfaa 100644 --- a/ui/app/pages/confirm-deploy-contract/confirm-deploy-contract.component.js +++ b/ui/app/pages/confirm-deploy-contract/confirm-deploy-contract.component.js @@ -1,7 +1,7 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; -import { toBuffer } from 'ethereumjs-util'; import ConfirmTransactionBase from '../confirm-transaction-base'; +import { toBuffer } from '../../../shared/modules/buffer-utils'; export default class ConfirmDeployContract extends Component { static contextTypes = { diff --git a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js index 54646ac64..39f45fbaf 100644 --- a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -1,4 +1,3 @@ -import { toBuffer } from 'ethereumjs-util'; import React, { Component } from 'react'; import PropTypes from 'prop-types'; import { ENVIRONMENT_TYPE_NOTIFICATION } from '../../../../shared/constants/app'; @@ -23,6 +22,8 @@ import { TRANSACTION_STATUSES, } from '../../../../shared/constants/transaction'; import { getTransactionTypeTitle } from '../../helpers/utils/transactions.util'; +import ErrorMessage from '../../components/ui/error-message'; +import { toBuffer } from '../../../../shared/modules/buffer-utils'; export default class ConfirmTransactionBase extends Component { static contextTypes = { From a137cd1acae4474f1dd16e207649b57554001066 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 24 May 2021 16:32:26 -0230 Subject: [PATCH 2/8] Add event property to event sent on tx:status-update in metamask-controller (#11165) --- app/scripts/metamask-controller.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 93faa0b48..f255f5c4b 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -347,6 +347,7 @@ export default class MetamaskController extends EventEmitter { if (txReceipt && txReceipt.status === '0x0') { this.metaMetricsController.trackEvent( { + event: 'Tx Status Update: On-Chain Failure', category: 'Background', properties: { action: 'Transactions', From 452a5e7de631f755fac4e5486a3714f7c2fc38b4 Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 24 May 2021 16:57:07 -0230 Subject: [PATCH 3/8] Add stringified payload to metametrics controller trackEvent error message (#11166) * Add stringified payload to metametrics controller trackEvent error message * Don't include full payload when throwing error on missing event or category in trackEvent --- app/scripts/controllers/metametrics.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/metametrics.js b/app/scripts/controllers/metametrics.js index 3d0b622ca..c55a97c85 100644 --- a/app/scripts/controllers/metametrics.js +++ b/app/scripts/controllers/metametrics.js @@ -330,7 +330,17 @@ export default class MetaMetricsController { async trackEvent(payload, options) { // event and category are required fields for all payloads if (!payload.event || !payload.category) { - throw new Error('Must specify event and category.'); + throw new Error( + `Must specify event and category. Event was: ${ + payload.event + }. Category was: ${payload.category}. Payload keys were: ${Object.keys( + payload, + )}. ${ + typeof payload.properties === 'object' + ? `Payload property keys were: ${Object.keys(payload.properties)}` + : '' + }`, + ); } if (!this.state.participateInMetaMetrics && !options?.isOptIn) { From 143a47ed0926cb236eed078276533e5817c2de38 Mon Sep 17 00:00:00 2001 From: MetaMask Bot Date: Mon, 24 May 2021 22:21:13 +0000 Subject: [PATCH 4/8] Version v9.5.5 --- CHANGELOG.md | 9 ++++++++- app/manifest/_base.json | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b5d916a3..b8aa1ca54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [9.5.5] +### Uncategorized +- [#11166](https://github.com/MetaMask/metamask-extension/pull/11166): Add stringified payload to metametrics controller trackEvent error message +- [#11165](https://github.com/MetaMask/metamask-extension/pull/11165): Add event property to event sent on tx:status-update in metamask-controller +- [#11159](https://github.com/MetaMask/metamask-extension/pull/11159): implement safer to buffer + ## [9.5.4] ### Fixed - [#11153](https://github.com/MetaMask/metamask-extension/pull/11153): Prevent UI crash when the transaction being retried or canceled is missing. @@ -2239,7 +2245,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Uncategorized - Added the ability to restore accounts from seed words. -[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v9.5.4...HEAD +[Unreleased]: https://github.com/MetaMask/metamask-extension/compare/v9.5.5...HEAD +[9.5.5]: https://github.com/MetaMask/metamask-extension/compare/v9.5.4...v9.5.5 [9.5.4]: https://github.com/MetaMask/metamask-extension/compare/v9.5.3...v9.5.4 [9.5.3]: https://github.com/MetaMask/metamask-extension/compare/v9.5.2...v9.5.3 [9.5.2]: https://github.com/MetaMask/metamask-extension/compare/v9.5.1...v9.5.2 diff --git a/app/manifest/_base.json b/app/manifest/_base.json index 47cc6a5f9..905172d78 100644 --- a/app/manifest/_base.json +++ b/app/manifest/_base.json @@ -71,6 +71,6 @@ "notifications" ], "short_name": "__MSG_appName__", - "version": "9.5.4", + "version": "9.5.5", "web_accessible_resources": ["inpage.js", "phishing.html"] } From 28315d64f5a8bbe4d34f077f9d59c6ea31a88d39 Mon Sep 17 00:00:00 2001 From: ryanml Date: Mon, 24 May 2021 16:07:06 -0700 Subject: [PATCH 5/8] Upgrading dns-packet to ^5.2.2 to resolve vulnerability (#11172) --- package.json | 1 + yarn.lock | 9 ++++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 050aa01a5..ead85680f 100644 --- a/package.json +++ b/package.json @@ -75,6 +75,7 @@ "**/xmlhttprequest-ssl": "^1.6.2", "3box/ipfs/ipld-zcash/zcash-bitcore-lib/lodash": "^4.17.21", "3box/ipfs/ipld-zcash/zcash-bitcore-lib/elliptic": "^6.5.4", + "3box/ipfs/libp2p-mdns/multicast-dns/dns-packet": "^5.2.2", "3box/**/libp2p-crypto/node-forge": "^0.10.0", "3box/**/libp2p-keychain/node-forge": "^0.10.0", "analytics-node/axios": "^0.21.1", diff --git a/yarn.lock b/yarn.lock index 4b05e3819..c40f5432a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9267,13 +9267,12 @@ dnd-core@^7.4.4: invariant "^2.2.4" redux "^4.0.1" -dns-packet@^4.0.0: - version "4.2.0" - resolved "https://registry.yarnpkg.com/dns-packet/-/dns-packet-4.2.0.tgz#3fd6f5ff5a4ec3194ed0b15312693ffe8776b343" - integrity sha512-bn1AKpfkFbm0MIioOMHZ5qJzl2uypdBwI4nYNsqvhjsegBhcKJUlCrMPWLx6JEezRjxZmxhtIz/FkBEur2l8Cw== +dns-packet@^4.0.0, dns-packet@^5.2.2: + version "5.2.2" + resolved "https://registry.yarnpkg.com/dns-packet/-/dns-packet-5.2.2.tgz#e4c7d12974cc320b0c0d4b9bbbf68ac151cfe81e" + integrity sha512-sQN+vLwC3PvOXiCH/oHcdzML2opFeIdVh8gjjMZrM45n4dR80QF6o3AzInQy6F9Eoc0VJYog4JpQTilt4RFLYQ== dependencies: ip "^1.1.5" - safe-buffer "^5.1.1" doctrine@1.5.0: version "1.5.0" From a79c6c4bbb4547cfad8a1885ce4846f874797823 Mon Sep 17 00:00:00 2001 From: ryanml Date: Mon, 24 May 2021 16:20:36 -0700 Subject: [PATCH 6/8] [skip e2e] Update changelog for v9.5.5 (#11173) --- CHANGELOG.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8aa1ca54..f82255c64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ## [9.5.5] -### Uncategorized -- [#11166](https://github.com/MetaMask/metamask-extension/pull/11166): Add stringified payload to metametrics controller trackEvent error message -- [#11165](https://github.com/MetaMask/metamask-extension/pull/11165): Add event property to event sent on tx:status-update in metamask-controller -- [#11159](https://github.com/MetaMask/metamask-extension/pull/11159): implement safer to buffer +### Fixed +- [#11159](https://github.com/MetaMask/metamask-extension/pull/11159): Fixes crash after entering invalid data in to the Hex Data field when sending a transaction ## [9.5.4] ### Fixed From 5364e417a3c4a6f4e299f95f3a53e86ed9b2bf9b Mon Sep 17 00:00:00 2001 From: ryanml Date: Mon, 24 May 2021 16:28:18 -0700 Subject: [PATCH 7/8] lint fix --- .../confirm-deploy-contract.component.js | 2 +- .../confirm-transaction-base.component.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/app/pages/confirm-deploy-contract/confirm-deploy-contract.component.js b/ui/app/pages/confirm-deploy-contract/confirm-deploy-contract.component.js index 63026bfaa..231cd484a 100644 --- a/ui/app/pages/confirm-deploy-contract/confirm-deploy-contract.component.js +++ b/ui/app/pages/confirm-deploy-contract/confirm-deploy-contract.component.js @@ -1,7 +1,7 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import ConfirmTransactionBase from '../confirm-transaction-base'; -import { toBuffer } from '../../../shared/modules/buffer-utils'; +import { toBuffer } from '../../../../shared/modules/buffer-utils'; export default class ConfirmDeployContract extends Component { static contextTypes = { diff --git a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js index 39f45fbaf..e20efc83a 100644 --- a/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -22,7 +22,6 @@ import { TRANSACTION_STATUSES, } from '../../../../shared/constants/transaction'; import { getTransactionTypeTitle } from '../../helpers/utils/transactions.util'; -import ErrorMessage from '../../components/ui/error-message'; import { toBuffer } from '../../../../shared/modules/buffer-utils'; export default class ConfirmTransactionBase extends Component { From f85697365e00a88b7f908b004a47182eaf7c0256 Mon Sep 17 00:00:00 2001 From: ryanml Date: Tue, 25 May 2021 12:14:58 -0700 Subject: [PATCH 8/8] Bumping package.json version -> 9.5.5 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 294f3025d..5d16ea1bb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "metamask-crx", - "version": "9.5.4", + "version": "9.5.5", "private": true, "repository": { "type": "git",