harden: ignore a nonce the page supplies with eth_sendTransaction (closes #404)

A site could fix the nonce of the transaction the user was asked to
sign: the same nonce as a pending transaction, at a higher fee,
replaces it, and a nonce above the account's next one leaves the new
transaction stuck behind a gap. `nonce` is no longer one of the fields
taken from the request, so the transaction always gets the account's
next nonce from the network, and that is the nonce the approval screen
shows and the popup signs.

Model: opus-5-5

README.md

@@ -1869,13 +1869,16 @@ view would leave a wallet one click from deletion.
   programmatically rather than by a user gesture. The background populates the
   transaction (nonce, gas limit, fees, chain id) against the RPC node _before_
   opening the window, so the screen shows a complete transaction and the signed
-  artifact can be compared with it field for field. A request that cannot be
-  populated — unreachable node, reverting gas estimate — opens no window and is
-  failed back to the site. Only one transaction approval exists at a time:
-  populating fixes the nonce, so a second `eth_sendTransaction` arriving while
-  one is unanswered is refused with EIP-1193 code `-32002` rather than being
-  populated at the same nonce. It opens no window and takes no nonce, and the
-  site can send it again once the pending one is answered.
+  artifact can be compared with it field for field. A nonce the site supplies is
+  ignored: the nonce is always the account's next nonce from the node, so a site
+  cannot replace one of the user's pending transactions or leave this one stuck
+  behind a gap. A request that cannot be populated — unreachable node, reverting
+  gas estimate — opens no window and is failed back to the site. Only one
+  transaction approval exists at a time: populating fixes the nonce, so a second
+  `eth_sendTransaction` arriving while one is unanswered is refused with
+  EIP-1193 code `-32002` rather than being populated at the same nonce. It opens
+  no window and takes no nonce, and the site can send it again once the pending
+  one is answered.
 - **Elements**:
     - "Transaction Request" heading
     - Phishing warning banner (shown when the hostname is on the phishing

TODO.md

@@ -45,6 +45,15 @@ but the review is broader than any of them.
 
 # Completed Steps
 
+- 2026-10-04: A nonce the site supplies with `eth_sendTransaction` is ignored
+  ([#404](https://git.eeqj.de/sneak/AutistMask/issues/404)). It was passed on to
+  the transaction, so a site could replace one of the user's pending
+  transactions (same nonce, higher fee) or leave the new one stuck behind a gap,
+  and the approval screen showed it as a bare number. `nonce` is no longer one
+  of the fields taken from the request in `src/shared/approvalTx.js`, so the
+  transaction always gets the account's next nonce from the node, and that is
+  the nonce the approval screen shows and the popup signs.
+
 - 2026-10-04: Remembered site permissions are held by full origin
   ([#402](https://git.eeqj.de/sneak/AutistMask/issues/402)). `allowedSites` and
   `deniedSites` stored the hostname alone, so a grant to `https://dapp.example`

src/shared/approvalTx.js

@@ -51,11 +51,15 @@ const POPULATE_TIMEOUT_MS = 20000;
 // passed to ethers: the object is page-controlled, and a future ethers that
 // learns to carry a new transaction field must not start picking one up out of
 // it without this module knowing.
+//
+// The nonce is not taken from the page; it is always the account's next nonce
+// from the network. A page that chose it could replace one of the user's
+// pending transactions (the same nonce at a higher fee) or leave this one stuck
+// behind a gap (a nonce above the next one).
 const REQUEST_FIELDS = [
     "to",
     "value",
     "data",
-    "nonce",
     "gasLimit",
     "gasPrice",
     "maxFeePerGas",

tests/approvalNonce.test.js

@@ -0,0 +1,167 @@
+// A nonce the page supplies is not used
+// (https://git.eeqj.de/sneak/AutistMask/issues/404). With it a page could
+// replace one of the user's pending transactions (the same nonce at a higher
+// fee) or leave the new one stuck behind a gap, so the transaction is always
+// given the account's next nonce from the network.
+//
+// Driven through the preparation the background runs on a page's
+// eth_sendTransaction (src/shared/approvalTx.js) and the real approval screen,
+// password and Confirm included, against a minimal DOM stub in the shape
+// tests/approvalOrigin.test.js uses. The vault is mocked so that no password
+// has to be hashed.
+
+jest.mock("../src/shared/vault", () => ({
+    decryptWithPassword: jest.fn(),
+}));
+
+globalThis.chrome = {
+    storage: { local: { get: async () => ({}), set: async () => {} } },
+};
+
+const { Network, Transaction } = require("ethers");
+const { state } = require("../src/shared/state");
+const { decryptWithPassword } = require("../src/shared/vault");
+const { prepareApprovalTx } = require("../src/shared/approvalTx");
+const approval = require("../src/popup/views/approval");
+
+// A well-known test phrase, and its first address.
+const PHRASE = "test test test test test test test test test test test junk";
+const FROM = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266";
+const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
+
+// The account's next nonce, as the network reports it.
+const NETWORK_NONCE = 7;
+
+const network = {
+    getNetwork: async () => Network.from(1),
+    getTransactionCount: async () => NETWORK_NONCE,
+    estimateGas: async () => 21000n,
+    getFeeData: async () => ({
+        gasPrice: 2000000000n,
+        maxFeePerGas: 2000000000n,
+        maxPriorityFeePerGas: 1000000000n,
+    }),
+};
+
+function makeElement(id) {
+    const classes = new Set();
+    const el = {
+        id,
+        textContent: "",
+        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"),
+        body: { prepend: () => {} },
+    };
+}
+
+function node(id) {
+    return globalThis.document.getElementById(id);
+}
+
+function click(id) {
+    return Promise.all((node(id).listeners.click || []).map((fn) => fn()));
+}
+
+// Open the transaction approval screen the way the popup does: the background
+// hands over the populated transaction and show() draws it. Returns every
+// message the screen sends to the background. The background's answer to the
+// signed transaction does not matter here; a retryable refusal keeps the
+// screen where it is.
+async function openTxApproval(approvedTx) {
+    const sent = [];
+    globalThis.document = makeDocument();
+    globalThis.window = { location: { search: "" }, close: () => {} };
+    globalThis.chrome.runtime = {
+        connect: () => ({ postMessage: () => {} }),
+        sendMessage: (msg, reply) => {
+            sent.push(msg);
+            if (!reply) return;
+            if (msg.type !== "AUTISTMASK_GET_APPROVAL") {
+                return reply({ error: "Not sent.", retryable: true });
+            }
+            reply({
+                type: "tx",
+                origin: "https://dapp.example",
+                isPhishingDomain: false,
+                approvedFrom: FROM,
+                approvedTx,
+            });
+        },
+    };
+    state.activeAddress = FROM;
+    state.wallets = [
+        {
+            type: "hd",
+            name: "Wallet 1",
+            xpub: "xpub-wallet-1",
+            encryptedSecret: "encrypted-secret-1",
+            nextIndex: 1,
+            addresses: [{ address: FROM, balance: "0", tokenBalances: [] }],
+        },
+    ];
+    approval.init({});
+    await approval.show(1);
+    return sent;
+}
+
+test("a page's nonce is replaced by the network's, on screen and in the signed transaction", async () => {
+    // What the background does with the page's request before it opens the
+    // approval window.
+    const approvedTx = await prepareApprovalTx(network, FROM, {
+        from: FROM,
+        to: RECIPIENT,
+        value: "0x0",
+        data: "0x",
+        nonce: "0x2",
+    });
+
+    const sent = await openTxApproval(approvedTx);
+    expect(node("approve-tx-nonce").textContent).toBe("7");
+
+    decryptWithPassword.mockResolvedValue(PHRASE);
+    node("approve-tx-password").value = "any password";
+    await click("btn-approve-tx");
+
+    const response = sent.find((msg) => msg.type === "AUTISTMASK_TX_RESPONSE");
+    expect(Transaction.from(response.rawSignedTx).nonce).toBe(NETWORK_NONCE);
+});

tests/approvalTx.test.js

@@ -121,7 +121,7 @@ describe("prepareApprovalTx", () => {
         );
     });
 
-    test("keeps a nonce, gas limit and fee the request did fix", async () => {
+    test("keeps a gas limit and fee the request did fix, but not its nonce", async () => {
         const approved = await prepareApprovalTx(
             providerWith(),
             signer.address,
@@ -133,7 +133,7 @@ describe("prepareApprovalTx", () => {
                 maxPriorityFeePerGas: "0x3b9aca00",
             },
         );
-        expect(approved.nonce).toBe("0x2");
+        expect(approved.nonce).toBe("0x7");
         expect(approved.gasLimit).toBe("0x30d40");
         expect(approved.maxFeePerGas).toBe("0x12a05f200");
     });