harden: one connection and one signature prompt per site at a time (closes #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. While a site's connection or signature prompt is unanswered, a further request of that kind from the same site is now refused with EIP-1193 -32002 and opens no window; all signing methods count as one kind. A connection prompt whose toolbar popup closed before it connected, and which the toolbar popup no longer opens, is shown again by the site's next request instead of refusing the site until the address changes. Model: opus-5-5
This commit is contained in:
@@ -1844,7 +1844,11 @@ view would leave a wallet one click from deletion.
|
||||
says nothing about `http://dapp.example` or another port of that host. 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 no new prompt. If that prompt was in a toolbar popup
|
||||
that closed before it connected, and the toolbar popup has since been set to
|
||||
open something else, the refused request shows that prompt again.
|
||||
- **Elements**:
|
||||
- "Connection Request" heading
|
||||
- Phishing warning banner (shown when the hostname is on the phishing
|
||||
@@ -1907,7 +1911,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
|
||||
|
||||
@@ -45,6 +45,18 @@ 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. A connection prompt whose
|
||||
toolbar popup closed before it connected, and which nothing shows any more, is
|
||||
shown again when the site asks again.
|
||||
|
||||
- 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`
|
||||
|
||||
+78
-8
@@ -87,7 +87,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 findPendingApproval().
|
||||
//
|
||||
// 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
|
||||
@@ -99,7 +100,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
|
||||
@@ -136,6 +137,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 findPendingApproval(origin, type) {
|
||||
return Object.values(pendingApprovals).find(
|
||||
(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
|
||||
@@ -288,7 +305,12 @@ async function proxyRpc(method, params) {
|
||||
return json.result;
|
||||
}
|
||||
|
||||
// The site-connection approval the toolbar popup is set to open, or null while
|
||||
// it opens the wallet. Set only by resetPopupUrl() and showInToolbarPopup().
|
||||
let toolbarPopupApprovalId = null;
|
||||
|
||||
function resetPopupUrl() {
|
||||
toolbarPopupApprovalId = null;
|
||||
if (actionNs && typeof actionNs.setPopup === "function") {
|
||||
actionNs.setPopup({ popup: "src/popup/index.html" });
|
||||
}
|
||||
@@ -466,9 +488,20 @@ async function openApprovalWindow(id) {
|
||||
function requestApproval(origin) {
|
||||
return new Promise((resolve) => {
|
||||
const id = crypto.randomUUID();
|
||||
pendingApprovals[id] = { id, origin, resolve };
|
||||
pendingApprovals[id] = { id, origin, resolve, type: "site" };
|
||||
|
||||
if (actionNs && typeof actionNs.openPopup === "function") {
|
||||
showInToolbarPopup(id);
|
||||
} else {
|
||||
openApprovalWindow(id);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Show a site-connection approval in the toolbar popup, or in a separate popup
|
||||
// window when the browser will not open the toolbar popup.
|
||||
function showInToolbarPopup(id) {
|
||||
toolbarPopupApprovalId = id;
|
||||
actionNs.setPopup({
|
||||
popup: "src/popup/index.html?approval=" + id,
|
||||
});
|
||||
@@ -480,10 +513,6 @@ function requestApproval(origin) {
|
||||
} catch {
|
||||
openApprovalWindow(id);
|
||||
}
|
||||
} else {
|
||||
openApprovalWindow(id);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Open a tx-approval popup and return a promise that resolves with txHash or error.
|
||||
@@ -641,6 +670,31 @@ async function handleConnectionRequest(origin) {
|
||||
return { result: [activeAddress] };
|
||||
}
|
||||
|
||||
const pending = findPendingApproval(origin, "site");
|
||||
if (pending) {
|
||||
// A toolbar popup that closed before it connected leaves its prompt
|
||||
// pending, and once the toolbar popup is set to open something else
|
||||
// nothing shows that prompt: the site would be refused until the
|
||||
// address changed. Show it again. A prompt in a window or in a
|
||||
// connected popup is settled when that closes, and one the toolbar
|
||||
// popup is still set to open is a click away, so those are left alone.
|
||||
if (
|
||||
actionNs &&
|
||||
typeof actionNs.openPopup === "function" &&
|
||||
!pending.windowId &&
|
||||
!pending.portConnected &&
|
||||
toolbarPopupApprovalId !== pending.id
|
||||
) {
|
||||
showInToolbarPopup(pending.id);
|
||||
}
|
||||
return {
|
||||
error: {
|
||||
code: APPROVAL_PENDING_CODE,
|
||||
message: APPROVAL_PENDING_MESSAGE,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// Open approval popup
|
||||
const decision = await requestApproval(origin);
|
||||
|
||||
@@ -865,6 +919,14 @@ async function handleRpc(method, params, origin) {
|
||||
"Only proceed if you fully understand what you are signing.";
|
||||
}
|
||||
|
||||
if (findPendingApproval(origin, "sign")) {
|
||||
return {
|
||||
error: {
|
||||
code: APPROVAL_PENDING_CODE,
|
||||
message: APPROVAL_PENDING_MESSAGE,
|
||||
},
|
||||
};
|
||||
}
|
||||
const decision = await requestSignApproval(
|
||||
origin,
|
||||
signParams,
|
||||
@@ -898,6 +960,14 @@ async function handleRpc(method, params, origin) {
|
||||
},
|
||||
};
|
||||
}
|
||||
if (findPendingApproval(origin, "sign")) {
|
||||
return {
|
||||
error: {
|
||||
code: APPROVAL_PENDING_CODE,
|
||||
message: APPROVAL_PENDING_MESSAGE,
|
||||
},
|
||||
};
|
||||
}
|
||||
const decision = await requestSignApproval(
|
||||
origin,
|
||||
signParams,
|
||||
@@ -969,7 +1039,7 @@ async function handleSendTransaction(params, origin) {
|
||||
if (!slot) {
|
||||
return {
|
||||
error: {
|
||||
code: TX_APPROVAL_PENDING_CODE,
|
||||
code: APPROVAL_PENDING_CODE,
|
||||
message: TX_APPROVAL_PENDING_MESSAGE,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -74,6 +74,22 @@ const NONCE = 7;
|
||||
// "Hello AutistMask" as the hex string a dApp passes to personal_sign.
|
||||
const MESSAGE = "0x48656c6c6f204175746973744d61736b";
|
||||
|
||||
// An EIP-712 document as a dApp passes it to eth_signTypedData_v4. The
|
||||
// background only carries it to the approval screen, so a small one does.
|
||||
const TYPED_DATA = JSON.stringify({
|
||||
domain: { name: "AutistMask Test", version: "1", chainId: 1 },
|
||||
primaryType: "Note",
|
||||
types: {
|
||||
EIP712Domain: [
|
||||
{ name: "name", type: "string" },
|
||||
{ name: "version", type: "string" },
|
||||
{ name: "chainId", type: "uint256" },
|
||||
],
|
||||
Note: [{ name: "contents", type: "string" }],
|
||||
},
|
||||
message: { contents: "Hello AutistMask" },
|
||||
});
|
||||
|
||||
// The transaction the background populates and the approval screen displays.
|
||||
// The nonce is a parameter because the duplicate case turns on two artifacts
|
||||
// differing in a field the dApp fixed nothing for.
|
||||
@@ -229,6 +245,7 @@ function loadBackground(options) {
|
||||
// raised through action.openPopup() opens no window at all, so this is
|
||||
// the only place its id appears.
|
||||
const actionPopups = [];
|
||||
const openPopup = jest.fn(() => Promise.resolve());
|
||||
|
||||
global.chrome = {
|
||||
storage,
|
||||
@@ -283,7 +300,7 @@ function loadBackground(options) {
|
||||
// popup: no window is created, so windows.onRemoved can never
|
||||
// fire for it and the port disconnect is the only close signal
|
||||
// that exists.
|
||||
...(opts.actionPopup ? { openPopup: () => Promise.resolve() } : {}),
|
||||
...(opts.actionPopup ? { openPopup } : {}),
|
||||
},
|
||||
};
|
||||
|
||||
@@ -330,7 +347,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(
|
||||
{
|
||||
@@ -338,7 +355,7 @@ function loadBackground(options) {
|
||||
method: "personal_sign",
|
||||
params: [MESSAGE, from || signer.address],
|
||||
},
|
||||
{ origin: ORIGIN },
|
||||
{ origin: origin || ORIGIN },
|
||||
(r) => {
|
||||
rpcResult = r;
|
||||
},
|
||||
@@ -352,6 +369,24 @@ function loadBackground(options) {
|
||||
};
|
||||
}
|
||||
|
||||
// The same through eth_signTypedData_v4, whose params name the address
|
||||
// first and the typed data second.
|
||||
function requestTypedData() {
|
||||
let rpcResult = null;
|
||||
messageListener(
|
||||
{
|
||||
type: "AUTISTMASK_RPC",
|
||||
method: "eth_signTypedData_v4",
|
||||
params: [signer.address, TYPED_DATA],
|
||||
},
|
||||
{ origin: ORIGIN },
|
||||
(r) => {
|
||||
rpcResult = r;
|
||||
},
|
||||
);
|
||||
return { result: () => rpcResult };
|
||||
}
|
||||
|
||||
// A dApp asking to connect. The origin defaults to one the persisted
|
||||
// state has never allowed, so the request really does raise a prompt
|
||||
// instead of being answered from allowedSites.
|
||||
@@ -426,12 +461,15 @@ function loadBackground(options) {
|
||||
send,
|
||||
requestTx,
|
||||
requestSign,
|
||||
requestTypedData,
|
||||
requestSite,
|
||||
connectApproval,
|
||||
closeWindow,
|
||||
broadcastTransaction,
|
||||
created,
|
||||
removed,
|
||||
actionPopups,
|
||||
openPopup,
|
||||
storage,
|
||||
// The user switching account in the toolbar popup, as the background
|
||||
// sees it: the persisted active address changes underneath a pending
|
||||
@@ -821,6 +859,158 @@ 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("a loop of eth_signTypedData_v4 opens one approval and refuses the rest", async () => {
|
||||
const bg = loadBackground();
|
||||
const requests = [];
|
||||
for (let i = 0; i < 5; i++) requests.push(bg.requestTypedData());
|
||||
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("a pending personal_sign also refuses eth_signTypedData_v4", async () => {
|
||||
const bg = loadBackground();
|
||||
bg.requestSign();
|
||||
const typed = bg.requestTypedData();
|
||||
await settle();
|
||||
|
||||
expect(typed.result()).toEqual(PENDING_REFUSAL);
|
||||
expect(bg.created).toHaveLength(1);
|
||||
});
|
||||
|
||||
test("in the toolbar popup, a loop of eth_requestAccounts opens it once", async () => {
|
||||
const bg = loadBackground({ actionPopup: true });
|
||||
const requests = [];
|
||||
for (let i = 0; i < 5; i++) requests.push(bg.requestSite());
|
||||
await settle();
|
||||
|
||||
expect(bg.openPopup).toHaveBeenCalledTimes(1);
|
||||
for (const extra of requests.slice(1)) {
|
||||
expect(extra.result()).toEqual(PENDING_REFUSAL);
|
||||
}
|
||||
});
|
||||
|
||||
// A toolbar popup that closes before it connects tells the background
|
||||
// nothing, so its prompt stays pending. Once the toolbar popup has been set
|
||||
// to open something else, nothing shows that prompt, and the site asking
|
||||
// again must show it again rather than be refused for good.
|
||||
test("a toolbar prompt nothing shows any more is shown again when the site asks again", async () => {
|
||||
const bg = loadBackground({ actionPopup: true });
|
||||
const first = bg.requestSite();
|
||||
await settle();
|
||||
const id = first.id();
|
||||
|
||||
// Its popup closed without connecting. Another site's prompt takes the
|
||||
// toolbar popup and is answered, which sets it back to the wallet.
|
||||
const other = bg.requestSite(UNCONNECTED_ORIGIN);
|
||||
await settle();
|
||||
const otherPort = bg.connectApproval(other.id());
|
||||
otherPort.decide(false, false);
|
||||
otherPort.disconnect();
|
||||
await settle();
|
||||
expect(bg.actionPopups[bg.actionPopups.length - 1]).toBe(
|
||||
"src/popup/index.html",
|
||||
);
|
||||
|
||||
const repeat = bg.requestSite();
|
||||
await settle();
|
||||
expect(repeat.result()).toEqual(PENDING_REFUSAL);
|
||||
expect(bg.openPopup).toHaveBeenCalledTimes(3);
|
||||
expect(bg.actionPopups[bg.actionPopups.length - 1]).toBe(
|
||||
"src/popup/index.html?approval=" + id,
|
||||
);
|
||||
|
||||
// The user answers it, and the first request gets that answer.
|
||||
bg.connectApproval(id).decide(true, false);
|
||||
await settle();
|
||||
expect(first.result()).toEqual({ result: [signer.address] });
|
||||
});
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user