harden: ignore a nonce the page supplies with eth_sendTransaction #434
@@ -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
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user