From b155c0fcd69f0c2f51c2b7a81a7fc0abdd9a19b9 Mon Sep 17 00:00:00 2001
From: clawbot <clawbot@noreply.example.org>
Date: Tue, 11 Aug 2026 15:31:50 +0200
Subject: [PATCH] fix: enforce the base58 checksum and reject non-master
 extended keys (closes #210)

---
 TODO.md                      |   4 +
 src/popup/index.html         |   4 +-
 src/popup/views/addWallet.js |  16 +++-
 src/shared/wallet.js         |  86 +++++++++++++++----
 tests/wallet.test.js         | 158 ++++++++++++++++++++++++++++++++---
 5 files changed, 238 insertions(+), 30 deletions(-)

diff --git a/TODO.md b/TODO.md
index 8b17986..985e5b6 100644
--- a/TODO.md
+++ b/TODO.md
@@ -57,6 +57,10 @@ undefined identifiers, which is how
   from the wallet row in Settings, wiped on leaving the screen and excluded from
   the views the popup can reopen onto
   ([#161](https://git.eeqj.de/sneak/AutistMask/issues/161)).
+- 2026-08-11: Extended-key import hardened — the base58 checksum is now enforced
+  on every xprv and xpub, and a non-master key is refused with an explanation
+  instead of being derived beneath
+  ([#210](https://git.eeqj.de/sneak/AutistMask/issues/210)).
 - 2026-08-11: Policy compliance sweep — conditional verbose test rerun, local
   Tailwind binary instead of `npx`, `--frozen-lockfile` on `make install`, and
   the Makefile-only targets documented in the README
diff --git a/src/popup/index.html b/src/popup/index.html
index 361e864..b40e098 100644
--- a/src/popup/index.html
+++ b/src/popup/index.html
@@ -136,7 +136,9 @@
                 <div id="add-wallet-section-xprv" class="hidden">
                     <p class="mb-2">
                         Paste your extended private key (xprv) below. This will
-                        import the HD wallet and scan for used addresses.
+                        import the HD wallet and scan for used addresses. It
+                        must be the master key for the wallet; an account-level
+                        or child key is not supported.
                     </p>
                     <div class="mb-2">
                         <input
diff --git a/src/popup/views/addWallet.js b/src/popup/views/addWallet.js
index 5a746e1..063bbbe 100644
--- a/src/popup/views/addWallet.js
+++ b/src/popup/views/addWallet.js
@@ -6,6 +6,7 @@ const {
     addressFromPrivateKey,
     hdWalletFromXprv,
     isValidXprv,
+    isMasterExtendedKey,
 } = require("../../shared/wallet");
 const { encryptWithPassword } = require("../../shared/vault");
 const { state, saveState } = require("../../shared/state");
@@ -213,14 +214,25 @@ async function importXprvKey(ctx) {
         return;
     }
     if (!isValidXprv(xprv)) {
-        showFlash("Invalid extended private key.");
+        showFlash(
+            "That extended private key is not valid. Please check it and try again.",
+        );
+        return;
+    }
+    if (!isMasterExtendedKey(xprv)) {
+        showFlash(
+            "That is an account-level or child key, which cannot be imported. " +
+                "Please paste the master extended private key for the wallet.",
+        );
         return;
     }
     let result;
     try {
         result = hdWalletFromXprv(xprv);
     } catch (e) {
-        showFlash("Invalid extended private key.");
+        showFlash(
+            "That extended private key is not valid. Please check it and try again.",
+        );
         return;
     }
     const { xpub, firstAddress } = result;
diff --git a/src/shared/wallet.js b/src/shared/wallet.js
index 66d760a..0d98a96 100644
--- a/src/shared/wallet.js
+++ b/src/shared/wallet.js
@@ -16,8 +16,60 @@ function generateMnemonic() {
     return m.phrase;
 }
 
+// Every extended key (xprv or xpub) entering the app goes through this.
+//
+// ethers' HDNodeWallet.fromExtendedKey does NOT verify the base58 checksum
+// when the decoded payload is the usual 82 bytes, which is exactly the case
+// the checksum exists to catch: a key with a one-character typo parses into a
+// *different* wallet instead of being rejected. Re-encoding the parsed node
+// reproduces a well-formed key byte for byte, checksum included, so comparing
+// the round trip against the input rejects any altered character. Measured by
+// the sweep in tests/wallet.test.js over every single-character substitution
+// of the BIP-32 vector 1 master key: 199 parse without the round-trip
+// comparison, 0 with it.
+//
+// Returns the parsed node, or null if the key is not a well-formed extended
+// key. Callers turn null into a user-facing error; none of them may fall back
+// to fromExtendedKey directly.
+function parseExtendedKey(key) {
+    if (typeof key !== "string") return null;
+    try {
+        const node = HDNodeWallet.fromExtendedKey(key);
+        return node.extendedKey === key ? node : null;
+    } catch {
+        return null;
+    }
+}
+
+// A master key is at depth 0. Only from there is BIP44_ETH_PATH the absolute
+// path it names; deriving it under an account-level or child key yields
+// addresses that correspond to nothing the user holds.
+const MASTER_DEPTH = 0;
+
+// Parse an extended private key that the BIP-44 Ethereum account path can be
+// derived from, or throw. Both callers derive BIP44_ETH_PATH from the result.
+function masterXprvOrThrow(key) {
+    const node = parseExtendedKey(key);
+    if (!node) {
+        throw new Error("Not a valid extended private key (xprv).");
+    }
+    if (!node.privateKey) {
+        throw new Error("Not an extended private key (xprv).");
+    }
+    if (node.depth !== MASTER_DEPTH) {
+        throw new Error(
+            "Not a master extended private key (xprv): an account-level or " +
+                "child key cannot be imported.",
+        );
+    }
+    return node;
+}
+
 function deriveAddressFromXpub(xpub, index) {
-    const node = HDNodeWallet.fromExtendedKey(xpub);
+    const node = parseExtendedKey(xpub);
+    if (!node) {
+        throw new Error("Not a valid extended key.");
+    }
     return node.deriveChild(index).address;
 }
 
@@ -29,23 +81,28 @@ function hdWalletFromMnemonic(mnemonic) {
 }
 
 function hdWalletFromXprv(xprv) {
-    const root = HDNodeWallet.fromExtendedKey(xprv);
-    if (!root.privateKey) {
-        throw new Error("Not an extended private key (xprv).");
-    }
-    const node = root.derivePath("44'/60'/0'/0");
+    // BIP44_ETH_PATH is absolute ("m/..."), which ethers will only derive from
+    // a depth-0 node. The relative form this used to derive would have been
+    // applied *beneath* an account-level key instead of being refused.
+    const node = masterXprvOrThrow(xprv).derivePath(BIP44_ETH_PATH);
     const xpub = node.neuter().extendedKey;
     const firstAddress = node.deriveChild(0).address;
     return { xpub, firstAddress };
 }
 
+// Well-formed extended private key. Says nothing about depth: the import view
+// reports a non-master key separately, since "check it for a typo" is the
+// wrong advice for a key the user copied correctly.
 function isValidXprv(key) {
-    try {
-        const node = HDNodeWallet.fromExtendedKey(key);
-        return !!node.privateKey;
-    } catch {
-        return false;
-    }
+    const node = parseExtendedKey(key);
+    return !!(node && node.privateKey);
+}
+
+// Whether an extended key is a master key, i.e. the one BIP44_ETH_PATH can be
+// derived from. False for anything parseExtendedKey rejects.
+function isMasterExtendedKey(key) {
+    const node = parseExtendedKey(key);
+    return !!node && node.depth === MASTER_DEPTH;
 }
 
 function addressFromPrivateKey(key) {
@@ -63,8 +120,8 @@ function getSignerForAddress(walletData, addrIndex, decryptedSecret) {
         return node.deriveChild(addrIndex);
     }
     if (walletData.type === "xprv") {
-        const root = HDNodeWallet.fromExtendedKey(decryptedSecret);
-        const node = root.derivePath("44'/60'/0'/0");
+        const node =
+            masterXprvOrThrow(decryptedSecret).derivePath(BIP44_ETH_PATH);
         return node.deriveChild(addrIndex);
     }
     return new Wallet(decryptedSecret);
@@ -89,6 +146,7 @@ module.exports = {
     hdWalletFromMnemonic,
     hdWalletFromXprv,
     isValidXprv,
+    isMasterExtendedKey,
     addressFromPrivateKey,
     getSignerForAddress,
     isValidMnemonic,
diff --git a/tests/wallet.test.js b/tests/wallet.test.js
index 9054552..0adf376 100644
--- a/tests/wallet.test.js
+++ b/tests/wallet.test.js
@@ -160,6 +160,31 @@ function masterXprv(phrase, passphrase = "") {
     ).extendedKey;
 }
 
+// The account-level (depth-3) extended private key m/44'/60'/0' for a phrase.
+// A normal thing for a user to hold, and not something the import flow can
+// derive the BIP-44 account path from.
+function accountXprv(phrase) {
+    return HDNodeWallet.fromSeed(
+        Mnemonic.fromPhrase(phrase, "").computeSeed(),
+    ).derivePath("m/44'/60'/0'").extendedKey;
+}
+
+// Every single-character substitution of `key`, using base58 characters that
+// are not the original. Base58 has no visually ambiguous characters, so each
+// of these is a plausible typo rather than a contrived string.
+const TYPO_CHARS = ["a", "b", "2", "Z"];
+
+function singleCharacterTypos(key) {
+    const out = [];
+    for (let i = 0; i < key.length; i++) {
+        for (const c of TYPO_CHARS) {
+            if (c === key[i]) continue;
+            out.push(key.slice(0, i) + c + key.slice(i + 1));
+        }
+    }
+    return out;
+}
+
 describe("hdWalletFromMnemonic", () => {
     test("first address matches the published vector for m/44'/60'/0'/0/0", () => {
         expect(wallet.hdWalletFromMnemonic(VECTOR_PHRASE).firstAddress).toBe(
@@ -299,19 +324,7 @@ describe("isValidXprv", () => {
         expect(wallet.isValidXprv(xpub)).toBe(false);
     });
 
-    // Skipped: this asserts the correct behaviour, which the code does not
-    // currently have. isValidXprv gates the paste-your-extended-private-key
-    // import in src/popup/views/addWallet.js:215, and it accepts a key with a
-    // one-character typo: ethers' HDNodeWallet.fromExtendedKey skips base58
-    // checksum verification whenever the decoded payload is the usual 82
-    // bytes, which is the whole point of that checksum. Measured on this
-    // vector: changing any one of the last 14 characters passes validation,
-    // and for 9 of those 14 positions the import silently yields a *different*
-    // wallet (e.g. 0x3F334f0a356d6B46B1d70B590E7437D77100d28D instead of
-    // 0x022b971dFF0C43305e691DEd7a14367AF19D6407) with no error shown.
-    // Tracked as https://git.eeqj.de/sneak/AutistMask/issues/210; out of scope
-    // here, which is tests only. Unskip when it is fixed.
-    test.skip("rejects an extended key with a one-character typo", () => {
+    test("rejects an extended key with a one-character typo", () => {
         const index = BIP32_VECTOR_1_XPRV.length - 8;
         const typo =
             BIP32_VECTOR_1_XPRV.slice(0, index) +
@@ -320,6 +333,125 @@ describe("isValidXprv", () => {
 
         expect(wallet.isValidXprv(typo)).toBe(false);
     });
+
+    // The base58 checksum exists to make a mistyped key impossible to use, and
+    // ethers does not enforce it: HDNodeWallet.fromExtendedKey skips checksum
+    // verification whenever the decoded payload is the usual 82 bytes, which
+    // is precisely the case it is there to catch. A typo anywhere in the key
+    // must be refused, not silently turned into someone else's wallet.
+    test("no single-character typo anywhere in the key is accepted", () => {
+        const accepted = singleCharacterTypos(BIP32_VECTOR_1_XPRV).filter(
+            (typo) => wallet.isValidXprv(typo),
+        );
+
+        expect(accepted).toEqual([]);
+    });
+
+    test("a typo never yields a wallet, let alone a different one", () => {
+        const correct = wallet.hdWalletFromXprv(BIP32_VECTOR_1_XPRV);
+        const derived = [];
+        for (const typo of singleCharacterTypos(BIP32_VECTOR_1_XPRV)) {
+            try {
+                derived.push(wallet.hdWalletFromXprv(typo).firstAddress);
+            } catch {
+                // Rejected, which is the required behaviour.
+            }
+        }
+
+        expect(derived).toEqual([]);
+        expect(correct.firstAddress).toBe(
+            "0x022b971dFF0C43305e691DEd7a14367AF19D6407",
+        );
+    });
+});
+
+describe("extended key depth", () => {
+    // hdWalletFromXprv derives the BIP-44 Ethereum account path from the key
+    // it is given. That is only the path it names when the key is the master
+    // key. Under an account-level key the same derivation lands at
+    // m/44'/60'/0'/44'/60'/0'/0, whose addresses correspond to nothing the
+    // user holds, so a non-master key is refused rather than derived from.
+    test("a master key is a master key", () => {
+        expect(wallet.isMasterExtendedKey(masterXprv(VECTOR_PHRASE))).toBe(
+            true,
+        );
+        expect(wallet.isMasterExtendedKey(BIP32_VECTOR_1_XPRV)).toBe(true);
+    });
+
+    test("an account-level key is not a master key", () => {
+        expect(wallet.isMasterExtendedKey(accountXprv(VECTOR_PHRASE))).toBe(
+            false,
+        );
+    });
+
+    test("a derived xpub is not a master key", () => {
+        expect(
+            wallet.isMasterExtendedKey(
+                wallet.hdWalletFromMnemonic(VECTOR_PHRASE).xpub,
+            ),
+        ).toBe(false);
+    });
+
+    test("a mistyped key is not a master key either", () => {
+        expect(wallet.isMasterExtendedKey(BIP32_VECTOR_1_XPRV + "a")).toBe(
+            false,
+        );
+    });
+
+    test("hdWalletFromXprv rejects an account-level key", () => {
+        expect(() =>
+            wallet.hdWalletFromXprv(accountXprv(VECTOR_PHRASE)),
+        ).toThrow(/master/i);
+    });
+
+    test("getSignerForAddress rejects an account-level key", () => {
+        expect(() =>
+            wallet.getSignerForAddress(
+                { type: "xprv" },
+                0,
+                accountXprv(VECTOR_PHRASE),
+            ),
+        ).toThrow(/master/i);
+    });
+
+    test("the account-level key is well-formed, so only depth rejects it", () => {
+        expect(wallet.isValidXprv(accountXprv(VECTOR_PHRASE))).toBe(true);
+    });
+
+    test("a master key still imports and derives the published addresses", () => {
+        const { xpub, firstAddress } = wallet.hdWalletFromXprv(
+            masterXprv(VECTOR_PHRASE),
+        );
+
+        expect(firstAddress).toBe(VECTOR_ADDRESSES[0]);
+        expect(
+            [0, 1, 2].map((i) => wallet.deriveAddressFromXpub(xpub, i)),
+        ).toEqual(VECTOR_ADDRESSES);
+    });
+});
+
+describe("deriveAddressFromXpub checksum enforcement", () => {
+    // The xpub path shares the hole: fromExtendedKey accepts a mistyped xpub
+    // just as readily, and deriveAddressFromXpub would hand back addresses
+    // from a different tree.
+    const { xpub } = wallet.hdWalletFromMnemonic(VECTOR_PHRASE);
+
+    test("the correct xpub still derives the published addresses", () => {
+        expect(wallet.deriveAddressFromXpub(xpub, 0)).toBe(VECTOR_ADDRESSES[0]);
+    });
+
+    test("no single-character typo anywhere in an xpub is accepted", () => {
+        const derived = [];
+        for (const typo of singleCharacterTypos(xpub)) {
+            try {
+                derived.push(wallet.deriveAddressFromXpub(typo, 0));
+            } catch {
+                // Rejected, which is the required behaviour.
+            }
+        }
+
+        expect(derived).toEqual([]);
+    });
 });
 
 describe("isValidMnemonic", () => {