fix: enforce mnemonic validation

packages/keyring-eth-hd/CHANGELOG.md

@@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add `HdKeyringV2` class implementing `KeyringV2` interface ([#398](https://github.com/MetaMask/accounts/pull/398)), ([#404](https://github.com/MetaMask/accounts/pull/404)), ([#410](https://github.com/MetaMask/accounts/pull/410)), ([#413](https://github.com/MetaMask/accounts/pull/413))
   - Wraps legacy `HdKeyring` to expose accounts via the unified `KeyringV2` API and the `KeyringAccount` type.
   - Extends `EthKeyringWrapper` for common Ethereum logic.
+- Add mnemonic validation using `validateMnemonic` from `@metamask/scure-bip39`
+  - Validates mnemonics against BIP39 specification (word count, wordlist, checksum) before use.
+  - Throws `'Eth-Hd-Keyring: Invalid secret recovery phrase provided'` for invalid mnemonics.
 
 ## [13.0.0]
 

packages/keyring-eth-hd/package.json

@@ -77,7 +77,9 @@
     "allowScripts": {
       "@lavamoat/preinstall-always-fail": false,
       "@metamask/eth-hd-keyring>ethereumjs-util>ethereum-cryptography>keccak": false,
-      "@metamask/eth-hd-keyring>ethereumjs-util>ethereum-cryptography>secp256k1": false
+      "@metamask/eth-hd-keyring>ethereumjs-util>ethereum-cryptography>secp256k1": false,
+      "old-hd-keyring>ethereumjs-util>ethereum-cryptography>keccak": false,
+      "old-hd-keyring>ethereumjs-util>ethereum-cryptography>secp256k1": false
     }
   }
 }

packages/keyring-eth-hd/src/hd-keyring.test.ts

@@ -250,7 +250,7 @@ describe('hd-keyring', () => {
       expect(cryptographicFunctions.pbkdf2Sha512).toHaveBeenCalledTimes(1);
     });
 
-    it('throws on invalid mnemonic', async () => {
+    it('throws on invalid mnemonic with wrong number of words', async () => {
       const keyring = new HdKeyring();
 
       await expect(
@@ -259,10 +259,87 @@ describe('hd-keyring', () => {
           numberOfAccounts: 2,
         }),
       ).rejects.toThrow(
-        'Invalid mnemonic phrase: The mnemonic phrase must consist of 12, 15, 18, 21, or 24 words.',
+        'Eth-Hd-Keyring: Invalid secret recovery phrase provided',
       );
     });
 
+    it('throws on mnemonic with invalid words', async () => {
+      const keyring = new HdKeyring();
+
+      // 12 words but invalid (not in BIP39 wordlist)
+      await expect(
+        keyring.deserialize({
+          mnemonic:
+            'invalid words that are not in the bip39 wordlist at all here',
+          numberOfAccounts: 1,
+        }),
+      ).rejects.toThrow(
+        'Eth-Hd-Keyring: Invalid secret recovery phrase provided',
+      );
+    });
+
+    it('throws on mnemonic with invalid checksum', async () => {
+      const keyring = new HdKeyring();
+
+      // Valid BIP39 words but invalid checksum (last word changed from 'mango' to 'abandon')
+      await expect(
+        keyring.deserialize({
+          mnemonic:
+            'finish oppose decorate face calm tragic certain desk hour urge dinosaur abandon',
+          numberOfAccounts: 1,
+        }),
+      ).rejects.toThrow(
+        'Eth-Hd-Keyring: Invalid secret recovery phrase provided',
+      );
+    });
+
+    it('throws on invalid mnemonic passed as Buffer', async () => {
+      const keyring = new HdKeyring();
+
+      // Invalid mnemonic as raw Buffer
+      await expect(
+        keyring.deserialize({
+          // @ts-expect-error testing Buffer mnemonic directly
+          mnemonic: Buffer.from('invalid mnemonic phrase here', 'utf8'),
+          numberOfAccounts: 1,
+        }),
+      ).rejects.toThrow(
+        'Eth-Hd-Keyring: Invalid secret recovery phrase provided',
+      );
+    });
+
+    it('deserializes valid mnemonic passed as Buffer', async () => {
+      const keyring = new HdKeyring();
+
+      await keyring.deserialize({
+        // @ts-expect-error testing Buffer mnemonic directly
+        mnemonic: Buffer.from(sampleMnemonic, 'utf8'),
+        numberOfAccounts: 1,
+      });
+
+      const accounts = await keyring.getAccounts();
+      expect(accounts[0]).toStrictEqual(firstAcct);
+    });
+
+    it('validates mnemonic passed as Uint8Array with valid mnemonic', async () => {
+      const keyring = new HdKeyring();
+
+      // Serialize a valid keyring to get the Uint8Array format
+      const tempKeyring = new HdKeyring();
+      await tempKeyring.deserialize({ mnemonic: sampleMnemonic });
+      const { mnemonic } = tempKeyring;
+      assert(mnemonic, 'Mnemonic should be defined');
+
+      await keyring.deserialize({
+        // @ts-expect-error testing Uint8Array mnemonic directly
+        mnemonic,
+        numberOfAccounts: 1,
+      });
+
+      const accounts = await keyring.getAccounts();
+      expect(accounts[0]).toStrictEqual(firstAcct);
+    });
+
     it('throws when numberOfAccounts is passed with no mnemonic', async () => {
       const keyring = new HdKeyring();
 

packages/keyring-eth-hd/src/hd-keyring.ts

@@ -20,7 +20,7 @@ import {
   mnemonicToSeed,
 } from '@metamask/key-tree';
 import type { Keyring } from '@metamask/keyring-utils';
-import { generateMnemonic } from '@metamask/scure-bip39';
+import { generateMnemonic, validateMnemonic } from '@metamask/scure-bip39';
 import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english';
 import {
   add0x,
@@ -610,6 +610,7 @@ export class HdKeyring implements Keyring {
       );
     }
 
+    this.#isValidMnemonic(mnemonic);
     this.mnemonic = this.#mnemonicToUint8Array(mnemonic);
 
     this.seed = await mnemonicToSeed(
@@ -644,4 +645,36 @@ export class HdKeyring implements Keyring {
     assert(normalized, 'Expected address to be set');
     return add0x(normalized);
   }
+
+  /**
+   * Validate the mnemonic seed phrase.
+   *
+   * @param mnemonic - The mnemonic seed phrase to validate.
+   * @throws If the mnemonic is invalid.
+   */
+  #isValidMnemonic(
+    mnemonic: string | number[] | SerializedBuffer | Buffer | Uint8Array,
+  ): void {
+    let mnemonicString: string;
+
+    if (typeof mnemonic === 'string') {
+      mnemonicString = mnemonic;
+    } else if (Array.isArray(mnemonic)) {
+      mnemonicString = Buffer.from(mnemonic).toString();
+    } else if (isSerializedBuffer(mnemonic)) {
+      mnemonicString = Buffer.from(mnemonic.data).toString();
+    } else if (Buffer.isBuffer(mnemonic)) {
+      mnemonicString = mnemonic.toString();
+    } else if (mnemonic instanceof Uint8Array) {
+      mnemonicString = this.#uint8ArrayToString(mnemonic);
+    } else {
+      throw new Error('Eth-Hd-Keyring: Invalid mnemonic format');
+    }
+
+    if (!validateMnemonic(mnemonicString, wordlist)) {
+      throw new Error(
+        'Eth-Hd-Keyring: Invalid secret recovery phrase provided',
+      );
+    }
+  }
 }

yarn.lock

@@ -673,6 +673,16 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@ethersproject/basex@npm:^5.8.0":
+  version: 5.8.0
+  resolution: "@ethersproject/basex@npm:5.8.0"
+  dependencies:
+    "@ethersproject/bytes": "npm:^5.8.0"
+    "@ethersproject/properties": "npm:^5.8.0"
+  checksum: 10/1a8d48a9397461ea42ec43b69a15a0d13ba0b9192695713750d9d391503c55b258cca435fa78a4014d23a813053f1a471593b89c7c0d89351639a78d50a12ef2
+  languageName: node
+  linkType: hard
+
 "@ethersproject/bignumber@npm:^5.7.0, @ethersproject/bignumber@npm:^5.8.0":
   version: 5.8.0
   resolution: "@ethersproject/bignumber@npm:5.8.0"
@@ -702,7 +712,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@ethersproject/hash@npm:^5.7.0":
+"@ethersproject/hash@npm:^5.7.0, @ethersproject/hash@npm:^5.8.0":
   version: 5.8.0
   resolution: "@ethersproject/hash@npm:5.8.0"
   dependencies:
@@ -719,6 +729,26 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@ethersproject/hdnode@npm:^5.8.0":
+  version: 5.8.0
+  resolution: "@ethersproject/hdnode@npm:5.8.0"
+  dependencies:
+    "@ethersproject/abstract-signer": "npm:^5.8.0"
+    "@ethersproject/basex": "npm:^5.8.0"
+    "@ethersproject/bignumber": "npm:^5.8.0"
+    "@ethersproject/bytes": "npm:^5.8.0"
+    "@ethersproject/logger": "npm:^5.8.0"
+    "@ethersproject/pbkdf2": "npm:^5.8.0"
+    "@ethersproject/properties": "npm:^5.8.0"
+    "@ethersproject/sha2": "npm:^5.8.0"
+    "@ethersproject/signing-key": "npm:^5.8.0"
+    "@ethersproject/strings": "npm:^5.8.0"
+    "@ethersproject/transactions": "npm:^5.8.0"
+    "@ethersproject/wordlists": "npm:^5.8.0"
+  checksum: 10/55b35cf30f0dd40e2d5ecd4b2f005ebea82a85a440717a61d4a483074f652d2c7063e9c704272b894bfdd500f7883aa36692931c6808591f702c1da7107ebb61
+  languageName: node
+  linkType: hard
+
 "@ethersproject/keccak256@npm:^5.7.0, @ethersproject/keccak256@npm:^5.8.0":
   version: 5.8.0
   resolution: "@ethersproject/keccak256@npm:5.8.0"
@@ -745,6 +775,16 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@ethersproject/pbkdf2@npm:^5.8.0":
+  version: 5.8.0
+  resolution: "@ethersproject/pbkdf2@npm:5.8.0"
+  dependencies:
+    "@ethersproject/bytes": "npm:^5.8.0"
+    "@ethersproject/sha2": "npm:^5.8.0"
+  checksum: 10/203bb992eec3042256702f4c8259a37202af7b341cc6e370614cdc52541042fc3b795fb040592bd6be8b67376a798c45312ca1e6d5d179c3e8eb7431882f1fd1
+  languageName: node
+  linkType: hard
+
 "@ethersproject/properties@npm:^5.7.0, @ethersproject/properties@npm:^5.8.0":
   version: 5.8.0
   resolution: "@ethersproject/properties@npm:5.8.0"
@@ -764,6 +804,17 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@ethersproject/sha2@npm:^5.8.0":
+  version: 5.8.0
+  resolution: "@ethersproject/sha2@npm:5.8.0"
+  dependencies:
+    "@ethersproject/bytes": "npm:^5.8.0"
+    "@ethersproject/logger": "npm:^5.8.0"
+    hash.js: "npm:1.1.7"
+  checksum: 10/ef8916e3033502476fba9358ba1993722ac3bb99e756d5681e4effa3dfa0f0bf0c29d3fa338662830660b45dd359cccb06ba40bc7b62cfd44f4a177b25829404
+  languageName: node
+  linkType: hard
+
 "@ethersproject/signing-key@npm:^5.8.0":
   version: 5.8.0
   resolution: "@ethersproject/signing-key@npm:5.8.0"
@@ -819,6 +870,19 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@ethersproject/wordlists@npm:^5.8.0":
+  version: 5.8.0
+  resolution: "@ethersproject/wordlists@npm:5.8.0"
+  dependencies:
+    "@ethersproject/bytes": "npm:^5.8.0"
+    "@ethersproject/hash": "npm:^5.8.0"
+    "@ethersproject/logger": "npm:^5.8.0"
+    "@ethersproject/properties": "npm:^5.8.0"
+    "@ethersproject/strings": "npm:^5.8.0"
+  checksum: 10/b8e6aa7d2195bb568847f360f6525ddc3d145404fbd4553e2e05daf4a95f58167591feb69e16e3398a28114ea85e1895fc8f5bd1c0cbf8b578123d7c1d21c32d
+  languageName: node
+  linkType: hard
+
 "@fivebinaries/coin-selection@npm:3.0.0":
   version: 3.0.0
   resolution: "@fivebinaries/coin-selection@npm:3.0.0"
@@ -1684,6 +1748,7 @@ __metadata:
   dependencies:
     "@ethereumjs/tx": "npm:^5.4.0"
     "@ethereumjs/util": "npm:^9.1.0"
+    "@ethersproject/hdnode": "npm:^5.8.0"
     "@lavamoat/allow-scripts": "npm:^3.2.1"
     "@lavamoat/preinstall-always-fail": "npm:^2.1.0"
     "@metamask/account-api": "workspace:^"