From c38691a651c1ada04b74eb9c98b8648a84f53939 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 23 Aug 2022 17:23:04 -0500 Subject: [PATCH 1/7] replace bip39 implementation with @metamask/scure-bip39 --- index.js | 76 +++++++++++++++++++++++++++++----------------------- package.json | 2 +- yarn.lock | 25 +++-------------- 3 files changed, 47 insertions(+), 56 deletions(-) diff --git a/index.js b/index.js index 12f29e39..7b109086 100644 --- a/index.js +++ b/index.js @@ -1,6 +1,7 @@ const { EventEmitter } = require('events'); const { Buffer } = require('buffer'); -const bip39 = require('@metamask/bip39'); +const bip39 = require('@metamask/scure-bip39'); +const { wordlist } = require('@metamask/scure-bip39/dist/wordlists/english'); const ObservableStore = require('obs-store'); const encryptor = require('@metamask/browser-passworder'); const { normalize: normalizeAddress } = require('@metamask/eth-sig-util'); @@ -105,47 +106,56 @@ class KeyringController extends EventEmitter { * creates a new encrypted store with the given password, * creates a new HD wallet from the given seed with 1 account. * - * @fires KeyringController#unlock - * @param {string} password - The password to encrypt the vault with. - * @param {string|Array} seedPhrase - The BIP39-compliant seed phrase, - * either as a string or an array of UTF-8 bytes that represent the string. - * @returns {Promise} A Promise that resolves to the state. - */ - async createNewVaultAndRestore(password, seedPhrase) { - const seedPhraseAsBuffer = - typeof seedPhrase === 'string' - ? Buffer.from(seedPhrase, 'utf8') - : Buffer.from(seedPhrase); + * @emits KeyringController#unlock + * @param {string} password - The password to encrypt the vault with + * @param {Uint8Array | string} seedPhrase - The BIP39-compliant seed phrase, + * either as a string or Uint8Array. + * @returns {Promise} A Promise that resolves to the state. + */ + createNewVaultAndRestore(password, seedPhrase) { + let encodedSeedPhrase = seedPhrase; + if (typeof encodedSeedPhrase === 'string') { + const indices = seedPhrase + .split(' ') + .map((word) => wordlist.indexOf(word)); + encodedSeedPhrase = new Uint8Array(new Uint16Array(indices).buffer); + } else if ( + seedPhrase instanceof Object && + !(seedPhrase instanceof Uint8Array) + ) { + // when passed from the frontend to the background process a Uint8Array becomes a javascript object + encodedSeedPhrase = Uint8Array.from(Object.values(seedPhrase)); + } if (typeof password !== 'string') { throw new Error('Password must be text.'); } - const wordlists = Object.values(bip39.wordlists); - if ( - wordlists.every( - (wordlist) => !bip39.validateMnemonic(seedPhraseAsBuffer, wordlist), - ) - ) { - throw new Error('Seed phrase is invalid.'); + if (!bip39.validateMnemonic(encodedSeedPhrase, wordlist)) { + return Promise.reject(new Error('Seed phrase is invalid.')); } this.password = password; - this.clearKeyrings(); - const firstKeyring = await this.addNewKeyring( - KEYRINGS_TYPE_MAP.HD_KEYRING, - { - mnemonic: seedPhraseAsBuffer, - numberOfAccounts: 1, - }, - ); - const [firstAccount] = await firstKeyring.getAccounts(); - if (!firstAccount) { - throw new Error('KeyringController - First Account not found.'); - } - this.setUnlocked(); - return this.fullUpdate(); + return this.persistAllKeyrings(password) + .then(() => { + return this.addNewKeyring(KEYRINGS_TYPE_MAP.HD_KEYRING, { + mnemonic: encodedSeedPhrase, + numberOfAccounts: 1, + }); + }) + .then((firstKeyring) => { + return firstKeyring.getAccounts(); + }) + .then(([firstAccount]) => { + if (!firstAccount) { + throw new Error('KeyringController - First Account not found.'); + } + return null; + }) + .then(this.persistAllKeyrings.bind(this, password)) + .then(this.setUnlocked.bind(this)) + .then(this.fullUpdate.bind(this)); } /** diff --git a/package.json b/package.json index cafec1c5..7fa775d6 100644 --- a/package.json +++ b/package.json @@ -30,11 +30,11 @@ "lint:fix": "yarn lint:eslint --fix && yarn lint:misc --write" }, "dependencies": { - "@metamask/bip39": "^4.0.0", "@metamask/browser-passworder": "^4.0.1", "@metamask/eth-hd-keyring": "^5.0.1", "@metamask/eth-sig-util": "5.0.2", "@metamask/eth-simple-keyring": "^5.0.0", + "@metamask/scure-bip39": "^2.0.3", "obs-store": "^4.0.3" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index a466c7fe..53cd0b8b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -758,18 +758,6 @@ __metadata: languageName: node linkType: hard -"@metamask/bip39@npm:^4.0.0": - version: 4.0.0 - resolution: "@metamask/bip39@npm:4.0.0" - dependencies: - "@types/node": 11.11.6 - create-hash: ^1.1.0 - pbkdf2: ^3.0.9 - randombytes: ^2.0.1 - checksum: 0d629de806dd0a6c6ea2ff4ab4752931b15eda5abcf2975f3beed8b4161bcc4765ec97bd8ba0146059b10178f6cf1753f74223655908bc585a76b565f26d4f16 - languageName: node - linkType: hard - "@metamask/browser-passworder@npm:^4.0.1": version: 4.0.2 resolution: "@metamask/browser-passworder@npm:4.0.2" @@ -1158,13 +1146,6 @@ __metadata: languageName: node linkType: hard -"@types/node@npm:11.11.6": - version: 11.11.6 - resolution: "@types/node@npm:11.11.6" - checksum: 075f1c011cf568e49701419acbcb55c24906b3bb5a34d9412a3b88f228a7a78401a5ad4d3e1cd6855c99aaea5ef96e37fc86ca097e50f06da92cf822befc1fff - languageName: node - linkType: hard - "@types/pbkdf2@npm:^3.0.0": version: 3.1.0 resolution: "@types/pbkdf2@npm:3.1.0" @@ -2786,7 +2767,6 @@ __metadata: dependencies: "@lavamoat/allow-scripts": ^2.1.0 "@metamask/auto-changelog": ^3.0.0 - "@metamask/bip39": ^4.0.0 "@metamask/browser-passworder": ^4.0.1 "@metamask/eslint-config": ^10.0.0 "@metamask/eslint-config-jest": ^10.0.0 @@ -2794,6 +2774,7 @@ __metadata: "@metamask/eth-hd-keyring": ^5.0.1 "@metamask/eth-sig-util": 5.0.2 "@metamask/eth-simple-keyring": ^5.0.0 + "@metamask/scure-bip39": ^2.0.3 eslint: ^8.21.0 eslint-config-prettier: ^8.3.0 eslint-plugin-import: ^2.26.0 @@ -5383,7 +5364,7 @@ __metadata: languageName: node linkType: hard -"pbkdf2@npm:^3.0.17, pbkdf2@npm:^3.0.9": +"pbkdf2@npm:^3.0.17": version: 3.1.1 resolution: "pbkdf2@npm:3.1.1" dependencies: @@ -5545,7 +5526,7 @@ __metadata: languageName: node linkType: hard -"randombytes@npm:^2.0.1, randombytes@npm:^2.0.6, randombytes@npm:^2.1.0": +"randombytes@npm:^2.0.6, randombytes@npm:^2.1.0": version: 2.1.0 resolution: "randombytes@npm:2.1.0" dependencies: From 55807bcb6fc0c165a6213d41f9fae3908cbf0866 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 14 Dec 2022 11:26:54 -0600 Subject: [PATCH 2/7] cleanup --- index.js | 71 +++++++++++++++++++++++++++++++++++++++------------ test/index.js | 2 +- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/index.js b/index.js index 7b109086..f4a15ce8 100644 --- a/index.js +++ b/index.js @@ -106,37 +106,28 @@ class KeyringController extends EventEmitter { * creates a new encrypted store with the given password, * creates a new HD wallet from the given seed with 1 account. * - * @emits KeyringController#unlock - * @param {string} password - The password to encrypt the vault with + * @fires KeyringController#unlock + * @param {string} password - The password to encrypt the vault with. * @param {Uint8Array | string} seedPhrase - The BIP39-compliant seed phrase, * either as a string or Uint8Array. - * @returns {Promise} A Promise that resolves to the state. + * @returns {Promise} A Promise that resolves to the state. */ createNewVaultAndRestore(password, seedPhrase) { - let encodedSeedPhrase = seedPhrase; - if (typeof encodedSeedPhrase === 'string') { - const indices = seedPhrase - .split(' ') - .map((word) => wordlist.indexOf(word)); - encodedSeedPhrase = new Uint8Array(new Uint16Array(indices).buffer); - } else if ( - seedPhrase instanceof Object && - !(seedPhrase instanceof Uint8Array) - ) { - // when passed from the frontend to the background process a Uint8Array becomes a javascript object - encodedSeedPhrase = Uint8Array.from(Object.values(seedPhrase)); - } + const encodedSeedPhrase = this._mnemonicToUint8Array(seedPhrase); if (typeof password !== 'string') { throw new Error('Password must be text.'); } if (!bip39.validateMnemonic(encodedSeedPhrase, wordlist)) { - return Promise.reject(new Error('Seed phrase is invalid.')); + return Promise.reject( + new Error('KeyringController - Seed phrase is invalid.'), + ); } this.password = password; + this.clearKeyrings(); return this.persistAllKeyrings(password) .then(() => { return this.addNewKeyring(KEYRINGS_TYPE_MAP.HD_KEYRING, { @@ -924,6 +915,52 @@ class KeyringController extends EventEmitter { return keyring; } + + /* + Utility Methods + */ + + _uint8ArrayToString(mnemonic) { + const recoveredIndices = Array.from( + new Uint16Array(new Uint8Array(mnemonic).buffer), + ); + return recoveredIndices.map((i) => wordlist[i]).join(' '); + } + + _stringToUint8Array(mnemonic) { + const indices = mnemonic.split(' ').map((word) => wordlist.indexOf(word)); + return new Uint8Array(new Uint16Array(indices).buffer); + } + + _mnemonicToUint8Array(mnemonic) { + let mnemonicData = mnemonic; + // when encrypted/decrypted, buffers get cast into js object with a property type set to buffer + if (mnemonic && mnemonic.type && mnemonic.type === 'Buffer') { + mnemonicData = mnemonic.data; + } + + if ( + // this block is for backwards compatibility with vaults that were previously stored as buffers, number arrays or plain text strings + typeof mnemonicData === 'string' || + Buffer.isBuffer(mnemonicData) || + Array.isArray(mnemonicData) + ) { + let mnemonicAsString = mnemonicData; + if (Array.isArray(mnemonicData)) { + mnemonicAsString = Buffer.from(mnemonicData).toString(); + } else if (Buffer.isBuffer(mnemonicData)) { + mnemonicAsString = mnemonicData.toString(); + } + return this._stringToUint8Array(mnemonicAsString); + } else if ( + mnemonicData instanceof Object && + !(mnemonicData instanceof Uint8Array) + ) { + // when encrypted/decrypted the Uint8Array becomes a js object we need to cast back to a Uint8Array + return Uint8Array.from(Object.values(mnemonicData)); + } + return mnemonicData; + } } /** diff --git a/test/index.js b/test/index.js index 4fdb7e37..eb1c3983 100644 --- a/test/index.js +++ b/test/index.js @@ -180,7 +180,7 @@ describe('KeyringController', function () { it('throws error if argument password is not a string', async function () { await expect(() => keyringController.createNewVaultAndRestore(12, walletTwoSeedWords), - ).rejects.toThrow('Password must be text.'); + ).toThrow('Password must be text.'); }); it('throws error if mnemonic passed is invalid', async function () { From 390991bb82313621b60986dbe50a784742948409 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 14 Dec 2022 13:34:31 -0600 Subject: [PATCH 3/7] cleanup --- index.js | 7 ------- test/index.js | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/index.js b/index.js index f4a15ce8..e3f55614 100644 --- a/index.js +++ b/index.js @@ -920,13 +920,6 @@ class KeyringController extends EventEmitter { Utility Methods */ - _uint8ArrayToString(mnemonic) { - const recoveredIndices = Array.from( - new Uint16Array(new Uint8Array(mnemonic).buffer), - ); - return recoveredIndices.map((i) => wordlist[i]).join(' '); - } - _stringToUint8Array(mnemonic) { const indices = mnemonic.split(' ').map((word) => wordlist.indexOf(word)); return new Uint8Array(new Uint16Array(indices).buffer); diff --git a/test/index.js b/test/index.js index eb1c3983..e4fc99b4 100644 --- a/test/index.js +++ b/test/index.js @@ -189,7 +189,7 @@ describe('KeyringController', function () { password, 'test test test palace city barely security section midnight wealth south deer', ), - ).rejects.toThrow('Seed phrase is invalid.'); + ).rejects.toThrow('KeyringController - Seed phrase is invalid.'); }); it('accepts mnemonic passed as type array of numbers', async function () { From 4eb0812d8d4ae2e1360d71e407075d59b5be084f Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 14 Dec 2022 13:42:40 -0600 Subject: [PATCH 4/7] fix up --- index.js | 38 ++++++++++++++------------------------ test/index.js | 2 +- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/index.js b/index.js index e3f55614..2edcb5a0 100644 --- a/index.js +++ b/index.js @@ -112,7 +112,7 @@ class KeyringController extends EventEmitter { * either as a string or Uint8Array. * @returns {Promise} A Promise that resolves to the state. */ - createNewVaultAndRestore(password, seedPhrase) { + async createNewVaultAndRestore(password, seedPhrase) { const encodedSeedPhrase = this._mnemonicToUint8Array(seedPhrase); if (typeof password !== 'string') { @@ -120,33 +120,23 @@ class KeyringController extends EventEmitter { } if (!bip39.validateMnemonic(encodedSeedPhrase, wordlist)) { - return Promise.reject( - new Error('KeyringController - Seed phrase is invalid.'), - ); + throw new Error('KeyringController - Seed phrase is invalid.'); } this.password = password; - this.clearKeyrings(); - return this.persistAllKeyrings(password) - .then(() => { - return this.addNewKeyring(KEYRINGS_TYPE_MAP.HD_KEYRING, { - mnemonic: encodedSeedPhrase, - numberOfAccounts: 1, - }); - }) - .then((firstKeyring) => { - return firstKeyring.getAccounts(); - }) - .then(([firstAccount]) => { - if (!firstAccount) { - throw new Error('KeyringController - First Account not found.'); - } - return null; - }) - .then(this.persistAllKeyrings.bind(this, password)) - .then(this.setUnlocked.bind(this)) - .then(this.fullUpdate.bind(this)); + await this.clearKeyrings(); + const keyring = await this.addNewKeyring(KEYRINGS_TYPE_MAP.HD_KEYRING, { + mnemonic: encodedSeedPhrase, + numberOfAccounts: 1, + }); + const [firstAccount] = await keyring.getAccounts(); + + if (!firstAccount) { + throw new Error('KeyringController - First Account not found.'); + } + this.setUnlocked(); + return this.fullUpdate(); } /** diff --git a/test/index.js b/test/index.js index e4fc99b4..61107ed3 100644 --- a/test/index.js +++ b/test/index.js @@ -180,7 +180,7 @@ describe('KeyringController', function () { it('throws error if argument password is not a string', async function () { await expect(() => keyringController.createNewVaultAndRestore(12, walletTwoSeedWords), - ).toThrow('Password must be text.'); + ).rejects.toThrow('Password must be text.'); }); it('throws error if mnemonic passed is invalid', async function () { From bcdb340505ef00a1749101444480007802bf9631 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 14 Dec 2022 14:08:14 -0600 Subject: [PATCH 5/7] add test for mnemonic passed as number - invalid --- index.js | 7 ++++--- test/index.js | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 2edcb5a0..d8bb5faf 100644 --- a/index.js +++ b/index.js @@ -113,13 +113,13 @@ class KeyringController extends EventEmitter { * @returns {Promise} A Promise that resolves to the state. */ async createNewVaultAndRestore(password, seedPhrase) { - const encodedSeedPhrase = this._mnemonicToUint8Array(seedPhrase); + const seedPhraseUint8Array = this._mnemonicToUint8Array(seedPhrase); if (typeof password !== 'string') { throw new Error('Password must be text.'); } - if (!bip39.validateMnemonic(encodedSeedPhrase, wordlist)) { + if (!bip39.validateMnemonic(seedPhraseUint8Array, wordlist)) { throw new Error('KeyringController - Seed phrase is invalid.'); } @@ -127,7 +127,7 @@ class KeyringController extends EventEmitter { await this.clearKeyrings(); const keyring = await this.addNewKeyring(KEYRINGS_TYPE_MAP.HD_KEYRING, { - mnemonic: encodedSeedPhrase, + mnemonic: seedPhraseUint8Array, numberOfAccounts: 1, }); const [firstAccount] = await keyring.getAccounts(); @@ -942,6 +942,7 @@ class KeyringController extends EventEmitter { // when encrypted/decrypted the Uint8Array becomes a js object we need to cast back to a Uint8Array return Uint8Array.from(Object.values(mnemonicData)); } + return mnemonicData; } } diff --git a/test/index.js b/test/index.js index 61107ed3..03678b58 100644 --- a/test/index.js +++ b/test/index.js @@ -190,6 +190,10 @@ describe('KeyringController', function () { 'test test test palace city barely security section midnight wealth south deer', ), ).rejects.toThrow('KeyringController - Seed phrase is invalid.'); + + await expect(() => + keyringController.createNewVaultAndRestore(password, 1234), + ).rejects.toThrow('KeyringController - Seed phrase is invalid.'); }); it('accepts mnemonic passed as type array of numbers', async function () { From 10f84c009aceaaf695c5331a98eae307107163c2 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 14 Dec 2022 14:12:54 -0600 Subject: [PATCH 6/7] lint --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index d8bb5faf..d8358697 100644 --- a/index.js +++ b/index.js @@ -942,7 +942,7 @@ class KeyringController extends EventEmitter { // when encrypted/decrypted the Uint8Array becomes a js object we need to cast back to a Uint8Array return Uint8Array.from(Object.values(mnemonicData)); } - + return mnemonicData; } } From dea4b70574e4da2d72882801dbe7db0d1e93f1b9 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 14 Dec 2022 15:27:23 -0600 Subject: [PATCH 7/7] remove bip39 dependence altogether --- index.js | 52 +-------------------------------------------------- package.json | 1 - test/index.js | 8 ++++++-- yarn.lock | 1 - 4 files changed, 7 insertions(+), 55 deletions(-) diff --git a/index.js b/index.js index d8358697..35c87de4 100644 --- a/index.js +++ b/index.js @@ -1,7 +1,4 @@ const { EventEmitter } = require('events'); -const { Buffer } = require('buffer'); -const bip39 = require('@metamask/scure-bip39'); -const { wordlist } = require('@metamask/scure-bip39/dist/wordlists/english'); const ObservableStore = require('obs-store'); const encryptor = require('@metamask/browser-passworder'); const { normalize: normalizeAddress } = require('@metamask/eth-sig-util'); @@ -113,21 +110,14 @@ class KeyringController extends EventEmitter { * @returns {Promise} A Promise that resolves to the state. */ async createNewVaultAndRestore(password, seedPhrase) { - const seedPhraseUint8Array = this._mnemonicToUint8Array(seedPhrase); - if (typeof password !== 'string') { throw new Error('Password must be text.'); } - - if (!bip39.validateMnemonic(seedPhraseUint8Array, wordlist)) { - throw new Error('KeyringController - Seed phrase is invalid.'); - } - this.password = password; await this.clearKeyrings(); const keyring = await this.addNewKeyring(KEYRINGS_TYPE_MAP.HD_KEYRING, { - mnemonic: seedPhraseUint8Array, + mnemonic: seedPhrase, numberOfAccounts: 1, }); const [firstAccount] = await keyring.getAccounts(); @@ -905,46 +895,6 @@ class KeyringController extends EventEmitter { return keyring; } - - /* - Utility Methods - */ - - _stringToUint8Array(mnemonic) { - const indices = mnemonic.split(' ').map((word) => wordlist.indexOf(word)); - return new Uint8Array(new Uint16Array(indices).buffer); - } - - _mnemonicToUint8Array(mnemonic) { - let mnemonicData = mnemonic; - // when encrypted/decrypted, buffers get cast into js object with a property type set to buffer - if (mnemonic && mnemonic.type && mnemonic.type === 'Buffer') { - mnemonicData = mnemonic.data; - } - - if ( - // this block is for backwards compatibility with vaults that were previously stored as buffers, number arrays or plain text strings - typeof mnemonicData === 'string' || - Buffer.isBuffer(mnemonicData) || - Array.isArray(mnemonicData) - ) { - let mnemonicAsString = mnemonicData; - if (Array.isArray(mnemonicData)) { - mnemonicAsString = Buffer.from(mnemonicData).toString(); - } else if (Buffer.isBuffer(mnemonicData)) { - mnemonicAsString = mnemonicData.toString(); - } - return this._stringToUint8Array(mnemonicAsString); - } else if ( - mnemonicData instanceof Object && - !(mnemonicData instanceof Uint8Array) - ) { - // when encrypted/decrypted the Uint8Array becomes a js object we need to cast back to a Uint8Array - return Uint8Array.from(Object.values(mnemonicData)); - } - - return mnemonicData; - } } /** diff --git a/package.json b/package.json index 7fa775d6..b03ef9bd 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,6 @@ "@metamask/eth-hd-keyring": "^5.0.1", "@metamask/eth-sig-util": "5.0.2", "@metamask/eth-simple-keyring": "^5.0.0", - "@metamask/scure-bip39": "^2.0.3", "obs-store": "^4.0.3" }, "devDependencies": { diff --git a/test/index.js b/test/index.js index 03678b58..30e8c3f4 100644 --- a/test/index.js +++ b/test/index.js @@ -189,11 +189,15 @@ describe('KeyringController', function () { password, 'test test test palace city barely security section midnight wealth south deer', ), - ).rejects.toThrow('KeyringController - Seed phrase is invalid.'); + ).rejects.toThrow( + 'Eth-Hd-Keyring: Invalid secret recovery phrase provided', + ); await expect(() => keyringController.createNewVaultAndRestore(password, 1234), - ).rejects.toThrow('KeyringController - Seed phrase is invalid.'); + ).rejects.toThrow( + 'Eth-Hd-Keyring: Invalid secret recovery phrase provided', + ); }); it('accepts mnemonic passed as type array of numbers', async function () { diff --git a/yarn.lock b/yarn.lock index 53cd0b8b..a99eb022 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2774,7 +2774,6 @@ __metadata: "@metamask/eth-hd-keyring": ^5.0.1 "@metamask/eth-sig-util": 5.0.2 "@metamask/eth-simple-keyring": ^5.0.0 - "@metamask/scure-bip39": ^2.0.3 eslint: ^8.21.0 eslint-config-prettier: ^8.3.0 eslint-plugin-import: ^2.26.0