diff --git a/TODO.md b/TODO.md index 78c8ceb..7b20f1b 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,11 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-11: Approval verification extended to every consequential field — + chain id against the selected network, nonce, gas limit, both EIP-1559 fees + and the legacy gas price — with a failed signing attempt made retryable + instead of leaving a dead button + ([#174](https://git.eeqj.de/sneak/AutistMask/issues/174)). - 2026-08-11: Wallet deletion repairs its own state — `hasWallet` follows the remaining wallets, the selection only moves when it was deleted, and the active-address change is broadcast to connected sites diff --git a/src/background/index.js b/src/background/index.js index 159ed55..513bc5e 100644 --- a/src/background/index.js +++ b/src/background/index.js @@ -13,7 +13,11 @@ const { } = require("../shared/state"); const { refreshBalances, getProvider } = require("../shared/balances"); const { debugFetch } = require("../shared/log"); -const { verifySignedTx, verifySignature } = require("../shared/approvalVerify"); +const { + verifySignedTx, + verifySignature, + failureIsRetryable, +} = require("../shared/approvalVerify"); const { isPhishingDomain, updatePhishingList, @@ -100,6 +104,15 @@ function resetPopupUrl() { } } +// Retire a pending approval. Only called once the request it belongs to has +// an outcome: an approval that failed in a way the user can retry stays in +// pendingApprovals, so a second attempt signs the same approved payload +// instead of finding nothing to sign. +function finishApproval(id) { + delete pendingApprovals[id]; + resetPopupUrl(); +} + // Open approval in a separate popup window. // This is the primary mechanism for tx/sign approvals (triggered programmatically, // not from a user gesture) and the fallback for site-connection approvals. @@ -713,21 +726,20 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { if (msg.type === "AUTISTMASK_TX_RESPONSE") { const approval = pendingApprovals[msg.id]; if (!approval) return false; - delete pendingApprovals[msg.id]; - resetPopupUrl(); if (!msg.approved) { + finishApproval(msg.id); approval.resolve({ error: { code: 4001, message: "User rejected the request." }, }); return true; } - // The popup signs; it reports back here when it could not. Fail the - // request the same way this handler used to when it did the signing. + // The popup signs; it reports back here when it could not. Keep the + // approval so the user can correct the problem and try again with the + // transaction they already saw. if (msg.error) { - approval.resolve({ error: { message: msg.error } }); - sendResponse({ error: msg.error }); + sendResponse({ error: msg.error, retryable: true }); return false; } @@ -737,22 +749,42 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { const activeAddress = await getActiveAddress(); // The popup holds the secret, but the background stays the // authority on what is broadcast: the raw transaction must be - // the approved one, signed by the approved address. + // the approved one, signed by the approved address, on the + // network that is selected. verifySignedTx( msg.rawSignedTx, approval.txParams, activeAddress, + currentNetwork().chainId, ); + } catch (e) { + // A signed transaction that is not the approved one is not + // retried against that approval; it is refused outright. + // Anything else that failed before the check ran is the + // user's to retry. + const errMsg = e.shortMessage || e.message; + const retryable = failureIsRetryable(e); + if (!retryable) { + finishApproval(msg.id); + approval.resolve({ error: { message: errMsg } }); + } + sendResponse({ error: errMsg, retryable }); + return; + } + + try { const provider = getProvider(state.rpcUrl); const tx = await provider.broadcastTransaction(msg.rawSignedTx); + finishApproval(msg.id); approval.resolve({ txHash: tx.hash }); sendResponse({ txHash: tx.hash }); } catch (e) { - const errMsg = e.shortMessage || e.message; - approval.resolve({ - error: { message: errMsg }, + // The node would not take it. The approval stays pending, so + // a retry re-signs the same approved transaction. + sendResponse({ + error: e.shortMessage || e.message, + retryable: true, }); - sendResponse({ error: errMsg }); } })(); return true; @@ -761,21 +793,20 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { if (msg.type === "AUTISTMASK_SIGN_RESPONSE") { const approval = pendingApprovals[msg.id]; if (!approval) return false; - delete pendingApprovals[msg.id]; - resetPopupUrl(); if (!msg.approved) { + finishApproval(msg.id); approval.resolve({ error: { code: 4001, message: "User rejected the request." }, }); return true; } - // The popup signs; it reports back here when it could not. Fail the - // request the same way this handler used to when it did the signing. + // The popup signs; it reports back here when it could not. Keep the + // approval so the user can correct the problem and try again with the + // message they already saw. if (msg.error) { - approval.resolve({ error: { message: msg.error } }); - sendResponse({ error: msg.error }); + sendResponse({ error: msg.error, retryable: true }); return false; } @@ -788,14 +819,17 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => { // address. const signature = msg.signature; verifySignature(approval.signParams, signature, activeAddress); + finishApproval(msg.id); approval.resolve({ signature }); sendResponse({ signature }); } catch (e) { const errMsg = e.shortMessage || e.message; - approval.resolve({ - error: { message: errMsg }, - }); - sendResponse({ error: errMsg }); + const retryable = failureIsRetryable(e); + if (!retryable) { + finishApproval(msg.id); + approval.resolve({ error: { message: errMsg } }); + } + sendResponse({ error: errMsg, retryable }); } })(); return true; diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index c9cc740..c6ad603 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -22,6 +22,7 @@ const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList"); const { decryptWithPassword } = require("../../shared/vault"); const { getSignerForAddress } = require("../../shared/wallet"); const { getProvider } = require("../../shared/balances"); +const { describeSigningFailure } = require("../../shared/approvalVerify"); const txStatus = require("./txStatus"); const uniswap = require("../../shared/uniswap"); const runtime = @@ -546,10 +547,20 @@ function init(ctx) { runtime.sendMessage(payload, (response) => { if (response && response.txHash) { txStatus.showWait(pendingTxDetails, response.txHash); + return; + } + // A retryable failure leaves the approval pending in the + // background, so stay on this screen with a live button rather + // than sending the user to a dead end. + const outcome = describeSigningFailure( + response, + "The transaction could not be sent.", + ); + if (outcome.retryable) { + showError("approve-tx-error", outcome.message); + setTxButtonBusy(false); } else { - const msg = - (response && response.error) || "Transaction failed."; - txStatus.showError(pendingTxDetails, null, msg); + txStatus.showError(pendingTxDetails, null, outcome.message); } }); }); @@ -644,11 +655,18 @@ function init(ctx) { runtime.sendMessage(payload, (response) => { if (response && response.signature) { window.close(); - } else { - const msg = (response && response.error) || "Signing failed."; - showError("approve-sign-error", msg); - setSignButtonBusy(false); + return; } + // The button comes back only when the approval is still pending in + // the background; otherwise it stays disabled and the message says + // why, because a control that cannot succeed must not look like it + // can. + const outcome = describeSigningFailure( + response, + "The message could not be signed.", + ); + showError("approve-sign-error", outcome.message); + if (outcome.retryable) setSignButtonBusy(false); }); }); diff --git a/src/shared/approvalVerify.js b/src/shared/approvalVerify.js index 4004425..f311611 100644 --- a/src/shared/approvalVerify.js +++ b/src/shared/approvalVerify.js @@ -7,6 +7,19 @@ // the signer from the artifact and checks it against the approval it is // holding before acting on it. All recovery is delegated to ethers. // +// Every consequential field is compared, and a mismatch is a refusal to act, +// never a warning: what the user approved is what gets broadcast, or nothing +// does. +// +// Fields the approval does not carry are not treated as zero. The popup +// populates nonce, gas limit, fee and chain id through populateTransaction() +// when the requesting page did not fix them, so there is no approved value to +// compare against; treating absent as zero would refuse every legitimate +// transaction. Those fields are instead held to the absolute ceilings below, +// and the chain id is always checked against the selected network rather than +// against the approval alone, which is what makes a cross-chain replay +// impossible. +// // Every failure message is a full sentence, because these strings are shown to // the user and returned to the dApp. @@ -18,6 +31,38 @@ const { verifyTypedData, } = require("ethers"); +// Above the block gas limit of every supported network (see networks.js), so +// no transaction that could ever be included is refused by it. +const MAX_GAS_LIMIT = 100000000n; + +// 100,000 gwei per gas: orders of magnitude above the highest fee either +// supported network has produced, and low enough to catch a fee that would +// hand the validator the balance. +const MAX_FEE_PER_GAS = 100000000000000n; + +// A refusal to act on an artifact: it is not the thing that was approved, so +// the approval it was offered against is spent and must not be retried. Every +// throw in this module is one of these; the background distinguishes them from +// transient failures (a busy node, a failed broadcast), which leave the +// approval standing so the user can try again. +class ApprovalMismatchError extends Error { + constructor(message) { + super(message); + this.name = "ApprovalMismatchError"; + this.approvalMismatch = true; + } +} + +function refuse(message) { + return new ApprovalMismatchError(message); +} + +// Whether a signing failure leaves the approval usable. Anything that is not a +// mismatch is the user's to correct and retry. +function failureIsRetryable(err) { + return !(err && err.approvalMismatch === true); +} + // Case-insensitive address comparison that tolerates absent values on either // side. Two absent addresses compare equal (contract creation has no `to`). function sameAddress(a, b) { @@ -31,58 +76,176 @@ function sameAddress(a, b) { } } +// Whether the approval fixed a value for a field at all. +function present(v) { + return v !== null && v !== undefined && v !== ""; +} + // Normalize a transaction value (hex string, decimal string, number or // bigint) to a bigint. An absent value is zero, matching ethers. function normalizeValue(v) { - if (v === null || v === undefined || v === "") return 0n; + if (!present(v)) return 0n; return BigInt(v); } +// Normalize a quantity that must be present, refusing anything that is not a +// number: an approval carrying junk in a fee field cannot be compared, and an +// uncomparable field is a refusal rather than a pass. +function normalizeQuantity(v, label) { + try { + return BigInt(v); + } catch { + throw refuse( + "The approved " + + label + + " is not a number, so it cannot be" + + " compared with the signed transaction.", + ); + } +} + // Normalize call data to a lowercase hex string. Absent data is "0x". function normalizeData(v) { if (v === null || v === undefined || v === "" || v === "0x") return "0x"; return String(v).toLowerCase(); } +// Quantity fields the requesting page may fix in the approval. Each is +// compared exactly when the approval carries it, and left to the ceilings +// above when it does not. +const APPROVED_QUANTITIES = [ + { + key: "nonce", + label: "nonce", + message: "The signed transaction does not carry the approved nonce.", + }, + { + key: "gasLimit", + label: "gas limit", + message: + "The signed transaction does not carry the approved gas limit.", + }, + { + key: "gasPrice", + label: "gas price", + message: + "The signed transaction does not carry the approved gas price.", + }, + { + key: "maxFeePerGas", + label: "maximum fee per gas", + message: + "The signed transaction does not carry the approved maximum fee per gas.", + }, + { + key: "maxPriorityFeePerGas", + label: "maximum priority fee per gas", + message: + "The signed transaction does not carry the approved maximum priority fee per gas.", + }, +]; + // Assert that a raw signed transaction is the transaction the user approved, -// signed by the address the approval was raised for. Returns the parsed -// ethers Transaction on success, throws otherwise. -function verifySignedTx(rawSignedTx, txParams, expectedFrom) { +// signed by the address the approval was raised for, on the network that is +// selected. Returns the parsed ethers Transaction on success, throws +// otherwise. +function verifySignedTx(rawSignedTx, txParams, expectedFrom, selectedChainId) { if (typeof rawSignedTx !== "string" || !rawSignedTx.startsWith("0x")) { - throw new Error("The signed transaction is missing or malformed."); + throw refuse("The signed transaction is missing or malformed."); } let parsed; try { parsed = Transaction.from(rawSignedTx); } catch { - throw new Error("The signed transaction could not be decoded."); + throw refuse("The signed transaction could not be decoded."); } if (!parsed.from) { - throw new Error("The signed transaction carries no valid signature."); + throw refuse("The signed transaction carries no valid signature."); } if (!sameAddress(parsed.from, expectedFrom)) { - throw new Error( + throw refuse( "The signed transaction was signed by a different address than the one that was approved.", ); } + + // The selected network, not the artifact, is the authority on which chain + // this may be broadcast to; without it nothing can be verified. + if (!present(selectedChainId)) { + throw refuse( + "The selected network is unknown, so the signed transaction cannot be checked against it.", + ); + } + if (parsed.chainId !== normalizeQuantity(selectedChainId, "network")) { + throw refuse( + "The signed transaction is for a different network than the one that is selected.", + ); + } + if ( + present(txParams.chainId) && + parsed.chainId !== normalizeQuantity(txParams.chainId, "network") + ) { + throw refuse( + "The signed transaction is for a different network than the one that was approved.", + ); + } + if (!sameAddress(parsed.to, txParams.to)) { - throw new Error( + throw refuse( "The signed transaction does not go to the approved recipient.", ); } if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) { - throw new Error( + throw refuse( "The signed transaction does not carry the approved value.", ); } if (normalizeData(parsed.data) !== normalizeData(txParams.data)) { - throw new Error( + throw refuse( "The signed transaction does not carry the approved call data.", ); } + // An approval that fixed EIP-1559 fees must not be signed as a legacy + // transaction, and vice versa: the fee the user agreed to is only + // meaningful under the mechanism it was quoted in. + const approvedEip1559 = + present(txParams.maxFeePerGas) || + present(txParams.maxPriorityFeePerGas); + const approvedLegacy = present(txParams.gasPrice); + const signedEip1559 = parsed.type === 2 || parsed.type === 3; + if ( + (approvedEip1559 && !signedEip1559) || + (approvedLegacy && signedEip1559) + ) { + throw refuse( + "The signed transaction does not use the approved fee mechanism.", + ); + } + + for (const field of APPROVED_QUANTITIES) { + if (!present(txParams[field.key])) continue; + const approved = normalizeQuantity(txParams[field.key], field.label); + if (normalizeQuantity(parsed[field.key], field.label) !== approved) { + throw refuse(field.message); + } + } + + if (parsed.gasLimit > MAX_GAS_LIMIT) { + throw refuse( + "The signed transaction sets a gas limit no network this wallet supports can accept.", + ); + } + for (const key of ["gasPrice", "maxFeePerGas", "maxPriorityFeePerGas"]) { + const fee = parsed[key]; + if (fee !== null && fee !== undefined && fee > MAX_FEE_PER_GAS) { + throw refuse( + "The signed transaction sets a fee per gas far above any plausible value.", + ); + } + } + return parsed; } @@ -91,7 +254,7 @@ function verifySignedTx(rawSignedTx, txParams, expectedFrom) { // address on success, throws otherwise. function verifySignature(signParams, signature, expectedFrom) { if (typeof signature !== "string" || !signature.startsWith("0x")) { - throw new Error("The signature is missing or malformed."); + throw refuse("The signature is missing or malformed."); } let recovered; @@ -109,11 +272,11 @@ function verifySignature(signParams, signature, expectedFrom) { recovered = verifyTypedData(domain, types, message, signature); } } catch { - throw new Error("The signature could not be verified."); + throw refuse("The signature could not be verified."); } if (!sameAddress(recovered, expectedFrom)) { - throw new Error( + throw refuse( "The signature was produced by a different address than the one that was approved.", ); } @@ -121,4 +284,29 @@ function verifySignature(signParams, signature, expectedFrom) { return recovered; } -module.exports = { verifySignedTx, verifySignature, sameAddress }; +// What the popup shows and does after the background reports a failed signing +// attempt. A retryable failure leaves the approval pending in the background, +// so the button goes back to being usable; a refusal spent the approval, and +// the popup says so rather than offering a button that cannot succeed. +function describeSigningFailure(response, fallbackMessage) { + let message = (response && response.error) || fallbackMessage; + if (!/[.!?]$/.test(message)) message += "."; + const retryable = !!(response && response.retryable); + if (!retryable) { + message += + " This request can no longer be signed. Please start it again" + + " from the site."; + } + return { message, retryable }; +} + +module.exports = { + verifySignedTx, + verifySignature, + sameAddress, + failureIsRetryable, + describeSigningFailure, + ApprovalMismatchError, + MAX_GAS_LIMIT, + MAX_FEE_PER_GAS, +}; diff --git a/tests/approvalVerify.test.js b/tests/approvalVerify.test.js index 1146416..2d370bf 100644 --- a/tests/approvalVerify.test.js +++ b/tests/approvalVerify.test.js @@ -3,6 +3,10 @@ const { verifySignedTx, verifySignature, sameAddress, + failureIsRetryable, + describeSigningFailure, + MAX_GAS_LIMIT, + MAX_FEE_PER_GAS, } = require("../src/shared/approvalVerify"); const { getSignerForAddress } = require("../src/shared/wallet"); @@ -18,6 +22,10 @@ const other = new Wallet(OTHER_KEY); const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; const OTHER_RECIPIENT = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; +// The chain id of the selected network, as networks.js carries it. +const SELECTED = "0x1"; +const SEPOLIA = "0xaa36a7"; + // Approved parameters as a dApp would supply them over eth_sendTransaction. const TX_PARAMS = { from: signer.address, @@ -27,25 +35,38 @@ const TX_PARAMS = { gas: "0x5208", }; +// The values populateTransaction() fills in when the dApp fixed none of them. +const POPULATED = { + chainId: 1, + nonce: 7, + gasLimit: 100000n, + maxFeePerGas: 2000000000n, + maxPriorityFeePerGas: 1000000000n, + type: 2, +}; + // Build a signable transaction from approved params. The popup does the same // thing through populateTransaction(); here the fields are fixed so the test -// needs no provider. -function txFor(params) { +// needs no provider. `overrides` stands in for what a tampered or misbuilt +// popup would put on the wire. +function txFor(params, overrides) { return { - chainId: 1, - nonce: 7, - gasLimit: 100000n, - maxFeePerGas: 2000000000n, - maxPriorityFeePerGas: 1000000000n, - type: 2, + ...POPULATED, to: params.to, value: params.value === undefined ? 0n : BigInt(params.value), data: params.data || "0x", + ...(overrides || {}), }; } -async function signedFor(params, withWallet) { - return (withWallet || signer).signTransaction(txFor(params)); +async function signedFor(params, withWallet, overrides) { + return (withWallet || signer).signTransaction(txFor(params, overrides)); +} + +// Sign the approved transaction with one field changed from what was +// populated, which is the shape of every tamper case below. +async function signedWith(overrides) { + return signedFor(TX_PARAMS, signer, overrides); } describe("sameAddress", () => { @@ -71,7 +92,7 @@ describe("sameAddress", () => { describe("verifySignedTx", () => { test("accepts the approved transaction signed by the approved address", async () => { const raw = await signedFor(TX_PARAMS); - const parsed = verifySignedTx(raw, TX_PARAMS, signer.address); + const parsed = verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED); expect(parsed.from).toBe(signer.address); expect(parsed.hash).toBe(Transaction.from(raw).hash); }); @@ -79,14 +100,16 @@ describe("verifySignedTx", () => { test("accepts a contract creation with no recipient", async () => { const params = { to: undefined, value: "0x0", data: "0x600160005500" }; const raw = await signedFor(params); - expect(() => verifySignedTx(raw, params, signer.address)).not.toThrow(); + expect(() => + verifySignedTx(raw, params, signer.address, SELECTED), + ).not.toThrow(); }); test("accepts an absent value as zero", async () => { const approved = { to: RECIPIENT, data: "0x" }; const raw = await signedFor(approved); expect(() => - verifySignedTx(raw, approved, signer.address), + verifySignedTx(raw, approved, signer.address, SELECTED), ).not.toThrow(); }); @@ -94,7 +117,7 @@ describe("verifySignedTx", () => { const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" }; const raw = await signedFor(approved); expect(() => - verifySignedTx(raw, approved, signer.address), + verifySignedTx(raw, approved, signer.address, SELECTED), ).not.toThrow(); }); @@ -103,9 +126,9 @@ describe("verifySignedTx", () => { ...TX_PARAMS, to: OTHER_RECIPIENT, }); - expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( - /approved recipient/, - ); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED), + ).toThrow(/approved recipient/); }); test("rejects an inflated value", async () => { @@ -113,48 +136,48 @@ describe("verifySignedTx", () => { ...TX_PARAMS, value: "0x4563918244f40000", }); - expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( - /approved value/, - ); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED), + ).toThrow(/approved value/); }); test("rejects substituted call data", async () => { const raw = await signedFor({ ...TX_PARAMS, data: "0xc0ffee" }); - expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( - /approved call data/, - ); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED), + ).toThrow(/approved call data/); }); test("rejects a transaction signed by a different address", async () => { const raw = await signedFor(TX_PARAMS, other); - expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( - /different address/, - ); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED), + ).toThrow(/different address/); }); test("rejects an unsigned transaction", () => { const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized; expect(() => - verifySignedTx(unsigned, TX_PARAMS, signer.address), + verifySignedTx(unsigned, TX_PARAMS, signer.address, SELECTED), ).toThrow(/no valid signature/); }); test("rejects a missing or malformed payload", () => { expect(() => - verifySignedTx(undefined, TX_PARAMS, signer.address), + verifySignedTx(undefined, TX_PARAMS, signer.address, SELECTED), ).toThrow(/missing or malformed/); - expect(() => verifySignedTx("nope", TX_PARAMS, signer.address)).toThrow( - /missing or malformed/, - ); expect(() => - verifySignedTx("0xc0ffee", TX_PARAMS, signer.address), + verifySignedTx("nope", TX_PARAMS, signer.address, SELECTED), + ).toThrow(/missing or malformed/); + expect(() => + verifySignedTx("0xc0ffee", TX_PARAMS, signer.address, SELECTED), ).toThrow(/could not be decoded/); }); test("every rejection message is a full sentence", async () => { const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT }); try { - verifySignedTx(raw, TX_PARAMS, signer.address); + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED); throw new Error("expected a rejection"); } catch (e) { expect(e.message).toMatch(/^[A-Z].*\.$/); @@ -162,6 +185,234 @@ describe("verifySignedTx", () => { }); }); +// One case per consequential field: the field alone differs from what was +// approved, and that alone must refuse the signature. +describe("verifySignedTx field comparison", () => { + test("rejects a chain id that is not the selected network", async () => { + const raw = await signedWith({ chainId: 11155111 }); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED), + ).toThrow(/different network than the one that is selected/); + }); + + test("rejects a chain id that is not the approved one", async () => { + // Selected network and signed chain id agree; the dApp asked for a + // different chain, so the artifact is not what was approved. + const approved = { ...TX_PARAMS, chainId: SEPOLIA }; + const raw = await signedWith({}); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).toThrow(/different network than the one that was approved/); + }); + + test("refuses when the selected network is unknown", async () => { + const raw = await signedWith({}); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, undefined), + ).toThrow(/selected network is unknown/); + }); + + test("rejects a substituted nonce", async () => { + const approved = { ...TX_PARAMS, nonce: 7 }; + const raw = await signedWith({ nonce: 8 }); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).toThrow(/approved nonce/); + }); + + test("rejects a substituted gas limit", async () => { + const approved = { ...TX_PARAMS, gasLimit: "0x186a0" }; + const raw = await signedWith({ gasLimit: 250000n }); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).toThrow(/approved gas limit/); + }); + + test("rejects a substituted maximum fee per gas", async () => { + const approved = { ...TX_PARAMS, maxFeePerGas: "0x77359400" }; + const raw = await signedWith({ maxFeePerGas: 900000000000n }); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).toThrow(/approved maximum fee per gas/); + }); + + test("rejects a substituted maximum priority fee per gas", async () => { + const approved = { ...TX_PARAMS, maxPriorityFeePerGas: "0x3b9aca00" }; + const raw = await signedWith({ maxPriorityFeePerGas: 1500000000n }); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).toThrow(/approved maximum priority fee per gas/); + }); + + test("rejects a substituted legacy gas price", async () => { + const approved = { ...TX_PARAMS, gasPrice: "0x77359400" }; + const legacy = { + type: 0, + gasPrice: 9000000000n, + maxFeePerGas: null, + maxPriorityFeePerGas: null, + }; + const raw = await signedWith(legacy); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).toThrow(/approved gas price/); + }); + + test("rejects an approved legacy fee signed as an EIP-1559 fee", async () => { + const approved = { ...TX_PARAMS, gasPrice: "0x77359400" }; + const raw = await signedWith({}); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).toThrow(/approved fee mechanism/); + }); + + test("rejects an approved EIP-1559 fee signed as a legacy fee", async () => { + const approved = { ...TX_PARAMS, maxFeePerGas: "0x77359400" }; + const raw = await signedWith({ + type: 0, + gasPrice: 2000000000n, + maxFeePerGas: null, + maxPriorityFeePerGas: null, + }); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).toThrow(/approved fee mechanism/); + }); + + test("rejects a gas limit above anything a supported network accepts", async () => { + const raw = await signedWith({ gasLimit: MAX_GAS_LIMIT + 1n }); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED), + ).toThrow(/gas limit no network this wallet supports/); + }); + + test("rejects an absurd fee per gas the approval never fixed", async () => { + const raw = await signedWith({ + maxFeePerGas: MAX_FEE_PER_GAS + 1n, + maxPriorityFeePerGas: MAX_FEE_PER_GAS + 1n, + }); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED), + ).toThrow(/fee per gas far above any plausible value/); + }); + + test("every field mismatch is a refusal, not a warning", async () => { + const raw = await signedWith({ nonce: 8 }); + try { + verifySignedTx( + raw, + { ...TX_PARAMS, nonce: 7 }, + signer.address, + SELECTED, + ); + throw new Error("expected a rejection"); + } catch (e) { + expect(e.approvalMismatch).toBe(true); + expect(e.message).toMatch(/^[A-Z].*\.$/); + } + }); +}); + +// The approval and the artifact spell the same values differently. None of +// these differences is tampering, so none may refuse the signature. +describe("verifySignedTx normalization", () => { + test("accepts a decimal chain id against a hex selected network", async () => { + const raw = await signedWith({}); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, 1), + ).not.toThrow(); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, "1"), + ).not.toThrow(); + }); + + test("accepts an approved chain id written in hex", async () => { + const raw = await signedWith({}); + const approved = { ...TX_PARAMS, chainId: "0x1" }; + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).not.toThrow(); + }); + + test("accepts a hex nonce against a numeric one", async () => { + const raw = await signedWith({ nonce: 7 }); + expect(() => + verifySignedTx( + raw, + { ...TX_PARAMS, nonce: "0x7" }, + signer.address, + SELECTED, + ), + ).not.toThrow(); + }); + + test("accepts a decimal gas limit against a hex one", async () => { + const raw = await signedWith({ gasLimit: 100000n }); + expect(() => + verifySignedTx( + raw, + { ...TX_PARAMS, gasLimit: "100000" }, + signer.address, + SELECTED, + ), + ).not.toThrow(); + }); + + test("accepts fee fields spelled as hex, decimal, number and bigint", async () => { + const raw = await signedWith({}); + for (const maxFee of [ + "0x77359400", + "2000000000", + 2000000000, + 2000000000n, + ]) { + expect(() => + verifySignedTx( + raw, + { ...TX_PARAMS, maxFeePerGas: maxFee }, + signer.address, + SELECTED, + ), + ).not.toThrow(); + } + }); + + test("accepts an approval that fixes no nonce, gas or fee at all", async () => { + const raw = await signedWith({}); + expect(() => + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED), + ).not.toThrow(); + }); + + test("accepts an approval whose recipient case differs", async () => { + const raw = await signedWith({}); + const approved = { ...TX_PARAMS, to: RECIPIENT.toLowerCase() }; + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).not.toThrow(); + }); + + test("accepts absent call data against 0x", async () => { + const approved = { to: RECIPIENT, value: "0x0" }; + const raw = await signedFor({ ...approved, data: "0x" }); + expect(() => + verifySignedTx(raw, approved, signer.address, SELECTED), + ).not.toThrow(); + }); + + test("refuses an approved quantity that is not a number", async () => { + const raw = await signedWith({}); + expect(() => + verifySignedTx( + raw, + { ...TX_PARAMS, maxFeePerGas: "cheap" }, + signer.address, + SELECTED, + ), + ).toThrow(/is not a number/); + }); +}); + const TYPED_DATA = JSON.stringify({ domain: { name: "AutistMask Test", @@ -281,6 +532,66 @@ describe("verifySignature", () => { }); }); +// What happens after a signing attempt fails: the background keeps the +// approval for anything the user can correct, and the popup only offers the +// button again when it did. +describe("signing failure and retry", () => { + test("a failure that is not a mismatch leaves the approval retryable", () => { + expect(failureIsRetryable(new Error("The node is unreachable."))).toBe( + true, + ); + expect(failureIsRetryable(undefined)).toBe(true); + }); + + test("a mismatch spends the approval", async () => { + const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT }); + try { + verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED); + throw new Error("expected a rejection"); + } catch (e) { + expect(failureIsRetryable(e)).toBe(false); + } + }); + + test("a retryable failure keeps the button usable and says only what failed", () => { + const outcome = describeSigningFailure( + { error: "The node rejected the transaction.", retryable: true }, + "The transaction could not be sent.", + ); + expect(outcome.retryable).toBe(true); + expect(outcome.message).toBe("The node rejected the transaction."); + }); + + test("a refusal tells the user to start again from the site", () => { + const outcome = describeSigningFailure( + { + error: "The signed transaction does not go to the approved recipient.", + retryable: false, + }, + "The transaction could not be sent.", + ); + expect(outcome.retryable).toBe(false); + expect(outcome.message).toMatch(/start it again from the site\.$/); + }); + + test("a response the background never sent is treated as a spent approval", () => { + const outcome = describeSigningFailure( + undefined, + "The transaction could not be sent.", + ); + expect(outcome.retryable).toBe(false); + expect(outcome.message).toMatch(/^The transaction could not be sent\./); + }); + + test("every failure message is a full sentence", () => { + const outcome = describeSigningFailure( + { error: "The node is on fire", retryable: true }, + "The transaction could not be sent.", + ); + expect(outcome.message).toMatch(/^[A-Z].*\.$/); + }); +}); + // End-to-end over the messaging boundary, without a browser: run the exact // sequence the approval popup runs, then hand the artifact to the exact check // the background runs before it broadcasts or resolves. Only what the popup @@ -314,7 +625,12 @@ describe("popup signing sequence to background verification", () => { test("a populated, signed transaction is accepted and broadcastable", async () => { const rawSignedTx = await popupSignsTx(TX_PARAMS); - const parsed = verifySignedTx(rawSignedTx, TX_PARAMS, signer.address); + const parsed = verifySignedTx( + rawSignedTx, + TX_PARAMS, + signer.address, + SELECTED, + ); expect(parsed.nonce).toBe(7); expect(parsed.chainId).toBe(1n); expect(parsed.gasLimit).toBe(21000n); @@ -349,7 +665,14 @@ describe("popup signing sequence to background verification", () => { to: OTHER_RECIPIENT, }); expect(() => - verifySignedTx(rawSignedTx, TX_PARAMS, signer.address), + verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SELECTED), ).toThrow(/approved recipient/); }); + + test("the background rejects a transaction populated on another network", async () => { + const rawSignedTx = await popupSignsTx(TX_PARAMS); + expect(() => + verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SEPOLIA), + ).toThrow(/different network than the one that is selected/); + }); });