diff --git a/README.md b/README.md index 90a7de1..4e164c3 100644 --- a/README.md +++ b/README.md @@ -1840,7 +1840,9 @@ view would leave a wallet one click from deletion. `wallet_requestPermissions` and is on neither the allowed nor the denied list. The background script prefers the toolbar popup (`action.openPopup()`) and falls back to a separate popup window (`src/background/index.js`, - `requestApproval()`). + `requestApproval()`). Only one exists per site at a time: a further connection + request from a site whose prompt is still unanswered is refused with EIP-1193 + code `-32002` and opens nothing. - **Elements**: - "Connection Request" heading - Phishing warning banner (shown when the hostname is on the phishing @@ -1901,7 +1903,10 @@ view would leave a wallet one click from deletion. - **When**: A connected website requests a message signature via `personal_sign`, `eth_sign`, or `eth_signTypedData_v4`. Opened the same way as - TxApproval, in a separate popup window. + TxApproval, in a separate popup window. Only one exists per site at a time: a + further signature request, by any of these methods, from a site whose + signature request is still unanswered is refused with EIP-1193 code `-32002` + and opens no window. - **Elements**: - "Signature Request" heading - Phishing warning banner (shown when the hostname is on the phishing diff --git a/TODO.md b/TODO.md index 78cc1a4..84efd54 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,16 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-04: A site has at most one connection prompt and one signature prompt + open at a time ([#405](https://git.eeqj.de/sneak/AutistMask/issues/405)). Each + `eth_requestAccounts` or `personal_sign` call opened another approval window, + so a page calling in a loop could cover the screen with identical prompts. A + further request of the same kind from a site whose prompt is still unanswered + is now refused with EIP-1193 `-32002`, the code a second transaction already + gets, and opens no window. Signing by `personal_sign`, `eth_sign` and + `eth_signTypedData_v4` counts as one kind. Other sites are not affected, and + the site may ask again once the user has answered. + - 2026-10-04: A page's request is credited only to the site the browser says sent it ([#407](https://git.eeqj.de/sneak/AutistMask/issues/407)). Where the browser does not give the sender's origin (Firefox before 126), the background diff --git a/src/background/index.js b/src/background/index.js index 649b527..696e22f 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -83,7 +83,8 @@ const pendingApprovals = {}; // authority on a nonce the network has not accepted, which an abandoned // approval then leaves a hole in. // -// Sign approvals are not gated: a signature consumes no nonce. +// Sign approvals do not take this slot: a signature consumes no nonce. They are +// limited per site instead; see hasPendingApproval(). // // The slot is null when free, and otherwise the handle of the request holding // it. Once that request has raised its approval the handle carries the @@ -95,7 +96,7 @@ let txApprovalSlot = null; // EIP-1474 "resource unavailable": the standard code for a request that is // refused because another one is already pending. -const TX_APPROVAL_PENDING_CODE = -32002; +const APPROVAL_PENDING_CODE = -32002; // True at every moment this can be sent: the slot is taken immediately before // the transaction is populated, so the other request is either being prepared @@ -132,6 +133,22 @@ function releaseTxApprovalSlotFor(approvalId) { } } +// One site-connection approval and one sign approval per site at a time: a +// page that asks again before the user has answered is refused with the code +// above instead of opening another window, so it cannot bury the user in +// prompts. The pending approval itself holds the place, so a caller must test +// this and raise its approval with nothing awaited in between. +function hasPendingApproval(origin, type) { + return Object.values(pendingApprovals).some( + (approval) => approval.origin === origin && approval.type === type, + ); +} + +const APPROVAL_PENDING_MESSAGE = + "AutistMask is already waiting for your answer to a request of this kind" + + " from this site, so this one was not shown. Please answer that one," + + " then send this one again."; + // Nonces this worker has already handed to the node, per chain and address. // This is the wallet's own knowledge that a nonce is spent, and it is checked // before a broadcast rather than after: a node's pending count can lag a @@ -462,7 +479,7 @@ async function openApprovalWindow(id) { function requestApproval(origin, hostname) { return new Promise((resolve) => { const id = crypto.randomUUID(); - pendingApprovals[id] = { id, origin, hostname, resolve }; + pendingApprovals[id] = { id, origin, hostname, resolve, type: "site" }; if (actionNs && typeof actionNs.openPopup === "function") { actionNs.setPopup({ @@ -640,6 +657,15 @@ async function handleConnectionRequest(origin) { return { result: [activeAddress] }; } + if (hasPendingApproval(origin, "site")) { + return { + error: { + code: APPROVAL_PENDING_CODE, + message: APPROVAL_PENDING_MESSAGE, + }, + }; + } + // Open approval popup const decision = await requestApproval(origin, hostname); @@ -868,6 +894,14 @@ async function handleRpc(method, params, origin) { "Only proceed if you fully understand what you are signing."; } + if (hasPendingApproval(origin, "sign")) { + return { + error: { + code: APPROVAL_PENDING_CODE, + message: APPROVAL_PENDING_MESSAGE, + }, + }; + } const decision = await requestSignApproval( origin, hostname, @@ -903,6 +937,14 @@ async function handleRpc(method, params, origin) { }, }; } + if (hasPendingApproval(origin, "sign")) { + return { + error: { + code: APPROVAL_PENDING_CODE, + message: APPROVAL_PENDING_MESSAGE, + }, + }; + } const decision = await requestSignApproval( origin, hostname, @@ -976,7 +1018,7 @@ async function handleSendTransaction(params, origin) { if (!slot) { return { error: { - code: TX_APPROVAL_PENDING_CODE, + code: APPROVAL_PENDING_CODE, message: TX_APPROVAL_PENDING_MESSAGE, }, }; diff --git a/tests/backgroundApproval.test.js b/tests/backgroundApproval.test.js index fe430f0..1486f3a 100644 --- a/tests/backgroundApproval.test.js +++ b/tests/backgroundApproval.test.js @@ -331,7 +331,7 @@ function loadBackground(options) { // The same for a message-signing approval, which pins the signing address // at approval time in exactly the same way. - function requestSign(from) { + function requestSign(from, origin) { let rpcResult = null; messageListener( { @@ -339,7 +339,7 @@ function loadBackground(options) { method: "personal_sign", params: [MESSAGE, from || signer.address], }, - { origin: ORIGIN }, + { origin: origin || ORIGIN }, (r) => { rpcResult = r; }, @@ -822,6 +822,87 @@ describe("one transaction approval at a time", () => { }); }); +// A page that asks again before the user has answered its last connection or +// signature request is refused, instead of opening one more window per call. +describe("one connection and one signature approval per site at a time", () => { + const PENDING_REFUSAL = { + error: { + code: -32002, + message: expect.stringMatching(/already waiting for your answer/), + }, + }; + + test("a loop of eth_requestAccounts opens one approval and refuses the rest", async () => { + const bg = loadBackground(); + const requests = []; + for (let i = 0; i < 5; i++) requests.push(bg.requestSite()); + await settle(); + + expect(bg.created).toHaveLength(1); + expect(requests[0].result()).toBeNull(); + for (const extra of requests.slice(1)) { + expect(extra.result()).toEqual(PENDING_REFUSAL); + } + + // Once the user has answered, the site may ask again. + bg.closeWindow(1); + await settle(); + expect(requests[0].result()).toEqual({ + error: { code: 4001, message: "User rejected the request." }, + }); + const again = bg.requestSite(); + await settle(); + expect(again.result()).toBeNull(); + expect(bg.created).toHaveLength(2); + }); + + test("a loop of personal_sign opens one approval and refuses the rest", async () => { + const bg = loadBackground(); + const requests = []; + for (let i = 0; i < 5; i++) requests.push(bg.requestSign()); + await settle(); + + expect(bg.created).toHaveLength(1); + expect(requests[0].result()).toBeNull(); + for (const extra of requests.slice(1)) { + expect(extra.result()).toEqual(PENDING_REFUSAL); + } + }); + + test("another site's connection request is not held up", async () => { + const bg = loadBackground(); + bg.requestSite(); + const repeat = bg.requestSite(); + const other = bg.requestSite(UNCONNECTED_ORIGIN); + await settle(); + + expect(repeat.result()).toEqual(PENDING_REFUSAL); + expect(other.result()).toBeNull(); + expect(bg.created).toHaveLength(2); + }); + + test("another site's signature request is not held up", async () => { + const bg = loadBackground(); + // Connect a second site, so that it may ask for a signature at all. + const connecting = bg.requestSite(); + await settle(); + const port = bg.connectApproval(connecting.id()); + port.decide(true, false); + port.disconnect(); + await settle(); + expect(connecting.result()).toEqual({ result: [signer.address] }); + + bg.requestSign(); + const repeat = bg.requestSign(); + const other = bg.requestSign(signer.address, FRESH_ORIGIN); + await settle(); + + expect(repeat.result()).toEqual(PENDING_REFUSAL); + expect(other.result()).toBeNull(); + expect(bg.created).toHaveLength(3); + }); +}); + // A nonce collision found before the transaction reaches the network is the // one send failure the wallet can speak about with certainty. The user is told // it did not go out and to send it again, rather than being warned it might