Compare commits

...

1 Commits

Author SHA1 Message Date
979bea2d0d harden: verify all approval fields and make failed signing retryable (closes #174)
All checks were successful
check / check (push) Successful in 33s
verifySignedTx compared only from, to, value and data, so a signed
transaction could differ from the approval in chain id, nonce, gas limit
or any fee field and still be broadcast. It now compares every
consequential field and refuses outright on any mismatch: the chain id
against the selected network (and against the approval when the page
fixed one), plus nonce, gas limit, gasPrice, maxFeePerGas and
maxPriorityFeePerGas wherever the approval carries a value, together
with the fee mechanism the approval implies. Fields the approval does
not carry are populated locally by the popup and have no approved value
to compare against, so they are held to absolute ceilings instead.

A failed signing attempt also left a button that could not succeed: the
background deleted the approval before it broadcast, so a retry found
nothing to sign. The approval is now retired only once the request has
an outcome, and the background tells the popup whether the failure is
retryable, so the button comes back for a failure the user can correct
and stays down with an explanation when the approval is spent.
2026-08-11 12:26:54 +00:00
5 changed files with 647 additions and 79 deletions

View File

@@ -44,6 +44,11 @@ undefined identifiers, which is how
# Completed Steps # 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: `docs/README.md` rewritten against the code: no competitor names, - 2026-08-11: `docs/README.md` rewritten against the code: no competitor names,
all five network destinations documented, password/Settings/Add Wallet all five network destinations documented, password/Settings/Add Wallet
sections corrected ([#163](https://git.eeqj.de/sneak/AutistMask/issues/163)). sections corrected ([#163](https://git.eeqj.de/sneak/AutistMask/issues/163)).

View File

@@ -13,7 +13,11 @@ const {
} = require("../shared/state"); } = require("../shared/state");
const { refreshBalances, getProvider } = require("../shared/balances"); const { refreshBalances, getProvider } = require("../shared/balances");
const { debugFetch } = require("../shared/log"); const { debugFetch } = require("../shared/log");
const { verifySignedTx, verifySignature } = require("../shared/approvalVerify"); const {
verifySignedTx,
verifySignature,
failureIsRetryable,
} = require("../shared/approvalVerify");
const { const {
isPhishingDomain, isPhishingDomain,
updatePhishingList, 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. // Open approval in a separate popup window.
// This is the primary mechanism for tx/sign approvals (triggered programmatically, // This is the primary mechanism for tx/sign approvals (triggered programmatically,
// not from a user gesture) and the fallback for site-connection approvals. // 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") { if (msg.type === "AUTISTMASK_TX_RESPONSE") {
const approval = pendingApprovals[msg.id]; const approval = pendingApprovals[msg.id];
if (!approval) return false; if (!approval) return false;
delete pendingApprovals[msg.id];
resetPopupUrl();
if (!msg.approved) { if (!msg.approved) {
finishApproval(msg.id);
approval.resolve({ approval.resolve({
error: { code: 4001, message: "User rejected the request." }, error: { code: 4001, message: "User rejected the request." },
}); });
return true; return true;
} }
// The popup signs; it reports back here when it could not. Fail the // The popup signs; it reports back here when it could not. Keep the
// request the same way this handler used to when it did the signing. // approval so the user can correct the problem and try again with the
// transaction they already saw.
if (msg.error) { if (msg.error) {
approval.resolve({ error: { message: msg.error } }); sendResponse({ error: msg.error, retryable: true });
sendResponse({ error: msg.error });
return false; return false;
} }
@@ -737,22 +749,42 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
const activeAddress = await getActiveAddress(); const activeAddress = await getActiveAddress();
// The popup holds the secret, but the background stays the // The popup holds the secret, but the background stays the
// authority on what is broadcast: the raw transaction must be // 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( verifySignedTx(
msg.rawSignedTx, msg.rawSignedTx,
approval.txParams, approval.txParams,
activeAddress, 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 provider = getProvider(state.rpcUrl);
const tx = await provider.broadcastTransaction(msg.rawSignedTx); const tx = await provider.broadcastTransaction(msg.rawSignedTx);
finishApproval(msg.id);
approval.resolve({ txHash: tx.hash }); approval.resolve({ txHash: tx.hash });
sendResponse({ txHash: tx.hash }); sendResponse({ txHash: tx.hash });
} catch (e) { } catch (e) {
const errMsg = e.shortMessage || e.message; // The node would not take it. The approval stays pending, so
approval.resolve({ // a retry re-signs the same approved transaction.
error: { message: errMsg }, sendResponse({
error: e.shortMessage || e.message,
retryable: true,
}); });
sendResponse({ error: errMsg });
} }
})(); })();
return true; return true;
@@ -761,21 +793,20 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
if (msg.type === "AUTISTMASK_SIGN_RESPONSE") { if (msg.type === "AUTISTMASK_SIGN_RESPONSE") {
const approval = pendingApprovals[msg.id]; const approval = pendingApprovals[msg.id];
if (!approval) return false; if (!approval) return false;
delete pendingApprovals[msg.id];
resetPopupUrl();
if (!msg.approved) { if (!msg.approved) {
finishApproval(msg.id);
approval.resolve({ approval.resolve({
error: { code: 4001, message: "User rejected the request." }, error: { code: 4001, message: "User rejected the request." },
}); });
return true; return true;
} }
// The popup signs; it reports back here when it could not. Fail the // The popup signs; it reports back here when it could not. Keep the
// request the same way this handler used to when it did the signing. // approval so the user can correct the problem and try again with the
// message they already saw.
if (msg.error) { if (msg.error) {
approval.resolve({ error: { message: msg.error } }); sendResponse({ error: msg.error, retryable: true });
sendResponse({ error: msg.error });
return false; return false;
} }
@@ -788,14 +819,17 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
// address. // address.
const signature = msg.signature; const signature = msg.signature;
verifySignature(approval.signParams, signature, activeAddress); verifySignature(approval.signParams, signature, activeAddress);
finishApproval(msg.id);
approval.resolve({ signature }); approval.resolve({ signature });
sendResponse({ signature }); sendResponse({ signature });
} catch (e) { } catch (e) {
const errMsg = e.shortMessage || e.message; const errMsg = e.shortMessage || e.message;
approval.resolve({ const retryable = failureIsRetryable(e);
error: { message: errMsg }, if (!retryable) {
}); finishApproval(msg.id);
sendResponse({ error: errMsg }); approval.resolve({ error: { message: errMsg } });
}
sendResponse({ error: errMsg, retryable });
} }
})(); })();
return true; return true;

View File

@@ -22,6 +22,7 @@ const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList");
const { decryptWithPassword } = require("../../shared/vault"); const { decryptWithPassword } = require("../../shared/vault");
const { getSignerForAddress } = require("../../shared/wallet"); const { getSignerForAddress } = require("../../shared/wallet");
const { getProvider } = require("../../shared/balances"); const { getProvider } = require("../../shared/balances");
const { describeSigningFailure } = require("../../shared/approvalVerify");
const txStatus = require("./txStatus"); const txStatus = require("./txStatus");
const uniswap = require("../../shared/uniswap"); const uniswap = require("../../shared/uniswap");
const runtime = const runtime =
@@ -546,10 +547,20 @@ function init(ctx) {
runtime.sendMessage(payload, (response) => { runtime.sendMessage(payload, (response) => {
if (response && response.txHash) { if (response && response.txHash) {
txStatus.showWait(pendingTxDetails, 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 { } else {
const msg = txStatus.showError(pendingTxDetails, null, outcome.message);
(response && response.error) || "Transaction failed.";
txStatus.showError(pendingTxDetails, null, msg);
} }
}); });
}); });
@@ -644,11 +655,18 @@ function init(ctx) {
runtime.sendMessage(payload, (response) => { runtime.sendMessage(payload, (response) => {
if (response && response.signature) { if (response && response.signature) {
window.close(); window.close();
} else { return;
const msg = (response && response.error) || "Signing failed.";
showError("approve-sign-error", msg);
setSignButtonBusy(false);
} }
// 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);
}); });
}); });

View File

@@ -7,6 +7,19 @@
// the signer from the artifact and checks it against the approval it is // the signer from the artifact and checks it against the approval it is
// holding before acting on it. All recovery is delegated to ethers. // 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 // Every failure message is a full sentence, because these strings are shown to
// the user and returned to the dApp. // the user and returned to the dApp.
@@ -18,6 +31,38 @@ const {
verifyTypedData, verifyTypedData,
} = require("ethers"); } = 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 // Case-insensitive address comparison that tolerates absent values on either
// side. Two absent addresses compare equal (contract creation has no `to`). // side. Two absent addresses compare equal (contract creation has no `to`).
function sameAddress(a, b) { 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 // Normalize a transaction value (hex string, decimal string, number or
// bigint) to a bigint. An absent value is zero, matching ethers. // bigint) to a bigint. An absent value is zero, matching ethers.
function normalizeValue(v) { function normalizeValue(v) {
if (v === null || v === undefined || v === "") return 0n; if (!present(v)) return 0n;
return BigInt(v); 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". // Normalize call data to a lowercase hex string. Absent data is "0x".
function normalizeData(v) { function normalizeData(v) {
if (v === null || v === undefined || v === "" || v === "0x") return "0x"; if (v === null || v === undefined || v === "" || v === "0x") return "0x";
return String(v).toLowerCase(); 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, // Assert that a raw signed transaction is the transaction the user approved,
// signed by the address the approval was raised for. Returns the parsed // signed by the address the approval was raised for, on the network that is
// ethers Transaction on success, throws otherwise. // selected. Returns the parsed ethers Transaction on success, throws
function verifySignedTx(rawSignedTx, txParams, expectedFrom) { // otherwise.
function verifySignedTx(rawSignedTx, txParams, expectedFrom, selectedChainId) {
if (typeof rawSignedTx !== "string" || !rawSignedTx.startsWith("0x")) { 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; let parsed;
try { try {
parsed = Transaction.from(rawSignedTx); parsed = Transaction.from(rawSignedTx);
} catch { } catch {
throw new Error("The signed transaction could not be decoded."); throw refuse("The signed transaction could not be decoded.");
} }
if (!parsed.from) { 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)) { 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 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)) { if (!sameAddress(parsed.to, txParams.to)) {
throw new Error( throw refuse(
"The signed transaction does not go to the approved recipient.", "The signed transaction does not go to the approved recipient.",
); );
} }
if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) { if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) {
throw new Error( throw refuse(
"The signed transaction does not carry the approved value.", "The signed transaction does not carry the approved value.",
); );
} }
if (normalizeData(parsed.data) !== normalizeData(txParams.data)) { if (normalizeData(parsed.data) !== normalizeData(txParams.data)) {
throw new Error( throw refuse(
"The signed transaction does not carry the approved call data.", "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; return parsed;
} }
@@ -91,7 +254,7 @@ function verifySignedTx(rawSignedTx, txParams, expectedFrom) {
// address on success, throws otherwise. // address on success, throws otherwise.
function verifySignature(signParams, signature, expectedFrom) { function verifySignature(signParams, signature, expectedFrom) {
if (typeof signature !== "string" || !signature.startsWith("0x")) { 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; let recovered;
@@ -109,11 +272,11 @@ function verifySignature(signParams, signature, expectedFrom) {
recovered = verifyTypedData(domain, types, message, signature); recovered = verifyTypedData(domain, types, message, signature);
} }
} catch { } catch {
throw new Error("The signature could not be verified."); throw refuse("The signature could not be verified.");
} }
if (!sameAddress(recovered, expectedFrom)) { if (!sameAddress(recovered, expectedFrom)) {
throw new Error( throw refuse(
"The signature was produced by a different address than the one that was approved.", "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; 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,
};

View File

@@ -3,6 +3,10 @@ const {
verifySignedTx, verifySignedTx,
verifySignature, verifySignature,
sameAddress, sameAddress,
failureIsRetryable,
describeSigningFailure,
MAX_GAS_LIMIT,
MAX_FEE_PER_GAS,
} = require("../src/shared/approvalVerify"); } = require("../src/shared/approvalVerify");
const { getSignerForAddress } = require("../src/shared/wallet"); const { getSignerForAddress } = require("../src/shared/wallet");
@@ -18,6 +22,10 @@ const other = new Wallet(OTHER_KEY);
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const OTHER_RECIPIENT = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; 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. // Approved parameters as a dApp would supply them over eth_sendTransaction.
const TX_PARAMS = { const TX_PARAMS = {
from: signer.address, from: signer.address,
@@ -27,25 +35,38 @@ const TX_PARAMS = {
gas: "0x5208", 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 // Build a signable transaction from approved params. The popup does the same
// thing through populateTransaction(); here the fields are fixed so the test // thing through populateTransaction(); here the fields are fixed so the test
// needs no provider. // needs no provider. `overrides` stands in for what a tampered or misbuilt
function txFor(params) { // popup would put on the wire.
function txFor(params, overrides) {
return { return {
chainId: 1, ...POPULATED,
nonce: 7,
gasLimit: 100000n,
maxFeePerGas: 2000000000n,
maxPriorityFeePerGas: 1000000000n,
type: 2,
to: params.to, to: params.to,
value: params.value === undefined ? 0n : BigInt(params.value), value: params.value === undefined ? 0n : BigInt(params.value),
data: params.data || "0x", data: params.data || "0x",
...(overrides || {}),
}; };
} }
async function signedFor(params, withWallet) { async function signedFor(params, withWallet, overrides) {
return (withWallet || signer).signTransaction(txFor(params)); 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", () => { describe("sameAddress", () => {
@@ -71,7 +92,7 @@ describe("sameAddress", () => {
describe("verifySignedTx", () => { describe("verifySignedTx", () => {
test("accepts the approved transaction signed by the approved address", async () => { test("accepts the approved transaction signed by the approved address", async () => {
const raw = await signedFor(TX_PARAMS); 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.from).toBe(signer.address);
expect(parsed.hash).toBe(Transaction.from(raw).hash); expect(parsed.hash).toBe(Transaction.from(raw).hash);
}); });
@@ -79,14 +100,16 @@ describe("verifySignedTx", () => {
test("accepts a contract creation with no recipient", async () => { test("accepts a contract creation with no recipient", async () => {
const params = { to: undefined, value: "0x0", data: "0x600160005500" }; const params = { to: undefined, value: "0x0", data: "0x600160005500" };
const raw = await signedFor(params); 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 () => { test("accepts an absent value as zero", async () => {
const approved = { to: RECIPIENT, data: "0x" }; const approved = { to: RECIPIENT, data: "0x" };
const raw = await signedFor(approved); const raw = await signedFor(approved);
expect(() => expect(() =>
verifySignedTx(raw, approved, signer.address), verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow(); ).not.toThrow();
}); });
@@ -94,7 +117,7 @@ describe("verifySignedTx", () => {
const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" }; const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" };
const raw = await signedFor(approved); const raw = await signedFor(approved);
expect(() => expect(() =>
verifySignedTx(raw, approved, signer.address), verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow(); ).not.toThrow();
}); });
@@ -103,9 +126,9 @@ describe("verifySignedTx", () => {
...TX_PARAMS, ...TX_PARAMS,
to: OTHER_RECIPIENT, to: OTHER_RECIPIENT,
}); });
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( expect(() =>
/approved recipient/, verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
); ).toThrow(/approved recipient/);
}); });
test("rejects an inflated value", async () => { test("rejects an inflated value", async () => {
@@ -113,48 +136,48 @@ describe("verifySignedTx", () => {
...TX_PARAMS, ...TX_PARAMS,
value: "0x4563918244f40000", value: "0x4563918244f40000",
}); });
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( expect(() =>
/approved value/, verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
); ).toThrow(/approved value/);
}); });
test("rejects substituted call data", async () => { test("rejects substituted call data", async () => {
const raw = await signedFor({ ...TX_PARAMS, data: "0xc0ffee" }); const raw = await signedFor({ ...TX_PARAMS, data: "0xc0ffee" });
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( expect(() =>
/approved call data/, verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
); ).toThrow(/approved call data/);
}); });
test("rejects a transaction signed by a different address", async () => { test("rejects a transaction signed by a different address", async () => {
const raw = await signedFor(TX_PARAMS, other); const raw = await signedFor(TX_PARAMS, other);
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( expect(() =>
/different address/, verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
); ).toThrow(/different address/);
}); });
test("rejects an unsigned transaction", () => { test("rejects an unsigned transaction", () => {
const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized; const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized;
expect(() => expect(() =>
verifySignedTx(unsigned, TX_PARAMS, signer.address), verifySignedTx(unsigned, TX_PARAMS, signer.address, SELECTED),
).toThrow(/no valid signature/); ).toThrow(/no valid signature/);
}); });
test("rejects a missing or malformed payload", () => { test("rejects a missing or malformed payload", () => {
expect(() => expect(() =>
verifySignedTx(undefined, TX_PARAMS, signer.address), verifySignedTx(undefined, TX_PARAMS, signer.address, SELECTED),
).toThrow(/missing or malformed/); ).toThrow(/missing or malformed/);
expect(() => verifySignedTx("nope", TX_PARAMS, signer.address)).toThrow(
/missing or malformed/,
);
expect(() => 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/); ).toThrow(/could not be decoded/);
}); });
test("every rejection message is a full sentence", async () => { test("every rejection message is a full sentence", async () => {
const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT }); const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
try { try {
verifySignedTx(raw, TX_PARAMS, signer.address); verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
throw new Error("expected a rejection"); throw new Error("expected a rejection");
} catch (e) { } catch (e) {
expect(e.message).toMatch(/^[A-Z].*\.$/); 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({ const TYPED_DATA = JSON.stringify({
domain: { domain: {
name: "AutistMask Test", 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 // 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 // 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 // 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 () => { test("a populated, signed transaction is accepted and broadcastable", async () => {
const rawSignedTx = await popupSignsTx(TX_PARAMS); 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.nonce).toBe(7);
expect(parsed.chainId).toBe(1n); expect(parsed.chainId).toBe(1n);
expect(parsed.gasLimit).toBe(21000n); expect(parsed.gasLimit).toBe(21000n);
@@ -349,7 +665,14 @@ describe("popup signing sequence to background verification", () => {
to: OTHER_RECIPIENT, to: OTHER_RECIPIENT,
}); });
expect(() => expect(() =>
verifySignedTx(rawSignedTx, TX_PARAMS, signer.address), verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SELECTED),
).toThrow(/approved recipient/); ).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/);
});
}); });