sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

harden: verify all approval fields and make failed signing retryable (closes #174)
All checks were successful
check / check (push) Successful in 35s
Details

Browse Source 
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. Worse, it named the fields it
checked and so admitted every field it did not name: a type 4 artifact
carrying an EIP-7702 authorization passed verification, paying the
approved amount to the approved recipient and, in the same transaction,
permanently installing another contract's code at the signer's own
account.

The check is now an allowlist in both directions. The transaction type
must be 0, 1 or 2 — the only types this wallet signs — so no later
EIP-2718 type can bring a field along; authorizationList, blobs, blob
commitments and blob gas fees are refused by name; and the access list
is compared with the approval. Every consequential field is compared and
any mismatch refuses outright: 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. Verification then closes by rebuilding the
transaction from exactly those checked fields and comparing the unsigned
bytes, so an artifact carrying anything this module does not account for
is refused without having to be named first.

An approved value that is not a number now refuses like every other
quantity rather than escaping as a raw BigInt conversion error, which
was reported as retryable and left a live button that could never
succeed.

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 once the request has an
outcome, and the background tells the popup which stage failed. A popup
that could not sign is retryable; a mismatch spends the approval; a
failed broadcast is terminal, because the node may have accepted the
transaction and still failed to answer and the popup's retry re-signs at
a freshly fetched nonce rather than re-broadcasting the same bytes,
which would send the approved transfer twice.

Keeping the approval alive for that retry cost it its single use: the
handler read it, then verified and broadcast asynchronously, so a second
AUTISTMASK_TX_RESPONSE carrying the same id started an independent
verify and broadcast instead of finding nothing. With the ordinary dApp
approval shape the page fixes no nonce, so two artifacts signed at
different nonces both verify and the approved transfer goes out twice; a
reloaded approval window during a slow broadcast is enough to send it,
since the only guard was popup-local button state. The approval is now
claimed synchronously, before the first await, and released only when an
attempt fails in a way the user may retry. Same interlock on
AUTISTMASK_SIGN_RESPONSE.

Surviving the whole verify-and-broadcast window put the approval within
reach of every other path that retires one, and those paths did not
consult the claim. Closing the approval popup, switching the active
address, or a reject arriving late each resolved the waiting promise
4001 while the attempt behind it ran to completion; the attempt's own
resolve then landed on a settled promise, so the transaction reached the
chain and the page was told the user rejected it. The user's natural
response is to redo the transfer from the site, which re-signs at a
fresh nonce and sends it twice — the outcome this change exists to
prevent, reached without an adversary, since the popup stays open across
the broadcast and a user closing an apparently-hung window is enough.

Every settlement now goes through one function. settleApproval() is the
only place an approval is resolved or removed, and it refuses a claimed
approval unless the caller holds the claim, so a path added later
inherits the interlock instead of having to remember it. The active-
address switch also leaves a claimed approval's window standing rather
than force-closing the window the attempt is reporting into. The
duplicate refusal on the sign path now carries a stage of its own, so
the popup stops telling the user to start again from the site while a
first attempt may still succeed.

Verification also compared only the decode against itself: both sides of
the closing byte comparison derive from one Transaction.from(), while
what is broadcast is the artifact string. An artifact re-encoded with a
leading zero byte on an RLP quantity therefore decoded to the approved
transaction, passed, and broadcast different bytes. The artifact is now
required to be the canonical encoding of its own decode, which is what
makes the claim that it *is* the approved transaction true.

The background's approval wiring had no tests, which is where these
defects lived. It has them now, driven through the real message listener
from eth_sendTransaction to broadcast, with windows.onRemoved captured
rather than stubbed away: each retirement path is asserted to leave a
mid-broadcast attempt alone and to still reject an approval no attempt
holds.
This commit is contained in:
clawbot
2026-08-12 08:44:41 +00:00
parent 937f699fb1
commit e94afc4c5e
6 changed files with 2227 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

480
src/shared/approvalVerify.js
View File

@@ -7,17 +7,144 @@
// the signer from the artifact and checks it against the approval it is
// holding before acting on it. All recovery is delegated to ethers.
//
// The check is an allowlist, in both directions, because a denylist cannot be
// correct against a transaction format that keeps gaining fields:
//
// - only transaction types 0, 1 and 2 are accepted. Every later EIP-2718 type
// adds a field with consequences of its own — EIP-7702's authorizationList
// rewrites the code at the signer's own account, EIP-4844's blob
// commitments carry a separate fee — and a check that enumerates the fields
// it refuses admits every one of them by default.
// - after the per-field comparisons, the artifact is rebuilt from those
// checked fields and nothing else, and the two are compared byte for byte.
// Anything the artifact carries that this module does not name is absent
// from the rebuild and changes the bytes, so the final assertion is that
// the artifact *is* the approved transaction, not merely that it is not one
// of the tampered shapes that were thought of.
// - every comparison runs against the decode, but the string handed to
// broadcastTransaction() is the artifact. So the artifact is also required
// to be the canonical re-encoding of its own decode, which is what makes
// the checked transaction and the broadcast bytes the same object rather
// than two things that merely decode alike.
//
// 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.
const {
Transaction,
accessListify,
getAddress,
getBytes,
verifyMessage,
verifyTypedData,
} = require("ethers");
// The only transaction types this wallet signs: legacy, EIP-2930 and
// EIP-1559. populateTransaction() produces nothing else, so nothing else can
// be an artifact of an approval this wallet raised.
const ALLOWED_TX_TYPES = [0, 1, 2];
// The serialized fields of each allowed type, which is also the complete set
// of fields the checks below compare or bound. The artifact is rebuilt from
// exactly these at the end of verification and compared byte for byte, so a
// field outside this table cannot ride along unexamined.
const SERIALIZED_FIELDS = {
0: ["chainId", "nonce", "gasPrice", "gasLimit", "to", "value", "data"],
1: [
"chainId",
"nonce",
"gasPrice",
"gasLimit",
"to",
"value",
"data",
"accessList",
],
2: [
"chainId",
"nonce",
"maxPriorityFeePerGas",
"maxFeePerGas",
"gasLimit",
"to",
"value",
"data",
"accessList",
],
};
// Fields no allowed type may carry. The type allowlist already excludes every
// type that defines them, and the structural check at the end of verification
// would catch them anyway; they are named here so that an artifact carrying
// one is refused with a message that says what it was.
const FORBIDDEN_FIELDS = [
{
key: "authorizationList",
message:
"The signed transaction would hand the signing account over to another contract, which was not approved.",
},
{
key: "blobVersionedHashes",
message:
"The signed transaction carries blob commitments, which were not approved.",
},
{
key: "blobs",
message:
"The signed transaction carries blobs, which were not approved.",
},
{
key: "maxFeePerBlobGas",
message:
"The signed transaction carries a blob gas fee, which was not approved.",
},
];
// 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,11 +158,64 @@ function sameAddress(a, b) {
}
}
// Whether the approval fixed a value for a field at all.
function present(v) {
return v !== null && v !== undefined && v !== "";
}
// Whether a field carries anything at all. An empty array is nothing: ethers
// reports an absent access list on a type 2 transaction as `[]`.
function carriesValue(v) {
if (!present(v)) return false;
if (Array.isArray(v)) return v.length > 0;
return true;
}
// 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 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. The value is
// page-controlled, so it goes through the same refusal as every other
// quantity rather than throwing a raw BigInt conversion error.
function normalizeValue(v) {
if (v === null || v === undefined || v === "") return 0n;
return BigInt(v);
if (!present(v)) return 0n;
return normalizeQuantity(v, "value");
}
// Normalize an access list to a comparable string. An absent or empty list is
// the empty string, so absent and `[]` are the same thing.
function normalizeAccessList(v) {
if (!carriesValue(v)) return "";
let list;
try {
list = accessListify(v);
} catch {
throw refuse(
"The approved access list is not a valid access list, so it cannot be compared with the signed transaction.",
);
}
return list
.map(
(entry) =>
String(entry.address).toLowerCase() +
":" +
entry.storageKeys.map((k) => String(k).toLowerCase()).join(","),
)
.join(";");
}
// Normalize call data to a lowercase hex string. Absent data is "0x".
@@ -44,44 +224,216 @@ function normalizeData(v) {
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.",
},
];
// Refuse a field only a transaction type this wallet does not sign can carry.
// The type allowlist keeps these unreachable in production, which is exactly
// what they are for; it also means nothing else exercises them, so this is
// exported and tested on its own rather than left to be believed.
function assertNoForbiddenFields(parsed) {
for (const field of FORBIDDEN_FIELDS) {
if (carriesValue(parsed[field.key])) throw refuse(field.message);
}
}
// Closing structural check. Rebuild the transaction from the fields the
// comparisons cover, and nothing else, then compare the unsigned bytes. Every
// field carried by the artifact but absent from the rebuild changes the
// serialization, so this refuses anything this module does not account for —
// including a field a future ethers learns to parse onto an allowed type —
// instead of waving it through by not naming it. Also exported for its own
// test: nothing reachable today can make the bytes differ.
function assertNothingUnchecked(parsed) {
let rebuilt;
try {
const fields = { type: parsed.type };
for (const key of SERIALIZED_FIELDS[parsed.type]) {
fields[key] = parsed[key];
}
rebuilt = Transaction.from(fields);
} catch {
throw refuse(
"The signed transaction could not be rebuilt from the fields that were checked, so it cannot be shown to be the approved transaction.",
);
}
if (rebuilt.unsignedSerialized !== parsed.unsignedSerialized) {
throw refuse(
"The signed transaction carries data beyond the fields that were checked against the approval.",
);
}
}
// The other half of the closing check, and the one that makes it bind on the
// bytes that actually leave: every comparison above runs against the decode,
// so on its own the rebuild proves only that the transaction ethers understood
// is the approved one. What the background hands to broadcastTransaction() is
// the artifact string itself. Requiring the artifact to be exactly the
// canonical re-encoding of its own decode closes the gap between the two —
// no encoding the decoder normalizes away (a leading zero byte on an RLP
// quantity, say) can differ from what was checked. Hex case is not part of the
// encoding, so only that is normalized before comparing.
function assertCanonicalBytes(parsed, rawSignedTx) {
if (parsed.serialized !== String(rawSignedTx).toLowerCase()) {
throw refuse(
"The signed transaction is not encoded canonically, so the bytes that would be broadcast are not the bytes that were checked.",
);
}
}
// 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.",
);
}
// Before any field is looked at: the type decides which fields exist at
// all, so an unrecognised type is refused outright rather than compared
// field by field against an approval that cannot describe it.
if (!ALLOWED_TX_TYPES.includes(parsed.type)) {
throw refuse(
"The signed transaction is of a type this wallet does not sign, so what it would do beyond the approved transfer cannot be checked.",
);
}
assertNoForbiddenFields(parsed);
// 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.",
);
}
if (
normalizeAccessList(parsed.accessList) !==
normalizeAccessList(txParams.accessList)
) {
throw refuse(
"The signed transaction does not carry the approved access list.",
);
}
// 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;
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.",
);
}
}
assertNothingUnchecked(parsed);
assertCanonicalBytes(parsed, rawSignedTx);
return parsed;
}
@@ -91,7 +443,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 +461,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 +473,98 @@ function verifySignature(signParams, signature, expectedFrom) {
return recovered;
}
module.exports = { verifySignedTx, verifySignature, sameAddress };
// The stage a transaction approval failed at. Which stage it is decides
// whether the approval survives the failure.
const TX_STAGE_SIGN = "sign";
const TX_STAGE_VERIFY = "verify";
const TX_STAGE_BROADCAST = "broadcast";
// Not a failure of this request at all: a second response arrived for an
// approval an attempt already holds. The first attempt is still running and
// may yet succeed, so the one thing the popup must not say is "start again
// from the site".
const TX_STAGE_INFLIGHT = "inflight";
function errorText(err) {
if (typeof err === "string" && err !== "") return err;
if (err && (err.shortMessage || err.message)) {
return err.shortMessage || err.message;
}
return "The transaction could not be sent.";
}
// What the background does with a pending transaction approval after a failed
// attempt: what it tells the popup, and whether the approval is spent
// (resolved to the requesting page as an error and deleted) or left standing
// so the user can try the transaction they already saw again.
//
// - sign: the popup could not produce an artifact, almost always a wrong
// password. Nothing left the extension, so the approval stands.
// - verify: a mismatch is a refusal and spends the approval — an artifact
// that is not the approved transaction must never be retried against that
// approval. Anything else failed before the check ran and is retryable.
// - broadcast: always terminal. A broadcast that throws after the node
// accepted the transaction is routine (a timeout, a dropped response, a
// node answering "already known"), and the popup's retry does not
// re-broadcast these bytes — it re-runs populateTransaction() and signs
// again at a freshly fetched pending-tag nonce. Retrying would therefore
// put a second transaction on the chain for one approval.
function describeTxFailure(stage, err) {
const error = errorText(err);
const retryable =
stage === TX_STAGE_SIGN ||
(stage === TX_STAGE_VERIFY && failureIsRetryable(err));
return { error, retryable, spendApproval: !retryable };
}
// 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.
//
// A failed broadcast gets its own wording: the transaction may already be on
// the network, so telling the user to start again from the site is exactly the
// wrong instruction.
function describeSigningFailure(response, fallbackMessage) {
let message = (response && response.error) || fallbackMessage;
if (!/[.!?]$/.test(message)) message += ".";
const retryable = !!(response && response.retryable);
const stage = response && response.stage;
if (!retryable) {
if (stage === TX_STAGE_BROADCAST) {
message +=
" The transaction may still have reached the network." +
" Check the account before sending it again.";
} else if (stage === TX_STAGE_INFLIGHT) {
message +=
" The first attempt is still running and may still succeed." +
" Wait for it rather than starting again.";
} else {
message +=
" This request can no longer be signed. Please start it" +
" again from the site.";
}
}
return { message, retryable };
}
module.exports = {
verifySignedTx,
verifySignature,
assertNoForbiddenFields,
assertNothingUnchecked,
assertCanonicalBytes,
sameAddress,
failureIsRetryable,
describeTxFailure,
describeSigningFailure,
ApprovalMismatchError,
ALLOWED_TX_TYPES,
SERIALIZED_FIELDS,
FORBIDDEN_FIELDS,
TX_STAGE_SIGN,
TX_STAGE_VERIFY,
TX_STAGE_BROADCAST,
TX_STAGE_INFLIGHT,
MAX_GAS_LIMIT,
MAX_FEE_PER_GAS,
};