fix: enforce mnemonic validation (#450)

The `HdKeyring` class accepts mnemonics during deserialization without
validating that they are valid BIP39 mnemonics. This means invalid
mnemonics (words not in the BIP39 wordlist or invalid checksums) could
be passed in and would fail later in the key derivation process with
unclear error messages.

This PR adds a `isValidMnemonic` private method that validates mnemonics
using `validateMnemonic` from `@metamask/scure-bip39` before processing
them.


<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Adds strict BIP39 validation during `deserialize`/mnemonic
initialization, which can cause previously-accepted (but invalid) secret
recovery phrases to fail earlier and may impact restoration flows.
> 
> **Overview**
> `HdKeyring` now validates provided mnemonics against the BIP39 english
wordlist and checksum (via `validateMnemonic`) before deriving the seed,
throwing `Eth-Hd-Keyring: Invalid secret recovery phrase provided` on
failure.
> 
> Tests were expanded to cover invalid word counts, invalid words,
invalid checksums, and mnemonic inputs in
`Buffer`/`Uint8Array`/serialized forms; the changelog notes the new
enforcement.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
5075bae. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Author: gantunesr

packages/keyring-eth-hd/CHANGELOG.md

@@ -13,6 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Wraps legacy `HdKeyring` to expose accounts via the unified `KeyringV2` API and the `KeyringAccount` type.
   - Extends `EthKeyringWrapper` for common Ethereum logic.
 
+### Fixed
+
+- Enforce mnemonics validation ([#450](https://github.com/MetaMask/accounts/pull/450))
+  - Validates mnemonics against BIP39 specification (word count, wordlist, checksum) before use.
+  - Throws for invalid mnemonics.
+
 ## [13.0.0]
 
 ### Added

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,110 @@ 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('validates mnemonic passed as plain object (simulating encryption/decryption cycle)', async () => {
+      const keyring = new HdKeyring();
+
+      const tempKeyring = new HdKeyring();
+      await tempKeyring.deserialize({ mnemonic: sampleMnemonic });
+      const { mnemonic } = tempKeyring;
+      assert(mnemonic, 'Mnemonic should be defined');
+
+      const mnemonicAsPlainObject: Record<string, number> = {};
+      mnemonic.forEach((value, index) => {
+        mnemonicAsPlainObject[index] = value;
+      });
+
+      await keyring.deserialize({
+        // @ts-expect-error testing plain object mnemonic directly
+        mnemonic: mnemonicAsPlainObject,
+        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,
@@ -38,6 +38,8 @@ import { keccak256 } from 'ethereum-cryptography/keccak';
 const hdPathString = `m/44'/60'/0'/0`;
 const type = 'HD Key Tree';
 
+type Mnemonic = string | number[] | SerializedBuffer | Buffer | Uint8Array;
+
 export type HDKeyringOptions = {
   cryptographicFunctions?: CryptographicFunctions;
 };
@@ -481,9 +483,7 @@ export class HdKeyring implements Keyring {
    * @param mnemonic - The mnemonic seed phrase.
    * @returns The Uint8Array mnemonic.
    */
-  #mnemonicToUint8Array(
-    mnemonic: Buffer | SerializedBuffer | string | Uint8Array | number[],
-  ): Uint8Array {
+  #mnemonicToUint8Array(mnemonic: Mnemonic): Uint8Array {
     let mnemonicData: unknown = mnemonic;
     // When using `Buffer.toJSON()`, the Buffer is serialized into an object
     // with the structure `{ type: 'Buffer', data: [...] }`
@@ -601,16 +601,18 @@ export class HdKeyring implements Keyring {
    * as a string, an array of UTF-8 bytes, or a Buffer. Mnemonic input
    * passed as type buffer or array of UTF-8 bytes must be NFKD normalized.
    */
-  async #initFromMnemonic(
-    mnemonic: string | number[] | SerializedBuffer | Buffer | Uint8Array,
-  ): Promise<void> {
+  async #initFromMnemonic(mnemonic: Mnemonic): Promise<void> {
     if (this.root) {
       throw new Error(
         'Eth-Hd-Keyring: Secret recovery phrase already provided',
       );
     }
 
-    this.mnemonic = this.#mnemonicToUint8Array(mnemonic);
+    // Convert and validate before assigning to instance property
+    // to avoid inconsistent state if validation fails
+    const mnemonicAsUint8Array = this.#mnemonicToUint8Array(mnemonic);
+    this.#assertValidMnemonic(mnemonicAsUint8Array);
+    this.mnemonic = mnemonicAsUint8Array;
 
     this.seed = await mnemonicToSeed(
       this.mnemonic,
@@ -644,4 +646,20 @@ export class HdKeyring implements Keyring {
     assert(normalized, 'Expected address to be set');
     return add0x(normalized);
   }
+
+  /**
+   * Assert that the mnemonic seed phrase is valid.
+   * Throws an error if the mnemonic is not a valid BIP39 phrase.
+   *
+   * @param mnemonic - The mnemonic seed phrase to validate (as Uint8Array).
+   * @throws If the mnemonic is not a valid BIP39 secret recovery phrase.
+   */
+  #assertValidMnemonic(mnemonic: Uint8Array): void {
+    const mnemonicString = this.#uint8ArrayToString(mnemonic);
+    if (!validateMnemonic(mnemonicString, wordlist)) {
+      throw new Error(
+        'Eth-Hd-Keyring: Invalid secret recovery phrase provided',
+      );
+    }
+  }
 }