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
|
programmatically rather than by a user gesture. The background populates the
|
||||||
transaction (nonce, gas limit, fees, chain id) against the RPC node _before_
|
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
|
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
|
artifact can be compared with it field for field. A nonce the site supplies is
|
||||||
populated — unreachable node, reverting gas estimate — opens no window and is
|
ignored: the nonce is always the account's next nonce from the node, so a site
|
||||||
failed back to the site. Only one transaction approval exists at a time:
|
cannot replace one of the user's pending transactions or leave this one stuck
|
||||||
populating fixes the nonce, so a second `eth_sendTransaction` arriving while
|
behind a gap. A request that cannot be populated — unreachable node, reverting
|
||||||
one is unanswered is refused with EIP-1193 code `-32002` rather than being
|
gas estimate — opens no window and is failed back to the site. Only one
|
||||||
populated at the same nonce. It opens no window and takes no nonce, and the
|
transaction approval exists at a time: populating fixes the nonce, so a second
|
||||||
site can send it again once the pending one is answered.
|
`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**:
|
- **Elements**:
|
||||||
- "Transaction Request" heading
|
- "Transaction 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,15 @@ but the review is broader than any of them.
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 2026-10-04: Remembered site permissions are held by full origin
|
||||||
([#402](https://git.eeqj.de/sneak/AutistMask/issues/402)). `allowedSites` and
|
([#402](https://git.eeqj.de/sneak/AutistMask/issues/402)). `allowedSites` and
|
||||||
`deniedSites` stored the hostname alone, so a grant to `https://dapp.example`
|
`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
|
// 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
|
// learns to carry a new transaction field must not start picking one up out of
|
||||||
// it without this module knowing.
|
// 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 = [
|
const REQUEST_FIELDS = [
|
||||||
"to",
|
"to",
|
||||||
"value",
|
"value",
|
||||||
"data",
|
"data",
|
||||||
"nonce",
|
|
||||||
"gasLimit",
|
"gasLimit",
|
||||||
"gasPrice",
|
"gasPrice",
|
||||||
"maxFeePerGas",
|
"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(
|
const approved = await prepareApprovalTx(
|
||||||
providerWith(),
|
providerWith(),
|
||||||
signer.address,
|
signer.address,
|
||||||
@@ -133,7 +133,7 @@ describe("prepareApprovalTx", () => {
|
|||||||
maxPriorityFeePerGas: "0x3b9aca00",
|
maxPriorityFeePerGas: "0x3b9aca00",
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
expect(approved.nonce).toBe("0x2");
|
expect(approved.nonce).toBe("0x7");
|
||||||
expect(approved.gasLimit).toBe("0x30d40");
|
expect(approved.gasLimit).toBe("0x30d40");
|
||||||
expect(approved.maxFeePerGas).toBe("0x12a05f200");
|
expect(approved.maxFeePerGas).toBe("0x12a05f200");
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user