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, the code a second transaction already gets, and opens no window. The pending approval itself is the record: the check and the approval's creation have nothing awaited between them. Other sites are unaffected, and the site may ask again once the user answers. Model: opus-5-5
This commit is contained in:
@@ -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.
|
`wallet_requestPermissions` and is on neither the allowed nor the denied list.
|
||||||
The background script prefers the toolbar popup (`action.openPopup()`) and
|
The background script prefers the toolbar popup (`action.openPopup()`) and
|
||||||
falls back to a separate popup window (`src/background/index.js`,
|
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**:
|
- **Elements**:
|
||||||
- "Connection Request" heading
|
- "Connection Request" heading
|
||||||
- Phishing warning banner (shown when the hostname is on the phishing
|
- 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
|
- **When**: A connected website requests a message signature via
|
||||||
`personal_sign`, `eth_sign`, or `eth_signTypedData_v4`. Opened the same way as
|
`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**:
|
- **Elements**:
|
||||||
- "Signature Request" heading
|
- "Signature Request" heading
|
||||||
- Phishing warning banner (shown when the hostname is on the phishing
|
- Phishing warning banner (shown when the hostname is on the phishing
|
||||||
|
|||||||
@@ -45,6 +45,16 @@ but the review is broader than any of them.
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 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
|
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
|
browser does not give the sender's origin (Firefox before 126), the background
|
||||||
|
|||||||
+46
-4
@@ -83,7 +83,8 @@ const pendingApprovals = {};
|
|||||||
// authority on a nonce the network has not accepted, which an abandoned
|
// authority on a nonce the network has not accepted, which an abandoned
|
||||||
// approval then leaves a hole in.
|
// 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
|
// 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
|
// 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
|
// EIP-1474 "resource unavailable": the standard code for a request that is
|
||||||
// refused because another one is already pending.
|
// 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
|
// 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
|
// 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.
|
// 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
|
// 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
|
// 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) {
|
function requestApproval(origin, hostname) {
|
||||||
return new Promise((resolve) => {
|
return new Promise((resolve) => {
|
||||||
const id = crypto.randomUUID();
|
const id = crypto.randomUUID();
|
||||||
pendingApprovals[id] = { id, origin, hostname, resolve };
|
pendingApprovals[id] = { id, origin, hostname, resolve, type: "site" };
|
||||||
|
|
||||||
if (actionNs && typeof actionNs.openPopup === "function") {
|
if (actionNs && typeof actionNs.openPopup === "function") {
|
||||||
actionNs.setPopup({
|
actionNs.setPopup({
|
||||||
@@ -640,6 +657,15 @@ async function handleConnectionRequest(origin) {
|
|||||||
return { result: [activeAddress] };
|
return { result: [activeAddress] };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (hasPendingApproval(origin, "site")) {
|
||||||
|
return {
|
||||||
|
error: {
|
||||||
|
code: APPROVAL_PENDING_CODE,
|
||||||
|
message: APPROVAL_PENDING_MESSAGE,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
// Open approval popup
|
// Open approval popup
|
||||||
const decision = await requestApproval(origin, hostname);
|
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.";
|
"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(
|
const decision = await requestSignApproval(
|
||||||
origin,
|
origin,
|
||||||
hostname,
|
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(
|
const decision = await requestSignApproval(
|
||||||
origin,
|
origin,
|
||||||
hostname,
|
hostname,
|
||||||
@@ -976,7 +1018,7 @@ async function handleSendTransaction(params, origin) {
|
|||||||
if (!slot) {
|
if (!slot) {
|
||||||
return {
|
return {
|
||||||
error: {
|
error: {
|
||||||
code: TX_APPROVAL_PENDING_CODE,
|
code: APPROVAL_PENDING_CODE,
|
||||||
message: TX_APPROVAL_PENDING_MESSAGE,
|
message: TX_APPROVAL_PENDING_MESSAGE,
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -331,7 +331,7 @@ function loadBackground(options) {
|
|||||||
|
|
||||||
// The same for a message-signing approval, which pins the signing address
|
// The same for a message-signing approval, which pins the signing address
|
||||||
// at approval time in exactly the same way.
|
// at approval time in exactly the same way.
|
||||||
function requestSign(from) {
|
function requestSign(from, origin) {
|
||||||
let rpcResult = null;
|
let rpcResult = null;
|
||||||
messageListener(
|
messageListener(
|
||||||
{
|
{
|
||||||
@@ -339,7 +339,7 @@ function loadBackground(options) {
|
|||||||
method: "personal_sign",
|
method: "personal_sign",
|
||||||
params: [MESSAGE, from || signer.address],
|
params: [MESSAGE, from || signer.address],
|
||||||
},
|
},
|
||||||
{ origin: ORIGIN },
|
{ origin: origin || ORIGIN },
|
||||||
(r) => {
|
(r) => {
|
||||||
rpcResult = 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
|
// 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
|
// 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
|
// it did not go out and to send it again, rather than being warned it might
|
||||||
|
|||||||
Reference in New Issue
Block a user