fix: typed data for another chain cannot be signed (closes #519)

EIP-712 has a wallet refuse typed data whose domain chainId is not the
active chain, because the signature is valid on the chain it names. The
sign screen now shows such a request with the reason on its error line
and Sign disabled, the same way it refuses a mismatched primary type.
The background checks again when the signature comes back, against the
network active then, and answers the site with that refusal instead of
the signature. A chain id is read as ethers signs it, so 1, "1" and
"0x1" are one chain. Typed data with no chainId signs as before.

Model: opus-5-5

README.md

@@ -1188,14 +1188,7 @@ The core hierarchy is **Wallets → Addresses**:
       of its stored `xpub` by `src/shared/walletDefects.js`, explained in the
       wallet list, and blocked from signing, sending and private-key export. It
       is never deleted or rewritten.
-- An **address** holds ETH and ERC-20 tokens. In an HD or xprv wallet each
-  address record also stores `index`, the derivation index it was made at: 0 for
-  the first address, the index the import scan found it at, or `nextIndex` when
-  "+" added it. An address's `index` and its position in the wallet's list
-  differ once the scan has passed over an unused index or an address has been
-  removed. Sending, site transactions, site signatures and private-key export
-  derive the key at `index`, and refuse it unless it is that address's own key
-  (`getSignerForAddress()` in `src/shared/wallet.js`).
+- An **address** holds ETH and ERC-20 tokens.
 - The user can have multiple wallets, each with multiple addresses (HD) or a
   single address (key).
 
@@ -1521,9 +1514,6 @@ view would leave a wallet one click from deletion.
       change)
     - "Reveal" (wrong password) → full-sentence error on the error line, nothing
       revealed (no screen change)
-    - "Reveal" (correct password, but the key derived is not this address's own,
-      see Data Model) → full-sentence error saying so, nothing revealed (no
-      screen change)
     - "Back" → previous screen (AddressDetail)
     - Settings gear → **Settings**, whose "Back" goes to AddressDetail: leaving
       drops the address the screen was showing, so it also takes the screen off
@@ -2170,6 +2160,13 @@ view would leave a wallet one click from deletion.
     - Typed data that states no primary type, or one other than the type it
       would be signed as, or that cannot be read → shown with the error line
       saying so and "Sign" disabled; only "Reject" remains
+    - Typed data whose domain `chainId` names a chain other than the active
+      network's → shown with the error line naming both chain ids and "Sign"
+      disabled; only "Reject" remains. EIP-712 asks for this, because the
+      signature is valid on the chain the domain names. Typed data with no
+      `chainId` is not refused. The background refuses such a signature too,
+      against the network active when the signature reaches it, and answers the
+      site with the same sentence
     - A `personal_sign` or `eth_sign` message that is not hex (`0x` or `0X` and
       an even number of hex digits, the form ethers' `getBytes` reads when
       signing) → shown as plain text, with the error line "This message is plain

src/background/index.js

@@ -22,6 +22,7 @@ const { debugFetch, log } = require("../shared/log");
 const {
     verifySignedTx,
     verifySignature,
+    typedDataChainRefusal,
     failureIsRetryable,
     describeTxFailure,
     sameAddress,
@@ -1764,7 +1765,8 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
 
         (async () => {
             try {
-                const activeAddress = await getActiveAddress();
+                const s = await getState();
+                const activeAddress = activeAddressOf(s);
                 // Same as the transaction path: the address the approval named
                 // is the one that must have signed, and a switch since then is
                 // a refusal rather than a signature from another account.
@@ -1783,6 +1785,20 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                     signature,
                     approval.approvedFrom,
                 );
+                // The sign screen refuses typed data whose domain names
+                // another chain. A signature for it that arrives anyway is
+                // refused here, against the network active now.
+                const sp = approval.signParams;
+                if (
+                    sp.method === "eth_signTypedData_v4" ||
+                    sp.method === "eth_signTypedData"
+                ) {
+                    const refusal = typedDataChainRefusal(
+                        JSON.parse(sp.typedData).domain,
+                        networkById(s.networkId),
+                    );
+                    if (refusal) throw new ApprovalMismatchError(refusal);
+                }
                 settleApproval(msg.id, { signature }, { holdsClaim: true });
                 sendResponse({ signature });
             } catch (e) {

src/popup/index.html

@@ -7,10 +7,9 @@
         <link rel="stylesheet" href="styles.css" />
     </head>
     <!-- Chrome gives extension pages a stylesheet of its own that sets the
-         font and its size on body, and a Tailwind class beats it only when
-         marked important: hence font-mono! and text-sm! rather than
-         font-mono and text-sm. -->
-    <body class="bg-bg text-fg font-mono! text-sm!">
+         font on body, and a Tailwind class beats it only when marked
+         important: hence font-mono! rather than font-mono. -->
+    <body class="bg-bg text-fg font-mono! text-sm">
         <div id="app" class="p-2 pr-5 overflow-x-hidden">
             <!-- ============ GLOBAL TITLE BAR ============ -->
             <div
@@ -88,14 +87,11 @@
 
                 <!-- Mnemonic form section -->
                 <div id="add-wallet-section-mnemonic">
-                    <!-- The die button shares the instruction's row: on a row
-                         of its own it pushes Import below the fold at
-                         360x600. -->
-                    <div class="mb-2 flex justify-between items-start gap-2">
-                        <p>
-                            Enter your 12 or 24 word recovery phrase below, or
-                            click the button to roll the die for a new one.
-                        </p>
+                    <p class="mb-2">
+                        Enter your 12 or 24 word recovery phrase below, or click
+                        the button to roll the die for a new one.
+                    </p>
+                    <div class="mb-1 flex justify-end">
                         <button
                             id="btn-generate-phrase"
                             class="border border-border px-2 py-1 hover:bg-fg hover:text-bg cursor-pointer text-xs"

src/popup/views/addWallet.js

@@ -184,12 +184,7 @@ async function importMnemonic(ctx) {
         encryptedSecret: encrypted,
         nextIndex: 1,
         addresses: [
-            {
-                address: firstAddress,
-                index: 0,
-                balance: "0.0000",
-                tokenBalances: [],
-            },
+            { address: firstAddress, balance: "0.0000", tokenBalances: [] },
         ],
     };
     state.wallets.push(wallet);
@@ -210,7 +205,6 @@ async function importMnemonic(ctx) {
     if (scan.addresses.length > 1) {
         wallet.addresses = scan.addresses.map((a) => ({
             address: a.address,
-            index: a.index,
             balance: "0.0000",
             tokenBalances: [],
         }));
@@ -305,12 +299,7 @@ async function importXprvKey(ctx) {
         encryptedSecret: encrypted,
         nextIndex: 1,
         addresses: [
-            {
-                address: firstAddress,
-                index: 0,
-                balance: "0.0000",
-                tokenBalances: [],
-            },
+            { address: firstAddress, balance: "0.0000", tokenBalances: [] },
         ],
     };
     state.wallets.push(wallet);
@@ -331,7 +320,6 @@ async function importXprvKey(ctx) {
     if (scan.addresses.length > 1) {
         wallet.addresses = scan.addresses.map((a) => ({
             address: a.address,
-            index: a.index,
             balance: "0.0000",
             tokenBalances: [],
         }));

src/popup/views/approval.js

@@ -12,7 +12,7 @@ const {
     formatFee,
     tokenLabel,
 } = require("./helpers");
-const { state, saveState } = require("../../shared/state");
+const { state, saveState, currentNetwork } = require("../../shared/state");
 const {
     networkById,
     networkByChainId,
@@ -46,7 +46,10 @@ const {
 const { decryptWithPassword } = require("../../shared/vault");
 const { getSignerForAddress } = require("../../shared/wallet");
 const { walletDefect } = require("../../shared/walletDefects");
-const { describeSigningFailure } = require("../../shared/approvalVerify");
+const {
+    describeSigningFailure,
+    typedDataChainRefusal,
+} = require("../../shared/approvalVerify");
 const txStatus = require("./txStatus");
 const uniswap = require("../../shared/uniswap");
 const { notify, runtimeApi, sendMessage } = require("../../shared/browserApi");
@@ -443,7 +446,8 @@ function signedPrimaryType(types) {
 // Why a signature request cannot be signed, as a sentence for the error line,
 // or null when it can. Only typed data is refused: the `primaryType` the page
 // states has to be the type ethers will sign, or this screen would name one
-// message while another is signed.
+// message while another is signed. Its domain may name no chain but the active
+// network's.
 function typedDataRefusal(sp) {
     if (sp.method === "personal_sign" || sp.method === "eth_sign") return null;
     let data;
@@ -466,7 +470,7 @@ function typedDataRefusal(sp) {
             ", so it cannot be signed."
         );
     }
-    return null;
+    return typedDataChainRefusal(data.domain, currentNetwork());
 }
 
 // The largest amount a Permit2 allowance can hold, a uint160. Permit2 treats
@@ -863,15 +867,15 @@ function gateOnWalletDefect(errorId, buttonId, address) {
     return true;
 }
 
-// Locate the wallet and the stored address record owning an address. Returns
-// null when no wallet holds it. Approvals look up the address they were raised
-// for, not whichever address is active now: the approval named one account,
-// and signing with another is what verification refuses.
+// Locate the wallet and the address index owning an address. Returns null when
+// no wallet holds it. Approvals look up the address they were raised for, not
+// whichever address is active now: the approval named one account, and signing
+// with another is what verification refuses.
 function findWalletFor(address) {
     for (const wallet of state.wallets) {
         for (let i = 0; i < wallet.addresses.length; i++) {
             if (wallet.addresses[i].address === address) {
-                return { wallet, addr: wallet.addresses[i] };
+                return { wallet, addrIndex: i };
             }
         }
     }
@@ -998,7 +1002,7 @@ function init(_ctx) {
         try {
             const signer = getSignerForAddress(
                 active.wallet,
-                active.addr,
+                active.addrIndex,
                 decryptedSecret,
             );
             // Sign the approved transaction exactly as it was displayed. The
@@ -1126,7 +1130,7 @@ function init(_ctx) {
         try {
             const signer = getSignerForAddress(
                 active.wallet,
-                active.addr,
+                active.addrIndex,
                 decryptedSecret,
             );
             const sp = pendingSignParams;

src/popup/views/confirmTx.js

@@ -583,7 +583,7 @@ function init(_ctx) {
         try {
             const signer = getSignerForAddress(
                 wallet,
-                wallet.addresses[state.selectedAddress],
+                state.selectedAddress,
                 decryptedSecret,
             );
             const provider = getProvider(state.rpcUrl, state.networkId);

src/popup/views/exportPrivkey.js

@@ -135,18 +135,7 @@ async function reveal() {
         // decrypt ran, the wipe has already happened, so the key is not
         // even derived, let alone written.
         if (!isCurrentReveal(generation)) return;
-        let signer;
-        try {
-            signer = getSignerForAddress(
-                wallet,
-                wallet.addresses[addressIndex],
-                secret,
-            );
-        } catch (e) {
-            // The password was right; it is the key that was refused.
-            showError("export-privkey-password-error", e.message);
-            return;
-        }
+        const signer = getSignerForAddress(wallet, addressIndex, secret);
         $("export-privkey-password").value = "";
         $("export-privkey-password-section").classList.add("hidden");
         $("export-privkey-value").textContent = signer.privateKey;

src/popup/views/home.js

@@ -354,7 +354,6 @@ function render(ctx) {
             );
             wallet.addresses.push({
                 address: newAddr,
-                index: wallet.nextIndex,
                 balance: "0.0000",
                 tokenBalances: [],
             });

src/shared/approvalVerify.js

@@ -53,6 +53,7 @@ const {
     accessListify,
     formatEther,
     getAddress,
+    getBigInt,
     getBytes,
     toQuantity,
     verifyMessage,
@@ -635,6 +636,33 @@ function verifySignature(signParams, signature, expectedFrom) {
     return recovered;
 }
 
+// Why typed data cannot be signed while `network` is the active network, as a
+// sentence, or null when it can. EIP-712 has a wallet refuse typed data whose
+// domain names another chain, because the signature is valid on that chain.
+// ethers signs no chain id when the domain's is null or absent, and reads any
+// other with getBigInt(), so 1, "1" and "0x1" are the same chain here too.
+function typedDataChainRefusal(domain, network) {
+    const chainId = domain?.chainId;
+    if (chainId === undefined || chainId === null) return null;
+    let named;
+    try {
+        named = getBigInt(chainId);
+    } catch {
+        return "This typed data cannot be read, so it cannot be signed.";
+    }
+    const active = getBigInt(network.chainId);
+    if (named === active) return null;
+    return (
+        "This typed data is for chain id " +
+        named +
+        ", but the active network is " +
+        network.name +
+        " (chain id " +
+        active +
+        "), so it cannot be signed."
+    );
+}
+
 // The stage a transaction approval failed at. Which stage it is decides
 // whether the approval survives the failure.
 const TX_STAGE_SIGN = "sign";
@@ -796,6 +824,7 @@ function describeSigningFailure(response, fallbackMessage) {
 module.exports = {
     verifySignedTx,
     verifySignature,
+    typedDataChainRefusal,
     assertNoForbiddenFields,
     assertNothingUnchecked,
     assertCanonicalBytes,

src/shared/wallet.js

@@ -110,21 +110,16 @@ function addressFromPrivateKey(key) {
     return w.address;
 }
 
-// The signer for `addr`, one of the wallet's stored address records. An HD or
-// xprv address is derived at its own `index`, never at its position in the
-// wallet's list: the two differ once an import scan passes over an unused
-// index, or once an address that is not the last is removed. Whatever the
-// wallet type, the key is refused unless it is the key of `addr.address`.
-function getSignerForAddress(walletData, addr, decryptedSecret) {
-    let signer;
+function getSignerForAddress(walletData, addrIndex, decryptedSecret) {
     if (walletData.type === "hd") {
         const node = HDNodeWallet.fromPhrase(
             decryptedSecret,
             "",
             BIP44_ETH_PATH,
         );
-        signer = node.deriveChild(addr.index);
-    } else if (walletData.type === "xprv") {
+        return node.deriveChild(addrIndex);
+    }
+    if (walletData.type === "xprv") {
         // Checked here rather than through masterXprvOrThrow so the message
         // fits the situation: nobody is importing anything at signing time,
         // and this wallet is already in storage. src/shared/walletDefects.js
@@ -142,18 +137,9 @@ function getSignerForAddress(walletData, addr, decryptedSecret) {
                     "is not a master key, so it cannot sign.",
             );
         }
-        signer = node.derivePath(BIP44_ETH_PATH).deriveChild(addr.index);
-    } else {
-        signer = new Wallet(decryptedSecret);
+        return node.derivePath(BIP44_ETH_PATH).deriveChild(addrIndex);
     }
-    if (signer.address.toLowerCase() !== addr.address.toLowerCase()) {
-        throw new Error(
-            "The key this wallet holds for " +
-                addr.address +
-                " belongs to a different address, so it was not used.",
-        );
-    }
-    return signer;
+    return new Wallet(decryptedSecret);
 }
 
 function isValidMnemonic(mnemonic) {

tests/addressKey.test.js

@@ -1,386 +0,0 @@
-// Sending, a site's signature request and private key export each use the key
-// of the address they name: the key at the index that address was derived at,
-// not the one at its position in the wallet's list. The two differ after an
-// import whose scan passed over an unused index, and after an address that is
-// not the last is removed (https://git.eeqj.de/sneak/AutistMask/issues/510).
-//
-// Driven through the real import, "+" button, address removal, confirmation
-// screen, signature approval screen and export screen, with the node, the vault
-// and the DOM stubbed.
-
-"use strict";
-
-// The addresses the stub node reports a balance for, and the signed
-// transactions it was handed.
-const mockNode = { funded: new Set(), broadcast: [] };
-
-jest.mock("ethers", () => {
-    const actual = jest.requireActual("ethers");
-    class StubProvider {
-        async getBalance(address) {
-            return mockNode.funded.has(address) ? 1n : 0n;
-        }
-        async getTransactionCount() {
-            return 0;
-        }
-        async estimateGas() {
-            return 21000n;
-        }
-        async getFeeData() {
-            return {
-                maxFeePerGas: 20000000000n,
-                maxPriorityFeePerGas: 1000000000n,
-                gasPrice: 10000000000n,
-            };
-        }
-        async getNetwork() {
-            return { chainId: 1n };
-        }
-        async getCode() {
-            return "0x";
-        }
-        async lookupAddress() {
-            return null;
-        }
-        async broadcastTransaction(signed) {
-            mockNode.broadcast.push(signed);
-            return { hash: "0x" + "ab".repeat(32) };
-        }
-    }
-    return { ...actual, JsonRpcProvider: StubProvider };
-});
-
-// No password is hashed: what the wallet stores is the phrase itself.
-jest.mock("../src/shared/vault", () => ({
-    encryptWithPassword: async (secret) => secret,
-    decryptWithPassword: async (secret) => secret,
-}));
-
-// The wait screen polls for a receipt; these tests stop at the broadcast.
-jest.mock("../src/popup/views/txStatus", () => ({
-    showWait: jest.fn(),
-    showError: jest.fn(),
-}));
-
-jest.mock("ethereum-blockies-base64", () => () => "data:image/png;base64,x");
-
-global.fetch = jest.fn(() => {
-    throw new Error("tests must not perform network requests");
-});
-
-const {
-    HDNodeWallet,
-    Transaction,
-    getBytes,
-    verifyMessage,
-} = require("ethers");
-
-// Publicly known development phrase; never fund it.
-const PHRASE = "test test test test test test test test test test test junk";
-const PASSWORD = "correct horse battery staple";
-const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
-// "Hello", as the hex a page sends.
-const MESSAGE = "0x48656c6c6f";
-
-// The address and key at derivation index `index` of PHRASE, worked out with
-// ethers directly rather than through the wallet's own code.
-function derived(index) {
-    return HDNodeWallet.fromPhrase(PHRASE, "", "m/44'/60'/0'/0/" + index);
-}
-
-// ------------------------------------------------------------ DOM stub
-
-function makeElement(id) {
-    const classes = new Set();
-    const el = {
-        id,
-        textContent: "",
-        title: "",
-        value: "",
-        innerHTML: "",
-        disabled: false,
-        style: {},
-        dataset: {},
-        listeners: {},
-        classList: {
-            add: (...names) => names.forEach((n) => classes.add(n)),
-            remove: (...names) => names.forEach((n) => classes.delete(n)),
-            contains: (n) => classes.has(n),
-            toggle: (n, force) => {
-                const on = force === undefined ? !classes.has(n) : force;
-                if (on) classes.add(n);
-                else classes.delete(n);
-                return on;
-            },
-        },
-        addEventListener: (name, fn) => {
-            el.listeners[name] = el.listeners[name] || [];
-            el.listeners[name].push(fn);
-        },
-        querySelectorAll: () => [],
-        appendChild: () => {},
-    };
-    // Views reach for .parentElement to hide whole sections.
-    Object.defineProperty(el, "parentElement", {
-        get: () => node(id + "-parent"),
-    });
-    return el;
-}
-
-function makeDocument() {
-    const els = new Map();
-    return {
-        getElementById(id) {
-            // The debug banner is created on demand by helpers.js; absent
-            // is the state a non-debug, non-testnet popup is in.
-            if (id === "debug-banner") return null;
-            if (!els.has(id)) els.set(id, makeElement(id));
-            return els.get(id);
-        },
-        createElement: () => makeElement("created"),
-        addEventListener: () => {},
-        body: { prepend: () => {} },
-    };
-}
-
-function node(id) {
-    return globalThis.document.getElementById(id);
-}
-
-function click(element) {
-    const event = { stopPropagation: () => {} };
-    return Promise.all((element.listeners.click || []).map((fn) => fn(event)));
-}
-
-// show() starts the fee estimate without awaiting it; this lets it settle.
-async function settle() {
-    for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
-}
-
-// ------------------------------------------------------------ harness
-
-globalThis.chrome = {
-    storage: { local: { get: async () => ({}), set: async () => {} } },
-};
-
-const { state } = require("../src/shared/state");
-const { removeAddressFromState } = require("../src/shared/walletDelete");
-const addWallet = require("../src/popup/views/addWallet");
-const home = require("../src/popup/views/home");
-const confirmTx = require("../src/popup/views/confirmTx");
-const approval = require("../src/popup/views/approval");
-const exportPrivkey = require("../src/popup/views/exportPrivkey");
-const txStatus = require("../src/popup/views/txStatus");
-
-// What the background answers when the approval window asks which approval it
-// was opened for, and every message the popup sent it.
-let approvalDetails;
-let sent;
-
-// The "+" button of the wallet list's last render. Each render attaches the
-// handler to the buttons it drew, as the browser's fresh list would have.
-let plusButton;
-
-beforeEach(() => {
-    approvalDetails = null;
-    sent = [];
-    mockNode.broadcast = [];
-    globalThis.document = makeDocument();
-    globalThis.window = { close: () => {} };
-    globalThis.chrome = {
-        storage: { local: { get: async () => ({}), set: async () => {} } },
-        runtime: {
-            connect: () => ({ postMessage: () => {} }),
-            sendMessage: (msg, reply) => {
-                sent.push(msg);
-                if (!reply) return;
-                reply(
-                    msg.type === "AUTISTMASK_GET_APPROVAL"
-                        ? approvalDetails
-                        : null,
-                );
-            },
-        },
-    };
-    node("wallet-list").querySelectorAll = (selector) => {
-        if (selector !== ".btn-add-address") return [];
-        plusButton = makeElement("btn-add-address");
-        plusButton.dataset.wallet = "0";
-        return [plusButton];
-    };
-    // The explorer and price lookups these screens start fail offline, and
-    // say so; that is not under test.
-    for (const method of ["log", "warn", "error"]) {
-        jest.spyOn(console, method).mockImplementation(() => {});
-    }
-    txStatus.showError.mockClear();
-
-    state.wallets = [];
-    state.rpcUrl = "https://rpc.example.invalid";
-    state.networkId = "mainnet";
-    state.viewStack = [];
-    state.currentView = "main";
-
-    addWallet.init({
-        renderWalletList: () => {},
-        doRefreshAndRender: () => {},
-        pageClosed: new AbortController().signal,
-    });
-    confirmTx.init({});
-    approval.init({});
-    exportPrivkey.init();
-});
-
-afterEach(() => {
-    jest.restoreAllMocks();
-});
-
-// Import PHRASE on the add-wallet screen, with the stub node reporting a
-// balance at each of `fundedIndices`, and wait for the address scan.
-async function importPhrase(fundedIndices) {
-    mockNode.funded = new Set(fundedIndices.map((i) => derived(i).address));
-    await click(node("tab-mnemonic"));
-    node("wallet-mnemonic").value = PHRASE;
-    node("add-wallet-password").value = PASSWORD;
-    node("add-wallet-password-confirm").value = PASSWORD;
-    await click(node("btn-add-wallet-confirm"));
-}
-
-// Press the wallet's "+" button on Home.
-async function addAddress() {
-    home.render({
-        doRefreshAndRender: () => {},
-        pageClosed: new AbortController().signal,
-    });
-    await click(plusButton);
-}
-
-// The key the export screen reveals for the address at `position`.
-async function exportedKey(position) {
-    exportPrivkey.show(0, position);
-    node("export-privkey-password").value = PASSWORD;
-    await click(node("btn-export-privkey-confirm"));
-    return node("export-privkey-value").textContent;
-}
-
-// Send from the address at `position`; resolves to the signed transaction the
-// node was handed, or undefined when none was.
-async function sendFrom(position) {
-    state.selectedWallet = 0;
-    state.selectedAddress = position;
-    confirmTx.show({
-        from: state.wallets[0].addresses[position].address,
-        to: RECIPIENT,
-        amount: "0.5",
-        token: "ETH",
-        balance: "1.0",
-    });
-    await settle();
-    node("confirm-tx-password").value = PASSWORD;
-    await click(node("btn-confirm-send"));
-    return mockNode.broadcast.pop();
-}
-
-// A site asks the address at `position` to sign MESSAGE; resolves to the
-// response the approval window sent the background.
-async function signFor(position) {
-    const address = state.wallets[0].addresses[position].address;
-    approvalDetails = {
-        type: "sign",
-        origin: "https://dapp.example",
-        isPhishingDomain: false,
-        approvedFrom: address,
-        signParams: {
-            method: "personal_sign",
-            message: MESSAGE,
-            from: address,
-        },
-    };
-    await approval.show(1);
-    node("approve-sign-password").value = PASSWORD;
-    await click(node("btn-approve-sign"));
-    return sent.find((msg) => msg.type === "AUTISTMASK_SIGN_RESPONSE");
-}
-
-describe.each([
-    [
-        "an import whose scan passed over index 3",
-        async () => {
-            await importPhrase([0, 1, 2, 4]);
-        },
-        // Address 4 on Home, derived at index 4.
-        { position: 3, index: 4 },
-    ],
-    [
-        "removing the middle one of three addresses",
-        async () => {
-            await importPhrase([0]);
-            await addAddress();
-            await addAddress();
-            removeAddressFromState(state, 0, 1);
-        },
-        // Address 2 on Home after the removal, derived at index 2.
-        { position: 1, index: 2 },
-    ],
-])("after %s", (_name, setUp, { position, index }) => {
-    beforeEach(setUp);
-
-    test("the wallet lists the address derived at that index there", () => {
-        expect(state.wallets[0].addresses[position].address).toBe(
-            derived(index).address,
-        );
-    });
-
-    test("export reveals that address's key", async () => {
-        expect(await exportedKey(position)).toBe(derived(index).privateKey);
-    });
-
-    test("a send is signed by that address", async () => {
-        const signed = await sendFrom(position);
-        expect(Transaction.from(signed).from).toBe(derived(index).address);
-    });
-
-    test("a site's signature is made by that address", async () => {
-        const response = await signFor(position);
-        expect(verifyMessage(getBytes(MESSAGE), response.signature)).toBe(
-            derived(index).address,
-        );
-    });
-});
-
-describe("an address whose stored index derives a different address", () => {
-    function refusal() {
-        return (
-            "The key this wallet holds for " +
-            derived(0).address +
-            " belongs to a different address, so it was not used."
-        );
-    }
-
-    beforeEach(async () => {
-        await importPhrase([0]);
-        state.wallets[0].addresses[0].index = 1;
-    });
-
-    test("export reveals nothing and says why", async () => {
-        expect(await exportedKey(0)).toBe("");
-        expect(node("export-privkey-password-error").textContent).toBe(
-            refusal(),
-        );
-    });
-
-    test("nothing is sent", async () => {
-        expect(await sendFrom(0)).toBeUndefined();
-        expect(txStatus.showError).toHaveBeenCalledWith(
-            expect.anything(),
-            null,
-            refusal(),
-        );
-    });
-
-    test("nothing is signed", async () => {
-        const response = await signFor(0);
-        expect(response.signature).toBeUndefined();
-        expect(response.error).toBe(refusal());
-    });
-});

tests/approvalNonce.test.js

@@ -136,9 +136,7 @@ async function openTxApproval(approvedTx) {
             xpub: "xpub-wallet-1",
             encryptedSecret: "encrypted-secret-1",
             nextIndex: 1,
-            addresses: [
-                { address: FROM, index: 0, balance: "0", tokenBalances: [] },
-            ],
+            addresses: [{ address: FROM, balance: "0", tokenBalances: [] }],
         },
     ];
     approval.init({});

tests/approvalVerify.test.js

@@ -1455,11 +1455,7 @@ describe("background population to popup signing to verification", () => {
 
     // What the popup does with it: signs it as given, populating nothing.
     async function popupSigns(approvedTx) {
-        const localSigner = getSignerForAddress(
-            walletData,
-            { address: signer.address },
-            SIGNER_KEY,
-        );
+        const localSigner = getSignerForAddress(walletData, 0, SIGNER_KEY);
         return localSigner.signTransaction({ ...approvedTx });
     }
 
@@ -1554,11 +1550,7 @@ describe("background population to popup signing to verification", () => {
     // address the user has switched to.
     test("the approved from stops the popup signing with another key", async () => {
         const approvedTx = await backgroundPrepares(TX_PARAMS);
-        const otherSigner = getSignerForAddress(
-            walletData,
-            { address: other.address },
-            OTHER_KEY,
-        );
+        const otherSigner = getSignerForAddress(walletData, 0, OTHER_KEY);
         await expect(
             otherSigner.signTransaction({ ...approvedTx }),
         ).rejects.toThrow(/from address mismatch/);

tests/backgroundApproval.test.js

@@ -397,7 +397,13 @@ function loadBackground(options) {
                 rpcResult = r;
             },
         );
-        return { result: () => rpcResult };
+        return {
+            id: () =>
+                new URL(created[created.length - 1].url).searchParams.get(
+                    "approval",
+                ),
+            result: () => rpcResult,
+        };
     }
 
     // A dApp asking to connect. The origin defaults to one the persisted
@@ -1484,6 +1490,55 @@ describe("what the approval is verified against", () => {
         });
     });
 
+    // Typed data whose domain names another chain is refused on the sign
+    // screen; a signature for it that reaches the background anyway is not
+    // handed to the page (https://git.eeqj.de/sneak/AutistMask/issues/519).
+    // TYPED_DATA names mainnet.
+    async function answerTypedData(bg, pending) {
+        const { domain, types, message } = JSON.parse(TYPED_DATA);
+        delete types.EIP712Domain;
+        const signature = await signer.signTypedData(domain, types, message);
+        const answer = bg.send(
+            {
+                type: "AUTISTMASK_SIGN_RESPONSE",
+                id: pending.id(),
+                approved: true,
+                signature,
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        return { answer, signature };
+    }
+
+    test("typed data for the active chain is handed back signed", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTypedData();
+        await settle();
+
+        const { answer, signature } = await answerTypedData(bg, pending);
+
+        expect(answer.sendResponse).toHaveBeenCalledWith({ signature });
+        expect(pending.result()).toEqual({ result: signature });
+    });
+
+    test("typed data for another chain is refused, not handed back", async () => {
+        const bg = loadBackground();
+        bg.setNetwork(SEPOLIA);
+        const pending = bg.requestTypedData();
+        await settle();
+
+        const { answer } = await answerTypedData(bg, pending);
+
+        const refusal =
+            "This typed data is for chain id 1, but the active network is Sepolia Testnet (chain id 11155111), so it cannot be signed.";
+        expect(answer.sendResponse).toHaveBeenCalledWith({
+            error: refusal,
+            retryable: false,
+        });
+        expect(pending.result()).toEqual({ error: { message: refusal } });
+    });
+
     test("a sign request from another address raises no approval at all", async () => {
         const bg = loadBackground();
         const pending = bg.requestSign(other.address);

tests/e2e/firefox/run.js

@@ -128,18 +128,6 @@ step("the popup is drawn in the monospace font it declares", async (env) => {
     );
 });
 
-// The same check as the Chrome suite's (#456), so both browsers are held to
-// the same size.
-step("the popup's text is drawn at the 14px text-sm declares", async (env) => {
-    const size = await env.driver.execute(
-        "return getComputedStyle(document.body).fontSize;",
-    );
-    assert(
-        size === "14px",
-        "the popup's text is drawn at " + size + ", not at the 14px of text-sm",
-    );
-});
-
 // The recovery screen (#361): the Chrome suite's four cases, run before any
 // wallet exists for the same reason. With no wallet nothing saves on a timer,
 // so no save can write a good record over the unreadable one. The last of them

tests/e2e/run.js

@@ -113,18 +113,6 @@ test("the popup is drawn in the monospace font it declares (#418)", async (env)
     );
 });
 
-// The same stylesheet sets the size on body to 75%, which would draw the
-// popup at 12px.
-test("the popup's text is drawn at the 14px text-sm declares (#456)", async (env) => {
-    const size = await env.page.evaluate(
-        () => getComputedStyle(document.body).fontSize,
-    );
-    assert(
-        size === "14px",
-        "the popup's text is drawn at " + size + ", not at the 14px of text-sm",
-    );
-});
-
 // The empirical half of #182. The manifest change is only a claim about
 // what the CSP permits; this is the observation. Two things have to hold
 // together, and the run covers both: the popup realm compiles WASM (here),

tests/typedDataPermit.test.js

@@ -240,6 +240,7 @@ function request(data) {
 beforeEach(() => {
     state.trackedTokens = [];
     state.wallets = [];
+    state.networkId = "mainnet";
 });
 
 describe("a token permission is warned about, naming spender and amount", () => {
@@ -532,3 +533,66 @@ describe("the sign screen refuses a mismatched primary type", () => {
         expect(node("btn-approve-sign").disabled).toBe(true);
     });
 });
+
+// EIP-712: a wallet refuses typed data whose domain names a chain other than
+// the active one, because the signature is valid on the chain it names
+// (https://git.eeqj.de/sneak/AutistMask/issues/519). The wallet is on Sepolia
+// and the site asks for a mainnet USDC Permit.
+describe("the sign screen refuses typed data for another chain", () => {
+    const REFUSAL =
+        "This typed data is for chain id 1, but the active network is Sepolia Testnet (chain id 11155111), so it cannot be signed.";
+
+    function withChainId(chainId) {
+        return { ...PERMIT, domain: { ...PERMIT.domain, chainId } };
+    }
+
+    beforeEach(() => {
+        decryptWithPassword.mockClear();
+        state.networkId = "sepolia";
+    });
+
+    test("another chain shows the error, with Sign disabled", async () => {
+        await openSignScreen(PERMIT);
+        expect(node("approve-sign-error").textContent).toBe(REFUSAL);
+        expect(node("btn-approve-sign").disabled).toBe(true);
+    });
+
+    test("Sign clicked anyway decrypts and signs nothing", async () => {
+        const sent = await openSignScreen(PERMIT);
+        node("approve-sign-password").value = "any password";
+        await click("btn-approve-sign");
+
+        expect(decryptWithPassword).not.toHaveBeenCalled();
+        expect(sent.map((msg) => msg.type)).not.toContain(
+            "AUTISTMASK_SIGN_RESPONSE",
+        );
+        expect(node("approve-sign-error").textContent).toBe(REFUSAL);
+    });
+
+    // ethers signs 1, "1" and "0x1" as the same chain id.
+    test("the chain id is refused in each form ethers signs", () => {
+        for (const chainId of [1, "1", "0x1"]) {
+            expect(typedDataRefusal(request(withChainId(chainId)))).toBe(
+                REFUSAL,
+            );
+        }
+    });
+
+    test("a chain id that is not a number is refused", () => {
+        expect(typedDataRefusal(request(withChainId("mainnet")))).toBe(
+            "This typed data cannot be read, so it cannot be signed.",
+        );
+    });
+
+    test("the active chain leaves Sign enabled", async () => {
+        await openSignScreen(withChainId("0xaa36a7"));
+        expect(node("approve-sign-error").textContent).toBe("");
+        expect(node("btn-approve-sign").disabled).toBe(false);
+    });
+
+    test("no chain id leaves Sign enabled", async () => {
+        await openSignScreen(withChainId(undefined));
+        expect(node("approve-sign-error").textContent).toBe("");
+        expect(node("btn-approve-sign").disabled).toBe(false);
+    });
+});

tests/wallet.test.js

@@ -416,7 +416,7 @@ describe("extended key depth", () => {
         expect(() =>
             wallet.getSignerForAddress(
                 { type: "xprv" },
-                { address: VECTOR_ADDRESSES[0], index: 0 },
+                0,
                 accountXprv(VECTOR_PHRASE),
             ),
         ).toThrow(/master/i);
@@ -504,7 +504,7 @@ describe("getSignerForAddress", () => {
     test.each([0, 1, 2])("hd wallet, address index %i", (index) => {
         const signer = wallet.getSignerForAddress(
             { type: "hd" },
-            { address: VECTOR_ADDRESSES[index], index },
+            index,
             VECTOR_PHRASE,
         );
 
@@ -515,7 +515,7 @@ describe("getSignerForAddress", () => {
     test.each([0, 1, 2])("xprv wallet, address index %i", (index) => {
         const signer = wallet.getSignerForAddress(
             { type: "xprv" },
-            { address: VECTOR_ADDRESSES[index], index },
+            index,
             masterXprv(VECTOR_PHRASE),
         );
 
@@ -527,7 +527,7 @@ describe("getSignerForAddress", () => {
         for (const index of [0, 1, 2]) {
             const signer = wallet.getSignerForAddress(
                 { type: "privkey" },
-                { address: VECTOR_ADDRESSES[1], index },
+                index,
                 VECTOR_PRIVATE_KEYS[1],
             );
 
@@ -535,33 +535,10 @@ describe("getSignerForAddress", () => {
         }
     });
 
-    // https://git.eeqj.de/sneak/AutistMask/issues/510: an address whose stored
-    // index does not derive it, or a private key that is not its key.
-    test.each([
-        ["hd", { address: VECTOR_ADDRESSES[2], index: 1 }, VECTOR_PHRASE],
-        [
-            "xprv",
-            { address: VECTOR_ADDRESSES[2], index: 1 },
-            masterXprv(VECTOR_PHRASE),
-        ],
-        ["privkey", { address: VECTOR_ADDRESSES[2] }, VECTOR_PRIVATE_KEYS[1]],
-    ])(
-        "%s wallet refuses a key that is not the address's own",
-        (type, addr, secret) => {
-            expect(() =>
-                wallet.getSignerForAddress({ type }, addr, secret),
-            ).toThrow(
-                "The key this wallet holds for " +
-                    VECTOR_ADDRESSES[2] +
-                    " belongs to a different address, so it was not used.",
-            );
-        },
-    );
-
     test("the returned signer signs recoverably as the expected address", async () => {
         const signer = wallet.getSignerForAddress(
             { type: "hd" },
-            { address: VECTOR_ADDRESSES[1], index: 1 },
+            1,
             VECTOR_PHRASE,
         );
         const message = "AutistMask derivation test";

tests/walletDefects.test.js

@@ -55,7 +55,7 @@ function healthyXprvWallet(name = "Master") {
         xpub,
         nextIndex: 1,
         encryptedSecret: "irrelevant-to-these-tests",
-        addresses: [{ address: firstAddress, index: 0, balance: "0.0000" }],
+        addresses: [{ address: firstAddress, balance: "0.0000" }],
     };
 }
 
@@ -558,7 +558,7 @@ describe("no path throws an unhandled error for a defective wallet", () => {
         try {
             wallet.getSignerForAddress(
                 { type: "xprv" },
-                brokenXprvWallet().addresses[0],
+                0,
                 accountXprv(VECTOR_PHRASE),
             );
         } catch (e) {
@@ -573,7 +573,7 @@ describe("no path throws an unhandled error for a defective wallet", () => {
     test("a healthy xprv wallet signs as it always did", () => {
         const signer = wallet.getSignerForAddress(
             { type: "xprv" },
-            healthyXprvWallet().addresses[0],
+            0,
             masterXprv(VECTOR_PHRASE),
         );