Compare commits
4 Commits
91b36d7d5d
...
993afbc5da
| Author | SHA1 | Date | |
|---|---|---|---|
| 993afbc5da | |||
| a08ba6a66d | |||
| 09b602579a | |||
| 18b47cd579 |
16
README.md
16
README.md
@@ -1041,7 +1041,12 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
|
||||
- **When**: A connected website requests a transaction via
|
||||
`eth_sendTransaction`. Always opened in a separate popup window by the
|
||||
background script (`windows.create()`), because the request is triggered
|
||||
programmatically rather than by a user gesture.
|
||||
programmatically rather than by a user gesture. The background populates the
|
||||
transaction (nonce, gas limit, fees, chain id) against the RPC node _before_
|
||||
opening the window, so the screen shows a complete transaction and the signed
|
||||
artifact can be compared with it field for field. A request that cannot be
|
||||
populated — unreachable node, reverting gas estimate — opens no window and is
|
||||
failed back to the site.
|
||||
- **Elements**:
|
||||
- "Transaction Request" heading
|
||||
- Phishing warning banner (shown when the hostname is on the phishing
|
||||
@@ -1053,13 +1058,16 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
|
||||
- Contract: color dot + full address + etherscan link (or "contract
|
||||
creation"), token symbol label if known
|
||||
- Value: amount in ETH (4 decimal places, USD in parentheses)
|
||||
- Network fee (max): gas limit × fee per gas in ETH (4 decimal places, USD
|
||||
in parentheses), with the gas limit and the fee per gas in gwei below it
|
||||
- Network and nonce
|
||||
- Raw data: full calldata displayed inline (shown if present)
|
||||
- Password input and an error line
|
||||
- "Confirm" / "Reject" buttons
|
||||
- **Transitions**:
|
||||
- "Confirm" (correct password) → decrypts and signs in the popup, hands the
|
||||
signed transaction to the background to broadcast, then → **WaitTx** in
|
||||
the same popup window
|
||||
- "Confirm" (correct password) → decrypts and signs the transaction it was
|
||||
shown, exactly as shown, hands the signed transaction to the background to
|
||||
broadcast, then → **WaitTx** in the same popup window
|
||||
- "Confirm" (wrong password) → error line, no screen change
|
||||
- "Reject" → closes popup (returns rejection to background)
|
||||
- Popup window closed without answering → the request is rejected with
|
||||
|
||||
35
TODO.md
35
TODO.md
@@ -44,6 +44,41 @@ undefined identifiers, which is how
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-12: The transaction a dApp asks for is now populated in the background
|
||||
before the approval window opens, so the object the user is shown is the
|
||||
object the signed artifact is verified against — nonce, gas limit and every
|
||||
fee field are compared exactly instead of being left to the ceilings, which
|
||||
stay as a backstop against what a lying RPC node can talk the wallet into
|
||||
displaying. The approval also pins the address it was raised for, so an
|
||||
address switch between approval and signing refuses rather than signing from
|
||||
an account the screen never named, and a request naming an address that is not
|
||||
the active one is refused outright. The approval screen now shows the fee, gas
|
||||
limit, network and nonce it vouches for
|
||||
([#216](https://git.eeqj.de/sneak/AutistMask/issues/216)).
|
||||
- 2026-08-12: The restored navigation stack is filtered against
|
||||
`RESTORABLE_VIEWS` on load, truncated at the first entry the popup would not
|
||||
render so that every surviving entry keeps the Back target it had. Back after
|
||||
reopening can no longer land on a view the popup declined to restore, such as
|
||||
`export-privkey` or `show-phrase`
|
||||
([#224](https://git.eeqj.de/sneak/AutistMask/issues/224)). Restorable views in
|
||||
the stack are still unhidden without being re-rendered; that is tracked
|
||||
separately in ([#268](https://git.eeqj.de/sneak/AutistMask/issues/268)).
|
||||
- 2026-08-12: One wording for a rejected password on every screen that asks for
|
||||
one — the send confirmation and the delete-wallet confirmation no longer say
|
||||
"Wrong password." (a fragment, which `RULES.md` Language & Labeling forbids)
|
||||
and the two reveal screens no longer say "not correct", so all five
|
||||
`decryptWithPassword` call sites now show the sentence the dApp approval paths
|
||||
introduced. Strings only, no behaviour change, and each error container
|
||||
measured at a 360px viewport in the pinned Playwright container
|
||||
([#172](https://git.eeqj.de/sneak/AutistMask/issues/172)).
|
||||
- 2026-08-12: Closed the empty-array hole in the end-to-end unstubbed-request
|
||||
guard. `batch.every()` is vacuously true on `[]`, so a POST with body `[]` was
|
||||
answered `200 []` instead of failing the suite; the guard now rejects an empty
|
||||
batch, demonstrated green-before/red-after with a throwaway probe. The comment
|
||||
claiming `postData()` returns `null` for undecodable bodies was corrected to
|
||||
the two real paths — an absent or empty body decodes to `null`, a binary body
|
||||
decodes lossily into invalid JSON
|
||||
([#187](https://git.eeqj.de/sneak/AutistMask/issues/187)).
|
||||
- 2026-08-12: The transaction confirmation screen has browser coverage. The
|
||||
end-to-end suite reaches ConfirmTx for both the native ETH and the ERC-20 path
|
||||
off a funded-balance fixture, and asserts the pending, funded, over-balance
|
||||
|
||||
@@ -311,10 +311,15 @@ pages. When a site requests access to your wallet:
|
||||
time.
|
||||
|
||||
When a connected site requests a transaction, a separate approval popup appears
|
||||
showing the transaction details (from, to, value, data). You must enter your
|
||||
password and click "Confirm" to authorize it. Message and typed-data signature
|
||||
requests work the same way, with a "Sign" button, and also require your
|
||||
password.
|
||||
showing the transaction details (from, to, value, data, network fee, network and
|
||||
nonce). Every one of those values is checked against the transaction that is
|
||||
actually signed before anything is broadcast, so what you read on that screen is
|
||||
what goes out or nothing does. The popup appears once the wallet has worked out
|
||||
the fee and gas from the network, which takes a moment; if that fails, no popup
|
||||
appears and the site is told the transaction could not be prepared. You must
|
||||
enter your password and click "Confirm" to authorize it. Message and typed-data
|
||||
signature requests work the same way, with a "Sign" button, and also require
|
||||
your password.
|
||||
|
||||
If the requesting site's domain is on the phishing blocklist, all three approval
|
||||
screens show a red phishing warning before you decide.
|
||||
|
||||
@@ -18,11 +18,14 @@ const {
|
||||
verifySignature,
|
||||
failureIsRetryable,
|
||||
describeTxFailure,
|
||||
sameAddress,
|
||||
ApprovalMismatchError,
|
||||
TX_STAGE_SIGN,
|
||||
TX_STAGE_VERIFY,
|
||||
TX_STAGE_BROADCAST,
|
||||
TX_STAGE_INFLIGHT,
|
||||
} = require("../shared/approvalVerify");
|
||||
const { prepareApprovalTx } = require("../shared/approvalTx");
|
||||
const {
|
||||
isPhishingDomain,
|
||||
refreshPhishingListOnSchedule,
|
||||
@@ -77,6 +80,14 @@ async function getActiveAddress() {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Whether a request names a signing address other than the active one. Such a
|
||||
// request is refused rather than quietly signed as whichever address happens
|
||||
// to be active: the page asked for account A and would otherwise be handed
|
||||
// something from account B.
|
||||
function namesAnotherAddress(requested, activeAddress) {
|
||||
return !!requested && !sameAddress(requested, activeAddress);
|
||||
}
|
||||
|
||||
async function getRpcUrl() {
|
||||
const s = await getState();
|
||||
return s.rpcUrl || DEFAULT_RPC_URL;
|
||||
@@ -225,13 +236,21 @@ function requestApproval(origin, hostname) {
|
||||
// Uses windows.create() directly because tx approvals are triggered programmatically
|
||||
// (from a dApp RPC call), not from a user gesture, so action.openPopup() is
|
||||
// unreliable in this context.
|
||||
function requestTxApproval(origin, hostname, txParams) {
|
||||
//
|
||||
// `approvedTx` is the fully populated transaction (see approvalTx.js): the
|
||||
// object the popup displays, the object it signs, and the object the artifact
|
||||
// is verified against. `approvedFrom` is the address that is active now, and
|
||||
// it is pinned here rather than read again at signing time — an address switch
|
||||
// between approval and signing must refuse, not sign from an account this
|
||||
// screen never named.
|
||||
function requestTxApproval(origin, hostname, approvedTx, approvedFrom) {
|
||||
return new Promise((resolve) => {
|
||||
const id = crypto.randomUUID();
|
||||
pendingApprovals[id] = {
|
||||
origin,
|
||||
hostname,
|
||||
txParams,
|
||||
approvedTx,
|
||||
approvedFrom,
|
||||
resolve,
|
||||
type: "tx",
|
||||
};
|
||||
@@ -244,13 +263,14 @@ function requestTxApproval(origin, hostname, txParams) {
|
||||
// Uses windows.create() directly because sign approvals are triggered programmatically
|
||||
// (from a dApp RPC call), not from a user gesture, so action.openPopup() is
|
||||
// unreliable in this context.
|
||||
function requestSignApproval(origin, hostname, signParams) {
|
||||
function requestSignApproval(origin, hostname, signParams, approvedFrom) {
|
||||
return new Promise((resolve) => {
|
||||
const id = crypto.randomUUID();
|
||||
pendingApprovals[id] = {
|
||||
origin,
|
||||
hostname,
|
||||
signParams,
|
||||
approvedFrom,
|
||||
resolve,
|
||||
type: "sign",
|
||||
};
|
||||
@@ -502,6 +522,16 @@ async function handleRpc(method, params, origin) {
|
||||
? { method, message: params[0], from: params[1] }
|
||||
: { method, message: params[1], from: params[0] };
|
||||
|
||||
if (namesAnotherAddress(signParams.from, activeAddress)) {
|
||||
return {
|
||||
error: {
|
||||
code: 4100,
|
||||
message:
|
||||
"This site asked to sign as an address that is not the active one.",
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
if (method === "eth_sign") {
|
||||
signParams.dangerWarning =
|
||||
"\u26a0\ufe0f DANGER: This site is requesting to sign a raw hash. " +
|
||||
@@ -513,6 +543,7 @@ async function handleRpc(method, params, origin) {
|
||||
origin,
|
||||
hostname,
|
||||
signParams,
|
||||
activeAddress,
|
||||
);
|
||||
if (decision.error) return { error: decision.error };
|
||||
return { result: decision.signature };
|
||||
@@ -534,10 +565,20 @@ async function handleRpc(method, params, origin) {
|
||||
}
|
||||
|
||||
const signParams = { method, typedData: params[1], from: params[0] };
|
||||
if (namesAnotherAddress(signParams.from, activeAddress)) {
|
||||
return {
|
||||
error: {
|
||||
code: 4100,
|
||||
message:
|
||||
"This site asked to sign as an address that is not the active one.",
|
||||
},
|
||||
};
|
||||
}
|
||||
const decision = await requestSignApproval(
|
||||
origin,
|
||||
hostname,
|
||||
signParams,
|
||||
activeAddress,
|
||||
);
|
||||
if (decision.error) return { error: decision.error };
|
||||
return { result: decision.signature };
|
||||
@@ -559,7 +600,51 @@ async function handleRpc(method, params, origin) {
|
||||
}
|
||||
|
||||
const txParams = params?.[0] || {};
|
||||
const decision = await requestTxApproval(origin, hostname, txParams);
|
||||
if (namesAnotherAddress(txParams.from, activeAddress)) {
|
||||
return {
|
||||
error: {
|
||||
code: 4100,
|
||||
message:
|
||||
"This site asked to send from an address that is not the active one.",
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// Populate here, before any window opens, so that the transaction the
|
||||
// user is shown is a complete one and is the same object the signed
|
||||
// artifact is checked against. A failure raises no approval at all and
|
||||
// is reported to the requesting page; see approvalTx.js.
|
||||
let approvedTx;
|
||||
try {
|
||||
approvedTx = await prepareApprovalTx(
|
||||
getProvider(await getRpcUrl()),
|
||||
activeAddress,
|
||||
txParams,
|
||||
);
|
||||
} catch (e) {
|
||||
return { error: { message: e.message } };
|
||||
}
|
||||
|
||||
// Population is a network round trip, and the user can switch address
|
||||
// during it. Raising the approval anyway would put an account on the
|
||||
// screen that the wallet is no longer on, and it could never be signed
|
||||
// — the signing handler refuses exactly that. Refuse it here instead,
|
||||
// while the page is still waiting and nothing has been displayed.
|
||||
if (!sameAddress(await getActiveAddress(), activeAddress)) {
|
||||
return {
|
||||
error: {
|
||||
message:
|
||||
"The active address changed while this transaction was being prepared, so it was not sent.",
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const decision = await requestTxApproval(
|
||||
origin,
|
||||
hostname,
|
||||
approvedTx,
|
||||
activeAddress,
|
||||
);
|
||||
if (decision.error) return { error: decision.error };
|
||||
return { result: decision.txHash };
|
||||
}
|
||||
@@ -810,11 +895,16 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
||||
};
|
||||
if (approval.type === "tx") {
|
||||
resp.type = "tx";
|
||||
resp.txParams = approval.txParams;
|
||||
// The populated transaction, and the address it was raised
|
||||
// for. The popup displays and signs exactly this and does not
|
||||
// populate or re-read anything itself.
|
||||
resp.approvedTx = approval.approvedTx;
|
||||
resp.approvedFrom = approval.approvedFrom;
|
||||
}
|
||||
if (approval.type === "sign") {
|
||||
resp.type = "sign";
|
||||
resp.signParams = approval.signParams;
|
||||
resp.approvedFrom = approval.approvedFrom;
|
||||
}
|
||||
// Flag if the requesting domain is on the phishing blocklist.
|
||||
resp.isPhishingDomain = isPhishingDomain(approval.hostname);
|
||||
@@ -888,14 +978,27 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
||||
try {
|
||||
await loadState();
|
||||
const activeAddress = await getActiveAddress();
|
||||
// An address switch between approval and signing refuses. The
|
||||
// approval named one account; signing from whichever account
|
||||
// is active now would send funds from an account this screen
|
||||
// never showed. A switch normally rejects every pending
|
||||
// approval on its way through broadcastAccountsChanged(), so
|
||||
// this is the case where that did not reach the approval —
|
||||
// and it is a refusal, not a retry, because the transaction
|
||||
// the user saw is no longer the transaction that would go out.
|
||||
if (!sameAddress(activeAddress, approval.approvedFrom)) {
|
||||
throw new ApprovalMismatchError(
|
||||
"The active address changed after this transaction was approved, so it was not sent.",
|
||||
);
|
||||
}
|
||||
// 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, on the
|
||||
// network that is selected.
|
||||
// the transaction that was displayed, signed by the address
|
||||
// the approval named, on the network that is selected.
|
||||
verifySignedTx(
|
||||
msg.rawSignedTx,
|
||||
approval.txParams,
|
||||
activeAddress,
|
||||
approval.approvedTx,
|
||||
approval.approvedFrom,
|
||||
currentNetwork().chainId,
|
||||
);
|
||||
} catch (e) {
|
||||
@@ -932,10 +1035,10 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
||||
sendResponse({ txHash: tx.hash });
|
||||
} catch (e) {
|
||||
// Terminal, never retried: 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 these bytes. Retrying would send the
|
||||
// approved transfer a second time.
|
||||
// transaction and still failed to answer, so the wallet cannot
|
||||
// tell a transaction that never left from one already in the
|
||||
// mempool. The page has been given its outcome for this
|
||||
// request; a second attempt would report a second one.
|
||||
const outcome = describeTxFailure(TX_STAGE_BROADCAST, e);
|
||||
settleApproval(
|
||||
msg.id,
|
||||
@@ -998,12 +1101,24 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
||||
(async () => {
|
||||
try {
|
||||
const activeAddress = await getActiveAddress();
|
||||
// Same as the transaction path: the address the approval named
|
||||
// is the one that must have signed, and a switch since then is
|
||||
// a refusal rather than a signature from another account.
|
||||
if (!sameAddress(activeAddress, approval.approvedFrom)) {
|
||||
throw new ApprovalMismatchError(
|
||||
"The active address changed after this request was approved, so it was not signed.",
|
||||
);
|
||||
}
|
||||
// The popup holds the secret, but the background stays the
|
||||
// authority on what is handed back to the page: the signature
|
||||
// must cover the approved payload and recover to the approved
|
||||
// address.
|
||||
// must cover the approved payload and recover to the address
|
||||
// the approval named.
|
||||
const signature = msg.signature;
|
||||
verifySignature(approval.signParams, signature, activeAddress);
|
||||
verifySignature(
|
||||
approval.signParams,
|
||||
signature,
|
||||
approval.approvedFrom,
|
||||
);
|
||||
settleApproval(msg.id, { signature }, { holdsClaim: true });
|
||||
sendResponse({ signature });
|
||||
} catch (e) {
|
||||
|
||||
@@ -1496,6 +1496,33 @@
|
||||
<div class="text-xs text-muted mb-1">Value</div>
|
||||
<div id="approve-tx-value" class="text-xs font-bold"></div>
|
||||
</div>
|
||||
<div class="mb-3">
|
||||
<div class="text-xs text-muted mb-1">Network fee (max)</div>
|
||||
<div
|
||||
id="approve-tx-fee"
|
||||
class="text-xs font-bold min-h-[1rem]"
|
||||
></div>
|
||||
<div
|
||||
id="approve-tx-fee-detail"
|
||||
class="text-xs text-muted min-h-[1rem]"
|
||||
></div>
|
||||
</div>
|
||||
<div class="mb-3 flex justify-between">
|
||||
<div>
|
||||
<div class="text-xs text-muted mb-1">Network</div>
|
||||
<div
|
||||
id="approve-tx-network"
|
||||
class="text-xs min-h-[1rem]"
|
||||
></div>
|
||||
</div>
|
||||
<div>
|
||||
<div class="text-xs text-muted mb-1">Nonce</div>
|
||||
<div
|
||||
id="approve-tx-nonce"
|
||||
class="text-xs min-h-[1rem]"
|
||||
></div>
|
||||
</div>
|
||||
</div>
|
||||
<div id="approve-tx-data-section" class="mb-3 hidden">
|
||||
<div class="text-xs text-muted mb-1">Raw data</div>
|
||||
<div id="approve-tx-data" class="text-xs break-all"></div>
|
||||
|
||||
@@ -10,6 +10,7 @@ const {
|
||||
onViewLeave,
|
||||
} = require("./helpers");
|
||||
const { state, saveState, currentNetwork } = require("../../shared/state");
|
||||
const { networkByChainId } = require("../../shared/networks");
|
||||
const {
|
||||
formatEther,
|
||||
formatUnits,
|
||||
@@ -23,7 +24,6 @@ const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList");
|
||||
const { decryptWithPassword } = require("../../shared/vault");
|
||||
const { getSignerForAddress } = require("../../shared/wallet");
|
||||
const { walletDefect } = require("../../shared/walletDefects");
|
||||
const { getProvider } = require("../../shared/balances");
|
||||
const { describeSigningFailure } = require("../../shared/approvalVerify");
|
||||
const txStatus = require("./txStatus");
|
||||
const uniswap = require("../../shared/uniswap");
|
||||
@@ -159,21 +159,61 @@ function showPhishingWarning(elementId, isPhishing) {
|
||||
}
|
||||
}
|
||||
|
||||
// The fields of the approved transaction the value and recipient lines do not
|
||||
// already carry: network, gas limit, fee per gas, the most the fee can come to,
|
||||
// and the nonce. The background compares every one of them against the signed
|
||||
// artifact, so every one of them has to be on the screen — a number that is
|
||||
// verified but never displayed is verified against nothing the user agreed to.
|
||||
function showTxFee(approvedTx, ethPrice) {
|
||||
const network = networkByChainId(approvedTx.chainId);
|
||||
$("approve-tx-network").textContent = network
|
||||
? network.name
|
||||
: "Unknown network (chain id " + BigInt(approvedTx.chainId) + ")";
|
||||
|
||||
const gasLimit = BigInt(approvedTx.gasLimit);
|
||||
const feePerGas = BigInt(approvedTx.maxFeePerGas || approvedTx.gasPrice);
|
||||
const maxFeeEth = formatTxValue(formatEther(gasLimit * feePerGas));
|
||||
const usdStr = formatUsd(
|
||||
ethPrice ? parseFloat(maxFeeEth) * ethPrice : null,
|
||||
);
|
||||
$("approve-tx-fee").textContent =
|
||||
maxFeeEth + " ETH" + (usdStr ? " (" + usdStr + ")" : "");
|
||||
|
||||
let detail =
|
||||
gasLimit.toString() +
|
||||
" gas at up to " +
|
||||
formatUnits(feePerGas, 9) +
|
||||
" gwei";
|
||||
if (approvedTx.maxPriorityFeePerGas) {
|
||||
detail +=
|
||||
", " +
|
||||
formatUnits(approvedTx.maxPriorityFeePerGas, 9) +
|
||||
" gwei priority";
|
||||
}
|
||||
$("approve-tx-fee-detail").textContent = detail;
|
||||
$("approve-tx-nonce").textContent = BigInt(approvedTx.nonce).toString();
|
||||
}
|
||||
|
||||
function showTxApproval(details) {
|
||||
showPhishingWarning(
|
||||
"approve-tx-phishing-warning",
|
||||
details.isPhishingDomain,
|
||||
);
|
||||
|
||||
pendingTxParams = details.txParams;
|
||||
// The transaction the background populated. It is displayed as it stands,
|
||||
// signed as it stands, and verified against as it stands — the popup fills
|
||||
// nothing in, so there is no number on this screen that the background
|
||||
// cannot compare with the artifact it gets back.
|
||||
pendingTxParams = details.approvedTx;
|
||||
const approvedTx = details.approvedTx;
|
||||
|
||||
const toAddr = details.txParams.to;
|
||||
const toAddr = approvedTx.to;
|
||||
const token = toAddr ? TOKEN_BY_ADDRESS.get(toAddr.toLowerCase()) : null;
|
||||
const ethValue = formatEther(details.txParams.value || "0");
|
||||
const ethValue = formatEther(approvedTx.value || "0");
|
||||
|
||||
// Build txInfo for status screens
|
||||
pendingTxDetails = {
|
||||
from: state.activeAddress,
|
||||
from: details.approvedFrom,
|
||||
to: toAddr || "",
|
||||
amount: formatTxValue(ethValue),
|
||||
token: "ETH",
|
||||
@@ -181,7 +221,7 @@ function showTxApproval(details) {
|
||||
};
|
||||
|
||||
// If this is an ERC-20 call, try to extract the real recipient and amount
|
||||
const decoded = decodeCalldata(details.txParams.data, toAddr || "");
|
||||
const decoded = decodeCalldata(approvedTx.data, toAddr || "");
|
||||
if (decoded && decoded.details) {
|
||||
let decodedTokenAddr = null;
|
||||
let decodedTokenSymbol = null;
|
||||
@@ -219,7 +259,7 @@ function showTxApproval(details) {
|
||||
}
|
||||
|
||||
$("approve-tx-hostname").textContent = details.hostname;
|
||||
$("approve-tx-from").innerHTML = approvalAddressHtml(state.activeAddress);
|
||||
$("approve-tx-from").innerHTML = approvalAddressHtml(details.approvedFrom);
|
||||
|
||||
// Show token symbol next to contract address if known
|
||||
const symbol = toAddr ? tokenLabel(toAddr) : null;
|
||||
@@ -235,7 +275,7 @@ function showTxApproval(details) {
|
||||
}
|
||||
|
||||
const ethValueFormatted = formatTxValue(
|
||||
formatEther(details.txParams.value || "0"),
|
||||
formatEther(approvedTx.value || "0"),
|
||||
);
|
||||
const ethPrice = getPrice("ETH");
|
||||
const ethUsd = ethPrice ? parseFloat(ethValueFormatted) * ethPrice : null;
|
||||
@@ -243,6 +283,8 @@ function showTxApproval(details) {
|
||||
$("approve-tx-value").textContent =
|
||||
ethValueFormatted + " ETH" + (usdStr ? " (" + usdStr + ")" : "");
|
||||
|
||||
showTxFee(approvedTx, ethPrice);
|
||||
|
||||
// Decode calldata (reuse decoded from above)
|
||||
const decodedEl = $("approve-tx-decoded");
|
||||
if (decoded) {
|
||||
@@ -271,8 +313,8 @@ function showTxApproval(details) {
|
||||
}
|
||||
|
||||
// Always show raw data when present
|
||||
if (details.txParams.data && details.txParams.data !== "0x") {
|
||||
$("approve-tx-data").textContent = details.txParams.data;
|
||||
if (approvedTx.data && approvedTx.data !== "0x") {
|
||||
$("approve-tx-data").textContent = approvedTx.data;
|
||||
$("approve-tx-data-section").classList.remove("hidden");
|
||||
} else {
|
||||
$("approve-tx-data-section").classList.add("hidden");
|
||||
@@ -283,7 +325,11 @@ function showTxApproval(details) {
|
||||
|
||||
showView("approve-tx");
|
||||
attachCopyHandlers("view-approve-tx");
|
||||
gateOnWalletDefect("approve-tx-error", "btn-approve-tx");
|
||||
gateOnWalletDefect(
|
||||
"approve-tx-error",
|
||||
"btn-approve-tx",
|
||||
details.approvedFrom,
|
||||
);
|
||||
}
|
||||
|
||||
function decodeHexMessage(hex) {
|
||||
@@ -342,9 +388,12 @@ function showSignApproval(details) {
|
||||
|
||||
const sp = details.signParams;
|
||||
pendingSignParams = sp;
|
||||
pendingSignFrom = details.approvedFrom;
|
||||
|
||||
$("approve-sign-hostname").textContent = details.hostname;
|
||||
$("approve-sign-from").innerHTML = approvalAddressHtml(sp.from);
|
||||
$("approve-sign-from").innerHTML = approvalAddressHtml(
|
||||
details.approvedFrom,
|
||||
);
|
||||
|
||||
const isTyped =
|
||||
sp.method === "eth_signTypedData_v4" ||
|
||||
@@ -383,7 +432,11 @@ function showSignApproval(details) {
|
||||
|
||||
showView("approve-sign");
|
||||
attachCopyHandlers("view-approve-sign");
|
||||
gateOnWalletDefect("approve-sign-error", "btn-approve-sign");
|
||||
gateOnWalletDefect(
|
||||
"approve-sign-error",
|
||||
"btn-approve-sign",
|
||||
details.approvedFrom,
|
||||
);
|
||||
}
|
||||
|
||||
function show(id) {
|
||||
@@ -418,11 +471,15 @@ function show(id) {
|
||||
|
||||
let approvalId = null;
|
||||
let pendingTxDetails = null;
|
||||
// The exact parameters shown to the user, kept so the popup signs what it
|
||||
// displayed rather than re-fetching anything at approval time. Both are
|
||||
// repopulated by show() when the popup is closed and reopened.
|
||||
// The exact objects shown to the user, kept so the popup signs what it
|
||||
// displayed rather than re-fetching or re-populating anything at approval
|
||||
// time. All are repopulated by show() when the popup is closed and reopened.
|
||||
let pendingTxParams = null;
|
||||
let pendingSignParams = null;
|
||||
// The address the approval was raised for. Signing uses this rather than the
|
||||
// active address, so that an address switch since the approval fails here
|
||||
// instead of producing a signature from an account the screen never named.
|
||||
let pendingSignFrom = null;
|
||||
|
||||
// Approve buttons stay disabled and muted while the popup derives the key and
|
||||
// signs, which is slow enough (Argon2id) that a double click is likely.
|
||||
@@ -437,12 +494,13 @@ function setSignButtonBusy(busy) {
|
||||
}
|
||||
|
||||
// Say so on the approval screen itself, and disable the approve button, when
|
||||
// the active address belongs to a wallet whose keys cannot be derived. Without
|
||||
// this the screen would take a password and fail after deriving it. Reject
|
||||
// stays available; the wallet is not touched. Returns true when it gated.
|
||||
function gateOnWalletDefect(errorId, buttonId) {
|
||||
const active = findActiveWallet();
|
||||
const defect = active ? walletDefect(active.wallet) : null;
|
||||
// the address the approval was raised for belongs to a wallet whose keys
|
||||
// cannot be derived. Without this the screen would take a password and fail
|
||||
// after deriving it. Reject stays available; the wallet is not touched.
|
||||
// Returns true when it gated.
|
||||
function gateOnWalletDefect(errorId, buttonId, address) {
|
||||
const owner = findWalletFor(address);
|
||||
const defect = owner ? walletDefect(owner.wallet) : null;
|
||||
if (!defect) return false;
|
||||
showError(errorId, defect.shortMessage);
|
||||
$(buttonId).disabled = true;
|
||||
@@ -450,12 +508,14 @@ function gateOnWalletDefect(errorId, buttonId) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Locate the wallet and the address index owning the currently active
|
||||
// address. Returns null when no wallet holds it.
|
||||
function findActiveWallet() {
|
||||
// Locate the wallet and the address index owning an address. Returns null when
|
||||
// no wallet holds it. Approvals look up the address they were raised for, not
|
||||
// whichever address is active now: the approval named one account, and signing
|
||||
// with another is what verification refuses.
|
||||
function findWalletFor(address) {
|
||||
for (const wallet of state.wallets) {
|
||||
for (let i = 0; i < wallet.addresses.length; i++) {
|
||||
if (wallet.addresses[i].address === state.activeAddress) {
|
||||
if (wallet.addresses[i].address === address) {
|
||||
return { wallet, addrIndex: i };
|
||||
}
|
||||
}
|
||||
@@ -517,12 +577,12 @@ function init(ctx) {
|
||||
hideError("approve-tx-error");
|
||||
setTxButtonBusy(true);
|
||||
|
||||
const active = findActiveWallet();
|
||||
const active = findWalletFor(pendingTxParams.from);
|
||||
if (!active) {
|
||||
password = null;
|
||||
showError(
|
||||
"approve-tx-error",
|
||||
"No wallet was found for the active address.",
|
||||
"No wallet was found for the address this transaction was approved for.",
|
||||
);
|
||||
setTxButtonBusy(false);
|
||||
return;
|
||||
@@ -569,15 +629,16 @@ function init(ctx) {
|
||||
active.addrIndex,
|
||||
decryptedSecret,
|
||||
);
|
||||
const provider = getProvider(state.rpcUrl);
|
||||
const connected = signer.connect(provider);
|
||||
// This is the sequence ethers' own sendTransaction() runs
|
||||
// internally, so nonce, gas, fee and chain id population are
|
||||
// identical to when the background did the signing.
|
||||
const populated =
|
||||
await connected.populateTransaction(pendingTxParams);
|
||||
delete populated.from;
|
||||
payload.rawSignedTx = await connected.signTransaction(populated);
|
||||
// Sign the approved transaction exactly as it was displayed. The
|
||||
// background populated it before this screen was drawn and checks
|
||||
// the artifact against it field for field, so there is nothing to
|
||||
// fill in here and no provider to fill it in from. The copy is
|
||||
// because ethers may strip `from` off what it is handed, and the
|
||||
// approval has to survive a retry intact; keeping `from` on it
|
||||
// makes ethers refuse a key that is not the approved address.
|
||||
payload.rawSignedTx = await signer.signTransaction({
|
||||
...pendingTxParams,
|
||||
});
|
||||
} catch (e) {
|
||||
payload.error =
|
||||
e.shortMessage || e.message || "Transaction signing failed.";
|
||||
@@ -626,12 +687,12 @@ function init(ctx) {
|
||||
hideError("approve-sign-error");
|
||||
setSignButtonBusy(true);
|
||||
|
||||
const active = findActiveWallet();
|
||||
const active = findWalletFor(pendingSignFrom);
|
||||
if (!active) {
|
||||
password = null;
|
||||
showError(
|
||||
"approve-sign-error",
|
||||
"No wallet was found for the active address.",
|
||||
"No wallet was found for the address this request was approved for.",
|
||||
);
|
||||
setSignButtonBusy(false);
|
||||
return;
|
||||
|
||||
@@ -422,7 +422,10 @@ function init(ctx) {
|
||||
password,
|
||||
);
|
||||
} catch (e) {
|
||||
showError("confirm-tx-password-error", "Wrong password.");
|
||||
showError(
|
||||
"confirm-tx-password-error",
|
||||
"That password is incorrect. Please try again.",
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -74,7 +74,8 @@ function init(_ctx) {
|
||||
try {
|
||||
await decryptWithPassword(wallet.encryptedSecret, pw);
|
||||
} catch (_e) {
|
||||
$("delete-wallet-flash").textContent = "Wrong password.";
|
||||
$("delete-wallet-flash").textContent =
|
||||
"That password is incorrect. Please try again.";
|
||||
$("delete-wallet-flash").style.visibility = "visible";
|
||||
btn.disabled = false;
|
||||
btn.classList.remove("text-muted");
|
||||
|
||||
@@ -144,7 +144,7 @@ async function reveal() {
|
||||
$("export-privkey-flash").style.visibility = "hidden";
|
||||
} catch {
|
||||
if (!isCurrentReveal(generation)) return;
|
||||
fail("That password is not correct. Please try again.");
|
||||
fail("That password is incorrect. Please try again.");
|
||||
} finally {
|
||||
btn.disabled = false;
|
||||
btn.classList.remove("text-muted");
|
||||
|
||||
@@ -126,7 +126,7 @@ async function reveal() {
|
||||
if (!isCurrentReveal(generation)) return;
|
||||
// Deliberately not the caught error: the message is fixed so that
|
||||
// nothing derived from the ciphertext or the attempt can surface.
|
||||
fail("That password is not correct. Please try again.");
|
||||
fail("That password is incorrect. Please try again.");
|
||||
} finally {
|
||||
btn.disabled = false;
|
||||
btn.classList.remove("text-muted");
|
||||
|
||||
213
src/shared/approvalTx.js
Normal file
213
src/shared/approvalTx.js
Normal file
@@ -0,0 +1,213 @@
|
||||
// Preparation of the transaction an approval screen displays.
|
||||
//
|
||||
// A dApp's eth_sendTransaction normally fixes only `to`, `value` and `data`.
|
||||
// The nonce, the gas limit and the fees have to be filled in from the network
|
||||
// before anything can be signed, and whoever fills them in decides what the
|
||||
// user is shown. That work used to happen in the popup, after the user had
|
||||
// already approved: the numbers on the approval screen came from the popup and
|
||||
// were compared against nothing, so a compromised popup could display one fee
|
||||
// and sign another, and the ceilings in approvalVerify.js were all that stood
|
||||
// between the user and a fee that hands the validator the balance.
|
||||
//
|
||||
// So it happens here instead, in the background, before the approval window is
|
||||
// opened. The background populates the transaction, shows that object, and
|
||||
// verifies the signed artifact against that same object — the popup is handed
|
||||
// a finished transaction and signs it as given. Every field the user reads is
|
||||
// then a field that is compared.
|
||||
//
|
||||
// The cost is an RPC round trip before the approval window exists. Nothing is
|
||||
// displayed while it is in flight, and a failure — an unreachable node, a
|
||||
// reverting gas estimate, a transaction type this wallet does not sign, a fee
|
||||
// past the ceilings — means no approval and no window at all: the error goes
|
||||
// back to the requesting page, which is where the user's click came from. That
|
||||
// is deliberate. The alternative, opening the window first and populating
|
||||
// behind a spinner, needs a pending approval that exists before it can be
|
||||
// displayed or signed, and a half-initialised approval is exactly the state
|
||||
// the settle interlock in the background exists to keep out of that record.
|
||||
// The failure also lands earlier than it used to rather than later: the same
|
||||
// estimate previously failed after the user had typed their password.
|
||||
|
||||
const {
|
||||
VoidSigner,
|
||||
accessListify,
|
||||
getAddress,
|
||||
getBytes,
|
||||
hexlify,
|
||||
toQuantity,
|
||||
} = require("ethers");
|
||||
const {
|
||||
ALLOWED_TX_TYPES,
|
||||
SERIALIZED_FIELDS,
|
||||
assertWithinCeilings,
|
||||
} = require("./approvalVerify");
|
||||
|
||||
// How long the population may take before the request is failed back to the
|
||||
// page. Without a bound a hung RPC endpoint leaves the dApp's promise pending
|
||||
// forever with nothing on screen to explain it; ethers' own request timeout is
|
||||
// minutes long, which is not a wait anyone will sit through.
|
||||
const POPULATE_TIMEOUT_MS = 20000;
|
||||
|
||||
// The request fields taken from the page. Anything else is dropped rather than
|
||||
// passed to ethers: the object is page-controlled, and a future ethers that
|
||||
// learns to carry a new transaction field must not start picking one up out of
|
||||
// it without this module knowing.
|
||||
const REQUEST_FIELDS = [
|
||||
"to",
|
||||
"value",
|
||||
"data",
|
||||
"nonce",
|
||||
"gasLimit",
|
||||
"gasPrice",
|
||||
"maxFeePerGas",
|
||||
"maxPriorityFeePerGas",
|
||||
"chainId",
|
||||
"accessList",
|
||||
"type",
|
||||
];
|
||||
|
||||
class ApprovalPrepareError extends Error {
|
||||
constructor(message) {
|
||||
super(message);
|
||||
this.name = "ApprovalPrepareError";
|
||||
}
|
||||
}
|
||||
|
||||
function fail(message) {
|
||||
return new ApprovalPrepareError(message);
|
||||
}
|
||||
|
||||
function present(v) {
|
||||
return v !== null && v !== undefined && v !== "";
|
||||
}
|
||||
|
||||
// These strings reach the user through the requesting page, so they are full
|
||||
// sentences even when the tail of one came from ethers or from the node.
|
||||
function sentence(text) {
|
||||
return /[.!?]$/.test(text) ? text : text + ".";
|
||||
}
|
||||
|
||||
// Reject a promise that has taken too long, and never leave the timer behind.
|
||||
async function withTimeout(promise, ms, message) {
|
||||
let timer = null;
|
||||
try {
|
||||
return await Promise.race([
|
||||
promise,
|
||||
new Promise((_resolve, reject) => {
|
||||
timer = setTimeout(() => reject(fail(message)), ms);
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
if (timer !== null) clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
|
||||
// The page's request, reduced to the fields this wallet acts on.
|
||||
function requestFrom(txParams, from) {
|
||||
const request = { from: getAddress(from) };
|
||||
for (const key of REQUEST_FIELDS) {
|
||||
if (present(txParams[key])) request[key] = txParams[key];
|
||||
}
|
||||
if (
|
||||
present(request.type) &&
|
||||
!ALLOWED_TX_TYPES.includes(Number(request.type))
|
||||
) {
|
||||
throw fail(
|
||||
"The site asked for a transaction of a type this wallet does not sign.",
|
||||
);
|
||||
}
|
||||
return request;
|
||||
}
|
||||
|
||||
// Turn a populated transaction into the object that crosses to the popup, is
|
||||
// displayed, and is compared with the signed artifact. It carries exactly the
|
||||
// fields its type serializes, plus the address it is to be signed by, and
|
||||
// every quantity as a hex string: extension messaging is JSON, which has no
|
||||
// bigint, and a field that did not survive the trip would be a field the user
|
||||
// was shown and nothing compared.
|
||||
function serializeApprovedTx(populated, from) {
|
||||
const type = Number(populated.type);
|
||||
if (!ALLOWED_TX_TYPES.includes(type)) {
|
||||
throw fail(
|
||||
"This transaction would have to be sent as a type this wallet does not sign.",
|
||||
);
|
||||
}
|
||||
const approved = { type, from: getAddress(from) };
|
||||
for (const key of SERIALIZED_FIELDS[type]) {
|
||||
if (key === "to") {
|
||||
approved.to = present(populated.to)
|
||||
? getAddress(populated.to)
|
||||
: null;
|
||||
} else if (key === "data") {
|
||||
approved.data = present(populated.data)
|
||||
? hexlify(getBytes(populated.data))
|
||||
: "0x";
|
||||
} else if (key === "accessList") {
|
||||
approved.accessList = accessListify(populated.accessList || []);
|
||||
} else if (key === "value") {
|
||||
approved.value = toQuantity(populated.value || 0);
|
||||
} else if (!present(populated[key])) {
|
||||
// Unreachable while populateTransaction() fills every quantity of
|
||||
// the type it produced. If it ever does not, the approval must not
|
||||
// be raised: an unfixed quantity is one the artifact cannot be
|
||||
// checked against.
|
||||
throw fail(
|
||||
"The transaction could not be prepared: the network did not supply a " +
|
||||
key +
|
||||
".",
|
||||
);
|
||||
} else {
|
||||
approved[key] = toQuantity(populated[key]);
|
||||
}
|
||||
}
|
||||
return approved;
|
||||
}
|
||||
|
||||
// Populate the transaction a site asked for, as the address it will be signed
|
||||
// by, and return the object to display, sign and verify against. Throws with a
|
||||
// full sentence when no approval can be raised.
|
||||
async function prepareApprovalTx(provider, from, txParams) {
|
||||
if (!present(from)) {
|
||||
throw fail("There is no active address to send this transaction from.");
|
||||
}
|
||||
const request = requestFrom(txParams || {}, from);
|
||||
|
||||
let populated;
|
||||
try {
|
||||
// The sequence ethers' own sendTransaction() runs internally, so the
|
||||
// nonce, gas, fee and chain id are populated exactly as they were when
|
||||
// the popup did this. VoidSigner cannot sign, which is the point: the
|
||||
// background prepares, the popup signs.
|
||||
populated = await withTimeout(
|
||||
new VoidSigner(getAddress(from), provider).populateTransaction(
|
||||
request,
|
||||
),
|
||||
POPULATE_TIMEOUT_MS,
|
||||
"The transaction could not be prepared: the network did not answer in time.",
|
||||
);
|
||||
} catch (e) {
|
||||
if (e instanceof ApprovalPrepareError) throw e;
|
||||
throw fail(
|
||||
sentence(
|
||||
"The transaction could not be prepared: " +
|
||||
(e.shortMessage ||
|
||||
e.message ||
|
||||
"the network did not answer"),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
const approved = serializeApprovedTx(populated, from);
|
||||
// The backstop, applied before the user is shown anything rather than
|
||||
// after they have approved it: what is displayed here is what gets signed,
|
||||
// so an RPC node reporting an absurd fee has to be refused here.
|
||||
assertWithinCeilings(approved);
|
||||
return approved;
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
prepareApprovalTx,
|
||||
serializeApprovedTx,
|
||||
ApprovalPrepareError,
|
||||
POPULATE_TIMEOUT_MS,
|
||||
REQUEST_FIELDS,
|
||||
};
|
||||
@@ -7,6 +7,13 @@
|
||||
// the signer from the artifact and checks it against the approval it is
|
||||
// holding before acting on it. All recovery is delegated to ethers.
|
||||
//
|
||||
// What the artifact is checked against is the transaction the background
|
||||
// populated and the popup displayed (see approvalTx.js), not the request the
|
||||
// dApp made. The two differ in every field a dApp normally leaves out — nonce,
|
||||
// gas limit, fees — and those are the fields the user reads off the approval
|
||||
// screen, so comparing against the request would leave the numbers on screen
|
||||
// vouched for by nothing.
|
||||
//
|
||||
// The check is an allowlist, in both directions, because a denylist cannot be
|
||||
// correct against a transaction format that keeps gaining fields:
|
||||
//
|
||||
@@ -31,14 +38,12 @@
|
||||
// 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.
|
||||
// The approved transaction is required to fix every field its type serializes,
|
||||
// so there is no "the approval did not say" branch to fall through: a quantity
|
||||
// the approval does not carry is a refusal, because an artifact that cannot be
|
||||
// compared with what was displayed has not been checked. The chain id is
|
||||
// checked against the selected network as well as against the approval, 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.
|
||||
@@ -113,6 +118,17 @@ const FORBIDDEN_FIELDS = [
|
||||
},
|
||||
];
|
||||
|
||||
// Absolute ceilings — a BACKSTOP, not the primary control.
|
||||
//
|
||||
// The primary control is equality: every field of the artifact is compared
|
||||
// with the populated transaction the user was shown, so nothing the popup
|
||||
// signs can differ from the screen. What equality cannot bound is the
|
||||
// populated transaction itself, which is built from what the configured RPC
|
||||
// node answered — a node that reports an absurd fee gets that fee displayed,
|
||||
// and a user who does not read the fee line would approve it. These ceilings
|
||||
// bound that, and they are therefore applied where the transaction is
|
||||
// populated (approvalTx.js) as well as here.
|
||||
//
|
||||
// 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;
|
||||
@@ -224,40 +240,151 @@ 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",
|
||||
// How each field of an approved transaction is compared with the artifact.
|
||||
// There is an entry here for every field any allowed type serializes — a test
|
||||
// pins that against SERIALIZED_FIELDS — so the comparison loop covers the
|
||||
// whole of what gets signed and cannot silently skip a field for want of a
|
||||
// comparator.
|
||||
//
|
||||
// `kind` decides how the two sides are made comparable. A `quantity` must be
|
||||
// fixed by the approval: it is one of the numbers on the approval screen, and
|
||||
// an absent one means the artifact cannot be checked against what was
|
||||
// displayed. `to`, `value`, `data` and `accessList` have canonical absent
|
||||
// forms — contract creation, zero, "0x" and the empty list — so they are
|
||||
// normalized on both sides instead.
|
||||
const APPROVED_FIELDS = {
|
||||
chainId: {
|
||||
kind: "quantity",
|
||||
label: "network",
|
||||
message:
|
||||
"The signed transaction is for a different network than the one that was approved.",
|
||||
},
|
||||
nonce: {
|
||||
kind: "quantity",
|
||||
label: "nonce",
|
||||
message: "The signed transaction does not carry the approved nonce.",
|
||||
},
|
||||
{
|
||||
key: "gasLimit",
|
||||
gasLimit: {
|
||||
kind: "quantity",
|
||||
label: "gas limit",
|
||||
message:
|
||||
"The signed transaction does not carry the approved gas limit.",
|
||||
},
|
||||
{
|
||||
key: "gasPrice",
|
||||
gasPrice: {
|
||||
kind: "quantity",
|
||||
label: "gas price",
|
||||
message:
|
||||
"The signed transaction does not carry the approved gas price.",
|
||||
},
|
||||
{
|
||||
key: "maxFeePerGas",
|
||||
maxFeePerGas: {
|
||||
kind: "quantity",
|
||||
label: "maximum fee per gas",
|
||||
message:
|
||||
"The signed transaction does not carry the approved maximum fee per gas.",
|
||||
},
|
||||
{
|
||||
key: "maxPriorityFeePerGas",
|
||||
maxPriorityFeePerGas: {
|
||||
kind: "quantity",
|
||||
label: "maximum priority fee per gas",
|
||||
message:
|
||||
"The signed transaction does not carry the approved maximum priority fee per gas.",
|
||||
},
|
||||
];
|
||||
to: {
|
||||
kind: "address",
|
||||
label: "recipient",
|
||||
message:
|
||||
"The signed transaction does not go to the approved recipient.",
|
||||
},
|
||||
value: {
|
||||
kind: "value",
|
||||
label: "value",
|
||||
message: "The signed transaction does not carry the approved value.",
|
||||
},
|
||||
data: {
|
||||
kind: "data",
|
||||
label: "call data",
|
||||
message:
|
||||
"The signed transaction does not carry the approved call data.",
|
||||
},
|
||||
accessList: {
|
||||
kind: "accessList",
|
||||
label: "access list",
|
||||
message:
|
||||
"The signed transaction does not carry the approved access list.",
|
||||
},
|
||||
};
|
||||
|
||||
// Compare one field of the artifact with the approved transaction. A field
|
||||
// with no entry in the table above is refused rather than skipped: the loop
|
||||
// below runs over the fields the type serializes, so an unmatched key means
|
||||
// something that gets signed has no comparator at all.
|
||||
function assertFieldMatches(key, parsed, approvedTx) {
|
||||
const field = APPROVED_FIELDS[key];
|
||||
if (!field) {
|
||||
throw refuse(
|
||||
"The signed transaction carries a field this wallet cannot compare with the approval.",
|
||||
);
|
||||
}
|
||||
switch (field.kind) {
|
||||
case "quantity": {
|
||||
if (!present(approvedTx[key])) {
|
||||
throw refuse(
|
||||
"The approved transaction fixes no " +
|
||||
field.label +
|
||||
", so the signed transaction cannot be checked" +
|
||||
" against what was shown.",
|
||||
);
|
||||
}
|
||||
const approved = normalizeQuantity(approvedTx[key], field.label);
|
||||
if (normalizeQuantity(parsed[key], field.label) !== approved) {
|
||||
throw refuse(field.message);
|
||||
}
|
||||
return;
|
||||
}
|
||||
case "address":
|
||||
if (!sameAddress(parsed[key], approvedTx[key])) {
|
||||
throw refuse(field.message);
|
||||
}
|
||||
return;
|
||||
case "value":
|
||||
if (normalizeValue(parsed[key]) !== normalizeValue(approvedTx[key]))
|
||||
throw refuse(field.message);
|
||||
return;
|
||||
case "data":
|
||||
if (normalizeData(parsed[key]) !== normalizeData(approvedTx[key]))
|
||||
throw refuse(field.message);
|
||||
return;
|
||||
default:
|
||||
if (
|
||||
normalizeAccessList(parsed[key]) !==
|
||||
normalizeAccessList(approvedTx[key])
|
||||
) {
|
||||
throw refuse(field.message);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The ceilings, applied to a transaction that is either about to be displayed
|
||||
// or about to be broadcast. See MAX_GAS_LIMIT above for what they are for:
|
||||
// they bound what the RPC node can talk this wallet into showing the user,
|
||||
// which is the one thing comparing the artifact with the screen cannot do.
|
||||
function assertWithinCeilings(tx) {
|
||||
if (
|
||||
present(tx.gasLimit) &&
|
||||
normalizeQuantity(tx.gasLimit, "gas limit") > 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"]) {
|
||||
if (!present(tx[key])) continue;
|
||||
if (normalizeQuantity(tx[key], "fee per gas") > MAX_FEE_PER_GAS) {
|
||||
throw refuse(
|
||||
"The signed transaction sets a fee per gas far above any plausible value.",
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
@@ -317,10 +444,28 @@ function assertCanonicalBytes(parsed, rawSignedTx) {
|
||||
// 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) {
|
||||
//
|
||||
// `approvedTx` is the populated transaction the approval screen displayed, and
|
||||
// `expectedFrom` is the address that was active when the approval was raised —
|
||||
// not whichever address is active now. An address switch between approval and
|
||||
// signing therefore refuses here rather than producing a transaction from an
|
||||
// account the approval did not name.
|
||||
function verifySignedTx(
|
||||
rawSignedTx,
|
||||
approvedTx,
|
||||
expectedFrom,
|
||||
selectedChainId,
|
||||
) {
|
||||
if (typeof rawSignedTx !== "string" || !rawSignedTx.startsWith("0x")) {
|
||||
throw refuse("The signed transaction is missing or malformed.");
|
||||
}
|
||||
// Nothing to compare against is a refusal like any other: an approval that
|
||||
// does not carry the transaction it displayed cannot vouch for one.
|
||||
if (!approvedTx || typeof approvedTx !== "object") {
|
||||
throw refuse(
|
||||
"There is no approved transaction to check the signed transaction against.",
|
||||
);
|
||||
}
|
||||
|
||||
let parsed;
|
||||
try {
|
||||
@@ -360,46 +505,15 @@ function verifySignedTx(rawSignedTx, txParams, expectedFrom, selectedChainId) {
|
||||
"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 refuse(
|
||||
"The signed transaction does not go to the approved recipient.",
|
||||
);
|
||||
}
|
||||
if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) {
|
||||
throw refuse(
|
||||
"The signed transaction does not carry the approved value.",
|
||||
);
|
||||
}
|
||||
if (normalizeData(parsed.data) !== normalizeData(txParams.data)) {
|
||||
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.
|
||||
// The approved fee mechanism, named before the type comparison below
|
||||
// subsumes it: the fee the user agreed to is only meaningful under the
|
||||
// mechanism it was quoted in, and saying so is more use than "a different
|
||||
// transaction type".
|
||||
const approvedEip1559 =
|
||||
present(txParams.maxFeePerGas) ||
|
||||
present(txParams.maxPriorityFeePerGas);
|
||||
const approvedLegacy = present(txParams.gasPrice);
|
||||
present(approvedTx.maxFeePerGas) ||
|
||||
present(approvedTx.maxPriorityFeePerGas);
|
||||
const approvedLegacy = present(approvedTx.gasPrice);
|
||||
const signedEip1559 = parsed.type === 2;
|
||||
if (
|
||||
(approvedEip1559 && !signedEip1559) ||
|
||||
@@ -410,28 +524,33 @@ function verifySignedTx(rawSignedTx, txParams, expectedFrom, selectedChainId) {
|
||||
);
|
||||
}
|
||||
|
||||
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) {
|
||||
// The type decides which fields are compared, so it is compared first and
|
||||
// against the approval, not merely checked for membership of the
|
||||
// allowlist above.
|
||||
if (!present(approvedTx.type)) {
|
||||
throw refuse(
|
||||
"The signed transaction sets a gas limit no network this wallet supports can accept.",
|
||||
"The approved transaction fixes no transaction type, so the signed transaction cannot be checked against what was shown.",
|
||||
);
|
||||
}
|
||||
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.",
|
||||
);
|
||||
}
|
||||
if (
|
||||
BigInt(parsed.type) !==
|
||||
normalizeQuantity(approvedTx.type, "transaction type")
|
||||
) {
|
||||
throw refuse(
|
||||
"The signed transaction does not use the approved transaction type.",
|
||||
);
|
||||
}
|
||||
|
||||
// Every field this type serializes, compared with the transaction the user
|
||||
// was shown. Driving the loop off SERIALIZED_FIELDS is what keeps this
|
||||
// exhaustive: the same table decides what assertNothingUnchecked() rebuilds
|
||||
// from, so a field that gets signed and is not compared here cannot exist.
|
||||
for (const key of SERIALIZED_FIELDS[parsed.type]) {
|
||||
assertFieldMatches(key, parsed, approvedTx);
|
||||
}
|
||||
|
||||
assertWithinCeilings(parsed);
|
||||
|
||||
assertNothingUnchecked(parsed);
|
||||
assertCanonicalBytes(parsed, rawSignedTx);
|
||||
|
||||
@@ -504,10 +623,10 @@ function errorText(err) {
|
||||
// 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.
|
||||
// node answering "already known"), so the wallet cannot tell a transaction
|
||||
// that never left from one that is already in the mempool. The approval is
|
||||
// spent and the requesting page has been given its outcome; a second
|
||||
// attempt against it would report a second outcome for one request.
|
||||
function describeTxFailure(stage, err) {
|
||||
const error = errorText(err);
|
||||
const retryable =
|
||||
@@ -553,6 +672,7 @@ module.exports = {
|
||||
assertNoForbiddenFields,
|
||||
assertNothingUnchecked,
|
||||
assertCanonicalBytes,
|
||||
assertWithinCeilings,
|
||||
sameAddress,
|
||||
failureIsRetryable,
|
||||
describeTxFailure,
|
||||
@@ -561,6 +681,7 @@ module.exports = {
|
||||
ALLOWED_TX_TYPES,
|
||||
SERIALIZED_FIELDS,
|
||||
FORBIDDEN_FIELDS,
|
||||
APPROVED_FIELDS,
|
||||
TX_STAGE_SIGN,
|
||||
TX_STAGE_VERIFY,
|
||||
TX_STAGE_BROADCAST,
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
const { DEFAULT_RPC_URL, DEFAULT_BLOCKSCOUT_URL } = require("./constants");
|
||||
const { networkById } = require("./networks");
|
||||
// Dependency-free constant module; safe to pull into a background bundle.
|
||||
const { RESTORABLE_VIEWS } = require("../popup/restorableViews");
|
||||
|
||||
const storageApi =
|
||||
typeof browser !== "undefined"
|
||||
@@ -43,6 +45,39 @@ const state = {
|
||||
viewStack: [],
|
||||
};
|
||||
|
||||
// Keep only the leading run of stored views the popup is willing to render.
|
||||
//
|
||||
// restoreView() refuses to reopen ONTO a non-restorable view, but the stack
|
||||
// behind it used to be restored verbatim, so Back could walk onto a screen
|
||||
// whose content is deliberately never re-rendered — and "show-phrase" has no
|
||||
// Back control to leave by. Truncating at the first such entry instead of
|
||||
// splicing it out keeps the result a prefix of the stored stack, so every
|
||||
// surviving entry's Back target is exactly the one it had; splicing would
|
||||
// silently re-point the entry above the hole at a different screen.
|
||||
//
|
||||
// Filtering happens here on load rather than in saveState(): the live
|
||||
// in-session stack is legitimate (the screen really is rendered while the
|
||||
// popup is open), and only a load-side filter also repairs the stacks
|
||||
// already in storage, including ones written before a view left the set.
|
||||
function restorableStack(stored, currentView) {
|
||||
// A stored stack that is missing or not an array keeps nothing, but it
|
||||
// still goes through the never-empty rule below rather than returning
|
||||
// early: otherwise a corrupt stack would depend on exactly the goBack()
|
||||
// fallback that the explicit ["main"] exists in order not to depend on.
|
||||
const source = Array.isArray(stored) ? stored : [];
|
||||
const cut = source.findIndex((view) => !RESTORABLE_VIEWS.has(view));
|
||||
const kept = cut === -1 ? source.slice() : source.slice(0, cut);
|
||||
// A view restored below the root still needs somewhere for Back to go.
|
||||
if (
|
||||
kept.length === 0 &&
|
||||
currentView !== "main" &&
|
||||
RESTORABLE_VIEWS.has(currentView)
|
||||
) {
|
||||
return ["main"];
|
||||
}
|
||||
return kept;
|
||||
}
|
||||
|
||||
// Return the network configuration for the currently selected network.
|
||||
function currentNetwork() {
|
||||
return networkById(state.networkId);
|
||||
@@ -150,7 +185,7 @@ async function loadState() {
|
||||
saved.selectedAddress !== undefined ? saved.selectedAddress : null;
|
||||
state.selectedToken = saved.selectedToken || null;
|
||||
state.viewData = saved.viewData || {};
|
||||
state.viewStack = Array.isArray(saved.viewStack) ? saved.viewStack : [];
|
||||
state.viewStack = restorableStack(saved.viewStack, state.currentView);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
309
tests/approvalTx.test.js
Normal file
309
tests/approvalTx.test.js
Normal file
@@ -0,0 +1,309 @@
|
||||
// Preparation of the transaction the approval screen displays.
|
||||
//
|
||||
// This is the half of the fix that makes the verification in
|
||||
// approvalVerify.test.js mean anything: the numbers the user reads have to be
|
||||
// produced before the screen is drawn and be the numbers that get signed. What
|
||||
// is asserted here is that the object leaving this module is complete (nothing
|
||||
// is left for the popup to fill in), that it survives the messaging boundary
|
||||
// (extension messaging is JSON, which has no bigint), and that nothing the
|
||||
// requesting page or the RPC node can say turns it into an approval that
|
||||
// should never have been raised.
|
||||
|
||||
const { Network, Wallet } = require("ethers");
|
||||
const {
|
||||
prepareApprovalTx,
|
||||
serializeApprovedTx,
|
||||
POPULATE_TIMEOUT_MS,
|
||||
} = require("../src/shared/approvalTx");
|
||||
const {
|
||||
SERIALIZED_FIELDS,
|
||||
MAX_FEE_PER_GAS,
|
||||
MAX_GAS_LIMIT,
|
||||
} = require("../src/shared/approvalVerify");
|
||||
|
||||
const SIGNER_KEY =
|
||||
"0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d";
|
||||
const signer = new Wallet(SIGNER_KEY);
|
||||
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
|
||||
|
||||
// The ordinary dApp request: recipient, value, call data, and nothing else.
|
||||
const TX_PARAMS = {
|
||||
from: signer.address,
|
||||
to: RECIPIENT,
|
||||
value: "0x2386f26fc10000",
|
||||
data: "0xdeadbeef",
|
||||
};
|
||||
|
||||
function providerWith(overrides) {
|
||||
return {
|
||||
getNetwork: async () => Network.from(1),
|
||||
getTransactionCount: async () => 7,
|
||||
estimateGas: async () => 21000n,
|
||||
getFeeData: async () => ({
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: 2000000000n,
|
||||
maxPriorityFeePerGas: 1000000000n,
|
||||
}),
|
||||
...(overrides || {}),
|
||||
};
|
||||
}
|
||||
|
||||
// A node that only quotes a flat gas price, so populateTransaction produces a
|
||||
// legacy transaction rather than an EIP-1559 one.
|
||||
const legacyProvider = providerWith({
|
||||
getFeeData: async () => ({
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
}),
|
||||
});
|
||||
|
||||
describe("prepareApprovalTx", () => {
|
||||
test("fills in everything the request left out", async () => {
|
||||
const approved = await prepareApprovalTx(
|
||||
providerWith(),
|
||||
signer.address,
|
||||
TX_PARAMS,
|
||||
);
|
||||
expect(approved).toEqual({
|
||||
type: 2,
|
||||
from: signer.address,
|
||||
chainId: "0x1",
|
||||
nonce: "0x7",
|
||||
gasLimit: "0x5208",
|
||||
maxPriorityFeePerGas: "0x3b9aca00",
|
||||
maxFeePerGas: "0x77359400",
|
||||
to: RECIPIENT,
|
||||
value: TX_PARAMS.value,
|
||||
data: TX_PARAMS.data,
|
||||
accessList: [],
|
||||
});
|
||||
});
|
||||
|
||||
// The object is displayed, signed and verified against on the far side of
|
||||
// chrome.runtime.sendMessage, which is JSON: a bigint would throw on the
|
||||
// way out and a field that did not survive the trip would be a field the
|
||||
// user was shown and nothing compared.
|
||||
test("survives the messaging boundary unchanged", async () => {
|
||||
const approved = await prepareApprovalTx(
|
||||
providerWith(),
|
||||
signer.address,
|
||||
TX_PARAMS,
|
||||
);
|
||||
expect(JSON.parse(JSON.stringify(approved))).toEqual(approved);
|
||||
for (const value of Object.values(approved)) {
|
||||
expect(typeof value).not.toBe("bigint");
|
||||
}
|
||||
});
|
||||
|
||||
test("carries exactly the fields its type serializes, and the signer", async () => {
|
||||
const approved = await prepareApprovalTx(
|
||||
providerWith(),
|
||||
signer.address,
|
||||
TX_PARAMS,
|
||||
);
|
||||
expect(Object.keys(approved).sort()).toEqual(
|
||||
["type", "from", ...SERIALIZED_FIELDS[2]].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
test("produces a legacy transaction when that is all the node quotes", async () => {
|
||||
const approved = await prepareApprovalTx(
|
||||
legacyProvider,
|
||||
signer.address,
|
||||
TX_PARAMS,
|
||||
);
|
||||
expect(approved.type).toBe(0);
|
||||
expect(approved.gasPrice).toBe("0x77359400");
|
||||
expect(approved.maxFeePerGas).toBeUndefined();
|
||||
expect(Object.keys(approved).sort()).toEqual(
|
||||
["type", "from", ...SERIALIZED_FIELDS[0]].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
test("keeps a nonce, gas limit and fee the request did fix", async () => {
|
||||
const approved = await prepareApprovalTx(
|
||||
providerWith(),
|
||||
signer.address,
|
||||
{
|
||||
...TX_PARAMS,
|
||||
nonce: "0x2",
|
||||
gasLimit: "0x30d40",
|
||||
maxFeePerGas: "0x12a05f200",
|
||||
maxPriorityFeePerGas: "0x3b9aca00",
|
||||
},
|
||||
);
|
||||
expect(approved.nonce).toBe("0x2");
|
||||
expect(approved.gasLimit).toBe("0x30d40");
|
||||
expect(approved.maxFeePerGas).toBe("0x12a05f200");
|
||||
});
|
||||
|
||||
test("carries an access list the request asked for", async () => {
|
||||
const approved = await prepareApprovalTx(
|
||||
providerWith(),
|
||||
signer.address,
|
||||
{
|
||||
...TX_PARAMS,
|
||||
accessList: [{ address: RECIPIENT, storageKeys: [] }],
|
||||
},
|
||||
);
|
||||
expect(approved.accessList).toEqual([
|
||||
{ address: RECIPIENT, storageKeys: [] },
|
||||
]);
|
||||
});
|
||||
|
||||
// The request is page-controlled. Anything this wallet does not act on is
|
||||
// dropped before ethers sees it, so a field a future ethers learns to
|
||||
// carry cannot be picked up out of it without this module knowing.
|
||||
test("drops request fields this wallet does not act on", async () => {
|
||||
const approved = await prepareApprovalTx(
|
||||
providerWith(),
|
||||
signer.address,
|
||||
{
|
||||
...TX_PARAMS,
|
||||
authorizationList: [{ address: RECIPIENT }],
|
||||
blobVersionedHashes: ["0x01" + "ab".repeat(31)],
|
||||
customData: { anything: true },
|
||||
},
|
||||
);
|
||||
expect(approved.authorizationList).toBeUndefined();
|
||||
expect(approved.blobVersionedHashes).toBeUndefined();
|
||||
expect(approved.customData).toBeUndefined();
|
||||
expect(approved.type).toBe(2);
|
||||
});
|
||||
|
||||
test("refuses a transaction type this wallet does not sign", async () => {
|
||||
await expect(
|
||||
prepareApprovalTx(providerWith(), signer.address, {
|
||||
...TX_PARAMS,
|
||||
type: 4,
|
||||
}),
|
||||
).rejects.toThrow(/type this wallet does not sign/);
|
||||
});
|
||||
|
||||
test("refuses to raise an approval with no active address", async () => {
|
||||
await expect(
|
||||
prepareApprovalTx(providerWith(), null, TX_PARAMS),
|
||||
).rejects.toThrow(/no active address/);
|
||||
});
|
||||
|
||||
// The ceilings as a backstop: equality with the screen cannot bound what
|
||||
// the node talks the wallet into putting on the screen, so it is refused
|
||||
// before the user is shown anything.
|
||||
test("refuses a fee the node quoted above the ceiling", async () => {
|
||||
const gouging = providerWith({
|
||||
getFeeData: async () => ({
|
||||
gasPrice: MAX_FEE_PER_GAS + 1n,
|
||||
maxFeePerGas: MAX_FEE_PER_GAS + 1n,
|
||||
maxPriorityFeePerGas: 1000000000n,
|
||||
}),
|
||||
});
|
||||
await expect(
|
||||
prepareApprovalTx(gouging, signer.address, TX_PARAMS),
|
||||
).rejects.toThrow(/fee per gas far above any plausible value/);
|
||||
});
|
||||
|
||||
test("refuses a gas limit the node estimated above the ceiling", async () => {
|
||||
const absurd = providerWith({
|
||||
estimateGas: async () => MAX_GAS_LIMIT + 1n,
|
||||
});
|
||||
await expect(
|
||||
prepareApprovalTx(absurd, signer.address, TX_PARAMS),
|
||||
).rejects.toThrow(/gas limit no network this wallet supports/);
|
||||
});
|
||||
|
||||
// No approval and no window: the failure goes back to the page the click
|
||||
// came from, in a sentence.
|
||||
test("reports a failed estimate as a full sentence", async () => {
|
||||
const reverting = providerWith({
|
||||
estimateGas: async () => {
|
||||
throw new Error("execution reverted: ERC20: transfer amount");
|
||||
},
|
||||
});
|
||||
let thrown;
|
||||
try {
|
||||
await prepareApprovalTx(reverting, signer.address, TX_PARAMS);
|
||||
} catch (e) {
|
||||
thrown = e;
|
||||
}
|
||||
expect(thrown.message).toMatch(
|
||||
/^The transaction could not be prepared/,
|
||||
);
|
||||
expect(thrown.message).toMatch(/execution reverted/);
|
||||
expect(thrown.message).toMatch(/^[A-Z].*\.$/);
|
||||
});
|
||||
|
||||
// Without a bound, an unreachable node leaves the page's promise pending
|
||||
// with nothing on screen to explain it.
|
||||
test("gives up on a node that never answers", async () => {
|
||||
jest.useFakeTimers();
|
||||
try {
|
||||
const hanging = providerWith({
|
||||
estimateGas: () => new Promise(() => {}),
|
||||
});
|
||||
const pending = prepareApprovalTx(
|
||||
hanging,
|
||||
signer.address,
|
||||
TX_PARAMS,
|
||||
);
|
||||
const settled = expect(pending).rejects.toThrow(
|
||||
/did not answer in time/,
|
||||
);
|
||||
await jest.advanceTimersByTimeAsync(POPULATE_TIMEOUT_MS + 1);
|
||||
await settled;
|
||||
} finally {
|
||||
jest.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("serializeApprovedTx", () => {
|
||||
// Unreachable through prepareApprovalTx while the request type is checked
|
||||
// first, which is what it is for: a node or an ethers upgrade that
|
||||
// populates a type this wallet does not sign must not produce an approval.
|
||||
test("refuses a populated transaction of a type this wallet does not sign", () => {
|
||||
expect(() =>
|
||||
serializeApprovedTx(
|
||||
{ type: 3, to: RECIPIENT, nonce: 7 },
|
||||
signer.address,
|
||||
),
|
||||
).toThrow(/type this wallet does not sign/);
|
||||
});
|
||||
|
||||
test("refuses a populated transaction missing a quantity", () => {
|
||||
expect(() =>
|
||||
serializeApprovedTx(
|
||||
{
|
||||
type: 2,
|
||||
chainId: 1n,
|
||||
nonce: 7,
|
||||
gasLimit: 21000n,
|
||||
maxFeePerGas: 2000000000n,
|
||||
to: RECIPIENT,
|
||||
value: 0n,
|
||||
data: "0x",
|
||||
},
|
||||
signer.address,
|
||||
),
|
||||
).toThrow(/did not supply a maxPriorityFeePerGas/);
|
||||
});
|
||||
|
||||
test("keeps a contract creation's absent recipient absent", () => {
|
||||
const approved = serializeApprovedTx(
|
||||
{
|
||||
type: 0,
|
||||
chainId: 1n,
|
||||
nonce: 7,
|
||||
gasPrice: 2000000000n,
|
||||
gasLimit: 21000n,
|
||||
to: null,
|
||||
value: 0n,
|
||||
data: "0x600160005500",
|
||||
},
|
||||
signer.address,
|
||||
);
|
||||
expect(approved.to).toBeNull();
|
||||
expect(approved.value).toBe("0x0");
|
||||
expect(approved.data).toBe("0x600160005500");
|
||||
});
|
||||
});
|
||||
@@ -11,6 +11,7 @@ const {
|
||||
assertNoForbiddenFields,
|
||||
assertNothingUnchecked,
|
||||
assertCanonicalBytes,
|
||||
assertWithinCeilings,
|
||||
sameAddress,
|
||||
failureIsRetryable,
|
||||
describeTxFailure,
|
||||
@@ -18,12 +19,14 @@ const {
|
||||
ALLOWED_TX_TYPES,
|
||||
SERIALIZED_FIELDS,
|
||||
FORBIDDEN_FIELDS,
|
||||
APPROVED_FIELDS,
|
||||
TX_STAGE_SIGN,
|
||||
TX_STAGE_VERIFY,
|
||||
TX_STAGE_BROADCAST,
|
||||
MAX_GAS_LIMIT,
|
||||
MAX_FEE_PER_GAS,
|
||||
} = require("../src/shared/approvalVerify");
|
||||
const { prepareApprovalTx } = require("../src/shared/approvalTx");
|
||||
const { getSignerForAddress } = require("../src/shared/wallet");
|
||||
|
||||
// Fixed test keys — never used for anything but these tests.
|
||||
@@ -42,7 +45,10 @@ const OTHER_RECIPIENT = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
|
||||
const SELECTED = "0x1";
|
||||
const SEPOLIA = "0xaa36a7";
|
||||
|
||||
// Approved parameters as a dApp would supply them over eth_sendTransaction.
|
||||
// Parameters as a dApp would supply them over eth_sendTransaction. Note what
|
||||
// is missing: nonce, gas limit and fees. The background fills those in before
|
||||
// the approval screen is drawn, which is why the approval below and not this
|
||||
// object is what every comparison runs against.
|
||||
const TX_PARAMS = {
|
||||
from: signer.address,
|
||||
to: RECIPIENT,
|
||||
@@ -61,8 +67,8 @@ const POPULATED = {
|
||||
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
|
||||
// Build a signable transaction from a request. The background populates the
|
||||
// same fields through populateTransaction(); here they are fixed so the test
|
||||
// needs no provider. `overrides` stands in for what a tampered or misbuilt
|
||||
// popup would put on the wire.
|
||||
function txFor(params, overrides) {
|
||||
@@ -75,6 +81,21 @@ function txFor(params, overrides) {
|
||||
};
|
||||
}
|
||||
|
||||
// The populated transaction the approval screen displayed, which is the object
|
||||
// the artifact is verified against. Built from the same fields as the signable
|
||||
// transaction above, because that is the point: displayed and verified are one
|
||||
// object.
|
||||
function approvedFor(params, overrides) {
|
||||
return {
|
||||
from: signer.address,
|
||||
accessList: [],
|
||||
...txFor(params, overrides),
|
||||
};
|
||||
}
|
||||
|
||||
// The ordinary case: the dApp's request, populated.
|
||||
const APPROVED = approvedFor(TX_PARAMS);
|
||||
|
||||
async function signedFor(params, withWallet, overrides) {
|
||||
return (withWallet || signer).signTransaction(txFor(params, overrides));
|
||||
}
|
||||
@@ -108,7 +129,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, SELECTED);
|
||||
const parsed = verifySignedTx(raw, APPROVED, signer.address, SELECTED);
|
||||
expect(parsed.from).toBe(signer.address);
|
||||
expect(parsed.hash).toBe(Transaction.from(raw).hash);
|
||||
});
|
||||
@@ -117,23 +138,23 @@ describe("verifySignedTx", () => {
|
||||
const params = { to: undefined, value: "0x0", data: "0x600160005500" };
|
||||
const raw = await signedFor(params);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, params, signer.address, SELECTED),
|
||||
verifySignedTx(raw, approvedFor(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);
|
||||
const params = { to: RECIPIENT, data: "0x" };
|
||||
const raw = await signedFor(params);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
verifySignedTx(raw, approvedFor(params), signer.address, SELECTED),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test("accepts call data whose case differs from the approval", async () => {
|
||||
const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" };
|
||||
const raw = await signedFor(approved);
|
||||
const params = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" };
|
||||
const raw = await signedFor(params);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
verifySignedTx(raw, approvedFor(params), signer.address, SELECTED),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
@@ -143,7 +164,7 @@ describe("verifySignedTx", () => {
|
||||
to: OTHER_RECIPIENT,
|
||||
});
|
||||
expect(() =>
|
||||
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/approved recipient/);
|
||||
});
|
||||
|
||||
@@ -153,47 +174,64 @@ describe("verifySignedTx", () => {
|
||||
value: "0x4563918244f40000",
|
||||
});
|
||||
expect(() =>
|
||||
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(raw, APPROVED, 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, SELECTED),
|
||||
verifySignedTx(raw, APPROVED, 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, SELECTED),
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/different address/);
|
||||
});
|
||||
|
||||
// The address the approval named, not whichever address is active when the
|
||||
// artifact comes back: an approval raised for one account cannot be
|
||||
// satisfied by a signature from another, whatever the wallet switched to
|
||||
// in between.
|
||||
test("rejects a signature from the address that is active now", async () => {
|
||||
const raw = await signedFor(TX_PARAMS, other);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/different address than the one that was approved/);
|
||||
// The same artifact against the same approval, verified for the other
|
||||
// address, is what would have happened had expectedFrom been read from
|
||||
// the wallet's current state.
|
||||
expect(() =>
|
||||
verifySignedTx(raw, APPROVED, other.address, SELECTED),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test("rejects an unsigned transaction", () => {
|
||||
const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized;
|
||||
expect(() =>
|
||||
verifySignedTx(unsigned, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(unsigned, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/no valid signature/);
|
||||
});
|
||||
|
||||
test("rejects a missing or malformed payload", () => {
|
||||
expect(() =>
|
||||
verifySignedTx(undefined, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(undefined, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/missing or malformed/);
|
||||
expect(() =>
|
||||
verifySignedTx("nope", TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx("nope", APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/missing or malformed/);
|
||||
expect(() =>
|
||||
verifySignedTx("0xc0ffee", TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx("0xc0ffee", APPROVED, 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, SELECTED);
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED);
|
||||
throw new Error("expected a rejection");
|
||||
} catch (e) {
|
||||
expect(e.message).toMatch(/^[A-Z].*\.$/);
|
||||
@@ -201,20 +239,113 @@ describe("verifySignedTx", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// The defect this file's approvals now stand against: for every field the dApp
|
||||
// left out, the old comparison had nothing to compare and skipped the field,
|
||||
// so the fee and the nonce the user read off the screen were checked by the
|
||||
// ceilings alone. A populated approval fixes all of them, and an approval that
|
||||
// does not fix one is a refusal rather than a pass.
|
||||
describe("verifySignedTx against what was displayed", () => {
|
||||
test("a fee differing from the displayed one is refused", async () => {
|
||||
// Ten times the fee the screen showed, and far below the ceiling: the
|
||||
// artifact the old comparison would have accepted.
|
||||
const inflated = 20000000000n;
|
||||
expect(inflated).toBeLessThan(MAX_FEE_PER_GAS);
|
||||
const raw = await signedWith({ maxFeePerGas: inflated });
|
||||
expect(() =>
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/approved maximum fee per gas/);
|
||||
});
|
||||
|
||||
test("a nonce differing from the displayed one is refused", async () => {
|
||||
const raw = await signedWith({ nonce: 8 });
|
||||
expect(() =>
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/approved nonce/);
|
||||
});
|
||||
|
||||
test("a gas limit differing from the displayed one is refused", async () => {
|
||||
const raw = await signedWith({ gasLimit: 250000n });
|
||||
expect(() =>
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/approved gas limit/);
|
||||
});
|
||||
|
||||
test("an approval fixing no quantity is refused, not waved through", async () => {
|
||||
const raw = await signedWith({});
|
||||
for (const key of [
|
||||
"chainId",
|
||||
"nonce",
|
||||
"gasLimit",
|
||||
"maxFeePerGas",
|
||||
"maxPriorityFeePerGas",
|
||||
]) {
|
||||
const incomplete = { ...APPROVED };
|
||||
delete incomplete[key];
|
||||
let thrown;
|
||||
try {
|
||||
verifySignedTx(raw, incomplete, signer.address, SELECTED);
|
||||
throw new Error("expected a rejection for " + key);
|
||||
} catch (e) {
|
||||
thrown = e;
|
||||
}
|
||||
expect(thrown.message).toMatch(/fixes no /);
|
||||
expect(thrown.approvalMismatch).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
test("no approved transaction at all is refused", async () => {
|
||||
const raw = await signedWith({});
|
||||
for (const approved of [undefined, null, "0xdeadbeef"]) {
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
).toThrow(/no approved transaction/);
|
||||
}
|
||||
});
|
||||
|
||||
test("an approval fixing no transaction type is refused", async () => {
|
||||
const raw = await signedWith({});
|
||||
const incomplete = { ...APPROVED };
|
||||
delete incomplete.type;
|
||||
expect(() =>
|
||||
verifySignedTx(raw, incomplete, signer.address, SELECTED),
|
||||
).toThrow(/fixes no transaction type/);
|
||||
});
|
||||
|
||||
test("an artifact of a type other than the approved one is refused", async () => {
|
||||
// Same fee mechanism on both sides, so only the type differs: a type 1
|
||||
// artifact against a type 2 approval.
|
||||
const approved = approvedFor(TX_PARAMS, {
|
||||
type: 1,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
});
|
||||
const raw = await signedWith({
|
||||
type: 0,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
});
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
).toThrow(/approved transaction type/);
|
||||
});
|
||||
});
|
||||
|
||||
// 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),
|
||||
verifySignedTx(raw, APPROVED, 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 };
|
||||
// Selected network and signed chain id agree; the approval was raised
|
||||
// for a different chain, so the artifact is not what was approved.
|
||||
const approved = { ...APPROVED, chainId: SEPOLIA };
|
||||
const raw = await signedWith({});
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
@@ -224,58 +355,59 @@ describe("verifySignedTx field comparison", () => {
|
||||
test("refuses when the selected network is unknown", async () => {
|
||||
const raw = await signedWith({});
|
||||
expect(() =>
|
||||
verifySignedTx(raw, TX_PARAMS, signer.address, undefined),
|
||||
verifySignedTx(raw, APPROVED, 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),
|
||||
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),
|
||||
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),
|
||||
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),
|
||||
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,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
};
|
||||
const raw = await signedWith(legacy);
|
||||
const approved = approvedFor(TX_PARAMS, legacy);
|
||||
const raw = await signedWith({ ...legacy, gasPrice: 9000000000n });
|
||||
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 approved = approvedFor(TX_PARAMS, {
|
||||
type: 0,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
});
|
||||
const raw = await signedWith({});
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
@@ -283,7 +415,6 @@ describe("verifySignedTx field comparison", () => {
|
||||
});
|
||||
|
||||
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,
|
||||
@@ -291,36 +422,72 @@ describe("verifySignedTx field comparison", () => {
|
||||
maxPriorityFeePerGas: null,
|
||||
});
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/approved fee mechanism/);
|
||||
});
|
||||
|
||||
// The ceilings are a backstop against what the RPC node can talk the
|
||||
// wallet into populating and displaying, so they are checked against an
|
||||
// approval that carries the absurd value too — equality alone would accept
|
||||
// it, which is exactly what the ceiling is there for.
|
||||
test("rejects a gas limit above anything a supported network accepts", async () => {
|
||||
const raw = await signedWith({ gasLimit: MAX_GAS_LIMIT + 1n });
|
||||
const overrides = { gasLimit: MAX_GAS_LIMIT + 1n };
|
||||
const raw = await signedWith(overrides);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(
|
||||
raw,
|
||||
approvedFor(TX_PARAMS, overrides),
|
||||
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({
|
||||
test("rejects an absurd fee per gas even when it was displayed", async () => {
|
||||
const overrides = {
|
||||
maxFeePerGas: MAX_FEE_PER_GAS + 1n,
|
||||
maxPriorityFeePerGas: MAX_FEE_PER_GAS + 1n,
|
||||
});
|
||||
};
|
||||
const raw = await signedWith(overrides);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(
|
||||
raw,
|
||||
approvedFor(TX_PARAMS, overrides),
|
||||
signer.address,
|
||||
SELECTED,
|
||||
),
|
||||
).toThrow(/fee per gas far above any plausible value/);
|
||||
});
|
||||
|
||||
test("assertWithinCeilings is the same check on either side of the screen", () => {
|
||||
expect(() =>
|
||||
assertWithinCeilings({ gasLimit: MAX_GAS_LIMIT + 1n }),
|
||||
).toThrow(/gas limit no network this wallet supports/);
|
||||
for (const key of [
|
||||
"gasPrice",
|
||||
"maxFeePerGas",
|
||||
"maxPriorityFeePerGas",
|
||||
]) {
|
||||
expect(() =>
|
||||
assertWithinCeilings({ [key]: MAX_FEE_PER_GAS + 1n }),
|
||||
).toThrow(/fee per gas far above any plausible value/);
|
||||
}
|
||||
expect(() =>
|
||||
assertWithinCeilings({
|
||||
gasLimit: MAX_GAS_LIMIT,
|
||||
maxFeePerGas: MAX_FEE_PER_GAS,
|
||||
maxPriorityFeePerGas: MAX_FEE_PER_GAS,
|
||||
}),
|
||||
).not.toThrow();
|
||||
// Nothing to bound is not a failure: a type 2 approval carries no gas
|
||||
// price, and a bare object must not be refused for lacking one.
|
||||
expect(() => assertWithinCeilings({})).not.toThrow();
|
||||
});
|
||||
|
||||
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,
|
||||
);
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED);
|
||||
throw new Error("expected a rejection");
|
||||
} catch (e) {
|
||||
expect(e.approvalMismatch).toBe(true);
|
||||
@@ -331,17 +498,17 @@ describe("verifySignedTx field comparison", () => {
|
||||
|
||||
// The transaction type decides which fields exist, so an artifact of a type
|
||||
// this wallet does not sign carries consequences the approval cannot describe
|
||||
// and none of the field comparisons can see. The approval used here is the
|
||||
// ordinary dApp shape with no fee fields — the common case, since
|
||||
// populateTransaction() fills them — which is exactly the case the
|
||||
// fee-mechanism check cannot catch by accident.
|
||||
// and none of the field comparisons can see. The refusal has to come from the
|
||||
// type allowlist rather than from a field comparison, so these run against an
|
||||
// approval whose every other field matches the artifact exactly.
|
||||
describe("verifySignedTx transaction type", () => {
|
||||
const BARE_APPROVAL = {
|
||||
const BARE_REQUEST = {
|
||||
from: signer.address,
|
||||
to: RECIPIENT,
|
||||
value: "0x2386f26fc10000",
|
||||
data: "0x",
|
||||
};
|
||||
const BARE_APPROVAL = approvedFor(BARE_REQUEST);
|
||||
|
||||
// An EIP-7702 artifact that pays the approved amount to the approved
|
||||
// recipient and, in the same transaction, installs the attacker's code at
|
||||
@@ -353,7 +520,7 @@ describe("verifySignedTx transaction type", () => {
|
||||
chainId: 1,
|
||||
nonce: 8,
|
||||
});
|
||||
const raw = await signedFor(BARE_APPROVAL, signer, {
|
||||
const raw = await signedFor(BARE_REQUEST, signer, {
|
||||
type: 4,
|
||||
authorizationList: [authorization],
|
||||
});
|
||||
@@ -366,7 +533,7 @@ describe("verifySignedTx transaction type", () => {
|
||||
});
|
||||
|
||||
test("refuses a type 3 blob artifact", async () => {
|
||||
const raw = await signedFor(BARE_APPROVAL, signer, {
|
||||
const raw = await signedFor(BARE_REQUEST, signer, {
|
||||
type: 3,
|
||||
maxFeePerBlobGas: 1000000000n,
|
||||
blobVersionedHashes: ["0x01" + "ab".repeat(31)],
|
||||
@@ -390,7 +557,7 @@ describe("verifySignedTx transaction type", () => {
|
||||
chainId: 1,
|
||||
nonce: 8,
|
||||
});
|
||||
const raw = await signedFor(BARE_APPROVAL, signer, {
|
||||
const raw = await signedFor(BARE_REQUEST, signer, {
|
||||
type: 4,
|
||||
authorizationList: [authorization],
|
||||
});
|
||||
@@ -404,40 +571,45 @@ describe("verifySignedTx transaction type", () => {
|
||||
});
|
||||
|
||||
test("accepts a legacy type 0 transaction", async () => {
|
||||
const approved = { ...BARE_APPROVAL, gasPrice: "0x77359400" };
|
||||
const raw = await signedFor(approved, signer, {
|
||||
const legacy = {
|
||||
type: 0,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
});
|
||||
};
|
||||
const raw = await signedFor(BARE_REQUEST, signer, legacy);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
verifySignedTx(
|
||||
raw,
|
||||
approvedFor(BARE_REQUEST, legacy),
|
||||
signer.address,
|
||||
SELECTED,
|
||||
),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test("accepts a type 1 transaction whose access list is the approved one", async () => {
|
||||
const accessList = [{ address: OTHER_RECIPIENT, storageKeys: [] }];
|
||||
const approved = {
|
||||
...BARE_APPROVAL,
|
||||
gasPrice: "0x77359400",
|
||||
accessList,
|
||||
};
|
||||
const raw = await signedFor(approved, signer, {
|
||||
const overrides = {
|
||||
type: 1,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
accessList,
|
||||
});
|
||||
accessList: [{ address: OTHER_RECIPIENT, storageKeys: [] }],
|
||||
};
|
||||
const raw = await signedFor(BARE_REQUEST, signer, overrides);
|
||||
expect(Transaction.from(raw).type).toBe(1);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
verifySignedTx(
|
||||
raw,
|
||||
approvedFor(BARE_REQUEST, overrides),
|
||||
signer.address,
|
||||
SELECTED,
|
||||
),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test("refuses an access list the approval never carried", async () => {
|
||||
const raw = await signedFor(BARE_APPROVAL, signer, {
|
||||
const raw = await signedFor(BARE_REQUEST, signer, {
|
||||
accessList: [{ address: OTHER_RECIPIENT, storageKeys: [] }],
|
||||
});
|
||||
expect(() =>
|
||||
@@ -446,8 +618,11 @@ describe("verifySignedTx transaction type", () => {
|
||||
});
|
||||
|
||||
test("treats an absent access list and an empty one as the same thing", async () => {
|
||||
const approved = { ...BARE_APPROVAL, accessList: [] };
|
||||
const raw = await signedFor(BARE_APPROVAL, signer, {});
|
||||
const approved = { ...BARE_APPROVAL };
|
||||
delete approved.accessList;
|
||||
expect(approved.accessList).toBeUndefined();
|
||||
const raw = await signedFor(BARE_REQUEST, signer, {});
|
||||
expect(Transaction.from(raw).accessList).toEqual([]);
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
).not.toThrow();
|
||||
@@ -498,6 +673,25 @@ describe("verifySignedTx exhaustiveness", () => {
|
||||
expect(exposed.filter((name) => !accounted.has(name))).toEqual([]);
|
||||
});
|
||||
|
||||
// The comparison loop runs over the fields a type serializes and refuses a
|
||||
// field it has no comparator for. That refusal is unreachable only while
|
||||
// the table covers the whole of SERIALIZED_FIELDS, so the coverage is
|
||||
// pinned here rather than assumed: adding a field to a type without a
|
||||
// comparator would otherwise turn every transaction of that type into a
|
||||
// refusal, and adding a comparator without the field would be a check that
|
||||
// never runs.
|
||||
test("every field a type serializes has a comparator", () => {
|
||||
const serialized = new Set(
|
||||
Object.values(SERIALIZED_FIELDS).flat().sort(),
|
||||
);
|
||||
expect([...serialized].filter((key) => !APPROVED_FIELDS[key])).toEqual(
|
||||
[],
|
||||
);
|
||||
expect(
|
||||
Object.keys(APPROVED_FIELDS).filter((key) => !serialized.has(key)),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
// The two layers behind the type allowlist. Nothing reachable through
|
||||
// verifySignedTx can trip either of them while the allowlist holds — that
|
||||
// is what they are for — so they are exercised directly rather than taken
|
||||
@@ -553,49 +747,30 @@ describe("verifySignedTx exhaustiveness", () => {
|
||||
test("an accepted artifact of each allowed type rebuilds byte for byte", async () => {
|
||||
const shapes = [
|
||||
{
|
||||
approved: { ...TX_PARAMS, gasPrice: "0x77359400" },
|
||||
overrides: {
|
||||
type: 0,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
},
|
||||
type: 0,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
},
|
||||
{
|
||||
approved: {
|
||||
...TX_PARAMS,
|
||||
gasPrice: "0x77359400",
|
||||
accessList: [
|
||||
{
|
||||
address: RECIPIENT,
|
||||
storageKeys: ["0x" + "11".repeat(32)],
|
||||
},
|
||||
],
|
||||
},
|
||||
overrides: {
|
||||
type: 1,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
accessList: [
|
||||
{
|
||||
address: RECIPIENT,
|
||||
storageKeys: ["0x" + "11".repeat(32)],
|
||||
},
|
||||
],
|
||||
},
|
||||
type: 1,
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: null,
|
||||
maxPriorityFeePerGas: null,
|
||||
accessList: [
|
||||
{
|
||||
address: RECIPIENT,
|
||||
storageKeys: ["0x" + "11".repeat(32)],
|
||||
},
|
||||
],
|
||||
},
|
||||
{ approved: TX_PARAMS, overrides: {} },
|
||||
{},
|
||||
];
|
||||
for (const shape of shapes) {
|
||||
const raw = await signedFor(
|
||||
shape.approved,
|
||||
signer,
|
||||
shape.overrides,
|
||||
);
|
||||
for (const overrides of shapes) {
|
||||
const raw = await signedFor(TX_PARAMS, signer, overrides);
|
||||
const parsed = verifySignedTx(
|
||||
raw,
|
||||
shape.approved,
|
||||
approvedFor(TX_PARAMS, overrides),
|
||||
signer.address,
|
||||
SELECTED,
|
||||
);
|
||||
@@ -644,7 +819,7 @@ describe("verifySignedTx canonical encoding", () => {
|
||||
test("refuses an artifact that is not its own canonical encoding", async () => {
|
||||
const mutated = await nonCanonical();
|
||||
expect(() =>
|
||||
verifySignedTx(mutated, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(mutated, APPROVED, signer.address, SELECTED),
|
||||
).toThrow(/not encoded canonically/);
|
||||
});
|
||||
|
||||
@@ -659,7 +834,7 @@ describe("verifySignedTx canonical encoding", () => {
|
||||
const raw = await signedWith({});
|
||||
const upper = "0x" + raw.slice(2).toUpperCase();
|
||||
expect(() =>
|
||||
verifySignedTx(upper, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(upper, APPROVED, signer.address, SELECTED),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
@@ -670,46 +845,33 @@ 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),
|
||||
verifySignedTx(raw, APPROVED, signer.address, 1),
|
||||
).not.toThrow();
|
||||
expect(() =>
|
||||
verifySignedTx(raw, TX_PARAMS, signer.address, "1"),
|
||||
verifySignedTx(raw, APPROVED, signer.address, "1"),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test("accepts an approved chain id written in hex", async () => {
|
||||
// The approved transaction crosses to the popup as JSON, so it comes back
|
||||
// spelled in hex quantities rather than in the bigints it was populated
|
||||
// with. None of that is tampering.
|
||||
test("accepts an approval spelled as the wire spells it", async () => {
|
||||
const raw = await signedWith({});
|
||||
const approved = { ...TX_PARAMS, chainId: "0x1" };
|
||||
const wire = {
|
||||
...APPROVED,
|
||||
chainId: "0x1",
|
||||
nonce: "0x7",
|
||||
gasLimit: "0x186a0",
|
||||
maxFeePerGas: "0x77359400",
|
||||
maxPriorityFeePerGas: "0x3b9aca00",
|
||||
value: "0x2386f26fc10000",
|
||||
};
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
verifySignedTx(raw, wire, 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 () => {
|
||||
test("accepts quantities spelled as hex, decimal, number and bigint", async () => {
|
||||
const raw = await signedWith({});
|
||||
for (const maxFee of [
|
||||
"0x77359400",
|
||||
@@ -720,7 +882,7 @@ describe("verifySignedTx normalization", () => {
|
||||
expect(() =>
|
||||
verifySignedTx(
|
||||
raw,
|
||||
{ ...TX_PARAMS, maxFeePerGas: maxFee },
|
||||
{ ...APPROVED, maxFeePerGas: maxFee },
|
||||
signer.address,
|
||||
SELECTED,
|
||||
),
|
||||
@@ -728,24 +890,19 @@ describe("verifySignedTx normalization", () => {
|
||||
}
|
||||
});
|
||||
|
||||
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() };
|
||||
const approved = { ...APPROVED, 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" });
|
||||
const params = { to: RECIPIENT, value: "0x0" };
|
||||
const approved = approvedFor(params);
|
||||
delete approved.data;
|
||||
const raw = await signedFor({ ...params, data: "0x" });
|
||||
expect(() =>
|
||||
verifySignedTx(raw, approved, signer.address, SELECTED),
|
||||
).not.toThrow();
|
||||
@@ -756,7 +913,7 @@ describe("verifySignedTx normalization", () => {
|
||||
expect(() =>
|
||||
verifySignedTx(
|
||||
raw,
|
||||
{ ...TX_PARAMS, maxFeePerGas: "cheap" },
|
||||
{ ...APPROVED, maxFeePerGas: "cheap" },
|
||||
signer.address,
|
||||
SELECTED,
|
||||
),
|
||||
@@ -774,7 +931,7 @@ describe("verifySignedTx normalization", () => {
|
||||
try {
|
||||
verifySignedTx(
|
||||
raw,
|
||||
{ ...TX_PARAMS, value },
|
||||
{ ...APPROVED, value },
|
||||
signer.address,
|
||||
SELECTED,
|
||||
);
|
||||
@@ -793,7 +950,7 @@ describe("verifySignedTx normalization", () => {
|
||||
expect(() =>
|
||||
verifySignedTx(
|
||||
raw,
|
||||
{ ...TX_PARAMS, accessList: ["nope"] },
|
||||
{ ...APPROVED, accessList: ["nope"] },
|
||||
signer.address,
|
||||
SELECTED,
|
||||
),
|
||||
@@ -934,7 +1091,7 @@ describe("signing failure and retry", () => {
|
||||
test("a mismatch spends the approval", async () => {
|
||||
const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
|
||||
try {
|
||||
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED);
|
||||
throw new Error("expected a rejection");
|
||||
} catch (e) {
|
||||
expect(failureIsRetryable(e)).toBe(false);
|
||||
@@ -1007,7 +1164,7 @@ describe("signing failure and retry", () => {
|
||||
const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
|
||||
let outcome;
|
||||
try {
|
||||
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
|
||||
verifySignedTx(raw, APPROVED, signer.address, SELECTED);
|
||||
} catch (e) {
|
||||
outcome = describeTxFailure(TX_STAGE_VERIFY, e);
|
||||
}
|
||||
@@ -1061,12 +1218,13 @@ describe("signing failure and retry", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// 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
|
||||
// puts on the wire is passed along, so this also pins down that the wire
|
||||
// payload is sufficient on its own.
|
||||
describe("popup signing sequence to background verification", () => {
|
||||
// End-to-end over the messaging boundary, without a browser: the background
|
||||
// populates the transaction, the object that produces crosses to the popup as
|
||||
// JSON and is signed there, and the artifact goes back to the exact check the
|
||||
// background runs before it broadcasts. Only what each side puts on the wire is
|
||||
// passed along, so this also pins down that the wire payloads are sufficient on
|
||||
// their own.
|
||||
describe("background population to popup signing to verification", () => {
|
||||
// Stand-in for the JSON-RPC provider. populateTransaction only needs the
|
||||
// nonce, the gas estimate, the network and the fee data.
|
||||
const fakeProvider = {
|
||||
@@ -1084,33 +1242,52 @@ describe("popup signing sequence to background verification", () => {
|
||||
// through getSignerForAddress() the way the popup does.
|
||||
const walletData = { type: "privkey" };
|
||||
|
||||
async function popupSignsTx(txParams) {
|
||||
const localSigner = getSignerForAddress(walletData, 0, SIGNER_KEY);
|
||||
const connected = localSigner.connect(fakeProvider);
|
||||
const populated = await connected.populateTransaction(txParams);
|
||||
delete populated.from;
|
||||
return connected.signTransaction(populated);
|
||||
// What the background does before the approval window opens.
|
||||
async function backgroundPrepares(txParams) {
|
||||
const approvedTx = await prepareApprovalTx(
|
||||
fakeProvider,
|
||||
signer.address,
|
||||
txParams,
|
||||
);
|
||||
// Extension messaging is JSON; the popup sees the other side of it.
|
||||
return JSON.parse(JSON.stringify(approvedTx));
|
||||
}
|
||||
|
||||
test("a populated, signed transaction is accepted and broadcastable", async () => {
|
||||
const rawSignedTx = await popupSignsTx(TX_PARAMS);
|
||||
// What the popup does with it: signs it as given, populating nothing.
|
||||
async function popupSigns(approvedTx) {
|
||||
const localSigner = getSignerForAddress(walletData, 0, SIGNER_KEY);
|
||||
return localSigner.signTransaction({ ...approvedTx });
|
||||
}
|
||||
|
||||
test("the populated transaction is what gets signed and what gets checked", async () => {
|
||||
const approvedTx = await backgroundPrepares(TX_PARAMS);
|
||||
const rawSignedTx = await popupSigns(approvedTx);
|
||||
const parsed = verifySignedTx(
|
||||
rawSignedTx,
|
||||
TX_PARAMS,
|
||||
approvedTx,
|
||||
signer.address,
|
||||
SELECTED,
|
||||
);
|
||||
expect(parsed.nonce).toBe(7);
|
||||
expect(parsed.chainId).toBe(1n);
|
||||
expect(parsed.gasLimit).toBe(21000n);
|
||||
expect(parsed.maxFeePerGas).toBe(2000000000n);
|
||||
expect(parsed.to).toBe(RECIPIENT);
|
||||
expect(parsed.value).toBe(BigInt(TX_PARAMS.value));
|
||||
expect(parsed.data).toBe(TX_PARAMS.data);
|
||||
expect(parsed.signature).not.toBeNull();
|
||||
// Every field the screen shows, and the artifact, are the same numbers.
|
||||
expect(BigInt(approvedTx.nonce)).toBe(BigInt(parsed.nonce));
|
||||
expect(BigInt(approvedTx.gasLimit)).toBe(parsed.gasLimit);
|
||||
expect(BigInt(approvedTx.maxFeePerGas)).toBe(parsed.maxFeePerGas);
|
||||
expect(BigInt(approvedTx.maxPriorityFeePerGas)).toBe(
|
||||
parsed.maxPriorityFeePerGas,
|
||||
);
|
||||
});
|
||||
|
||||
test("the wire payload carries no password and no secret", async () => {
|
||||
const rawSignedTx = await popupSignsTx(TX_PARAMS);
|
||||
const approvedTx = await backgroundPrepares(TX_PARAMS);
|
||||
const rawSignedTx = await popupSigns(approvedTx);
|
||||
const payload = {
|
||||
type: "AUTISTMASK_TX_RESPONSE",
|
||||
id: "test-approval-id",
|
||||
@@ -1128,20 +1305,54 @@ describe("popup signing sequence to background verification", () => {
|
||||
expect(wire).not.toContain(SIGNER_KEY.slice(2).toLowerCase());
|
||||
});
|
||||
|
||||
// The popup is the component whose compromise this check exists to detect,
|
||||
// so it is given the approved transaction and signs something else.
|
||||
test("a popup that signs a different fee than it was given is refused", async () => {
|
||||
const approvedTx = await backgroundPrepares(TX_PARAMS);
|
||||
const rawSignedTx = await popupSigns({
|
||||
...approvedTx,
|
||||
maxFeePerGas: "0x3b9aca000",
|
||||
});
|
||||
expect(() =>
|
||||
verifySignedTx(rawSignedTx, approvedTx, signer.address, SELECTED),
|
||||
).toThrow(/approved maximum fee per gas/);
|
||||
});
|
||||
|
||||
test("a popup that signs a different nonce than it was given is refused", async () => {
|
||||
const approvedTx = await backgroundPrepares(TX_PARAMS);
|
||||
const rawSignedTx = await popupSigns({ ...approvedTx, nonce: "0x8" });
|
||||
expect(() =>
|
||||
verifySignedTx(rawSignedTx, approvedTx, signer.address, SELECTED),
|
||||
).toThrow(/approved nonce/);
|
||||
});
|
||||
|
||||
test("the background rejects a transaction the popup did not approve", async () => {
|
||||
const rawSignedTx = await popupSignsTx({
|
||||
...TX_PARAMS,
|
||||
const approvedTx = await backgroundPrepares(TX_PARAMS);
|
||||
const rawSignedTx = await popupSigns({
|
||||
...approvedTx,
|
||||
to: OTHER_RECIPIENT,
|
||||
});
|
||||
expect(() =>
|
||||
verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SELECTED),
|
||||
verifySignedTx(rawSignedTx, approvedTx, signer.address, SELECTED),
|
||||
).toThrow(/approved recipient/);
|
||||
});
|
||||
|
||||
test("the background rejects a transaction populated on another network", async () => {
|
||||
const rawSignedTx = await popupSignsTx(TX_PARAMS);
|
||||
const approvedTx = await backgroundPrepares(TX_PARAMS);
|
||||
const rawSignedTx = await popupSigns(approvedTx);
|
||||
expect(() =>
|
||||
verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SEPOLIA),
|
||||
verifySignedTx(rawSignedTx, approvedTx, signer.address, SEPOLIA),
|
||||
).toThrow(/different network than the one that is selected/);
|
||||
});
|
||||
|
||||
// ethers refuses to sign for an address that is not the key's own, so a
|
||||
// popup working from the approved object cannot quietly sign as whichever
|
||||
// address the user has switched to.
|
||||
test("the approved from stops the popup signing with another key", async () => {
|
||||
const approvedTx = await backgroundPrepares(TX_PARAMS);
|
||||
const otherSigner = getSignerForAddress(walletData, 0, OTHER_KEY);
|
||||
await expect(
|
||||
otherSigner.signTransaction({ ...approvedTx }),
|
||||
).rejects.toThrow(/from address mismatch/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,16 +8,24 @@
|
||||
// already saw — which means the entry being present is not by itself proof
|
||||
// that no attempt is running. A second response carrying the same id (a
|
||||
// reloaded approval window re-rendering a live Approve button, a popup that
|
||||
// emits the message twice) must not start a second verify and broadcast: with
|
||||
// the ordinary dApp approval shape the page fixes no nonce, so two artifacts
|
||||
// signed at different nonces both verify, and the approved transfer would go
|
||||
// out twice.
|
||||
// emits the message twice) must not start a second verify and broadcast: the
|
||||
// same approved transaction signed twice verifies twice, and the transfer
|
||||
// would go out twice.
|
||||
//
|
||||
// It also covers what the approval is verified against. The approval now
|
||||
// carries the transaction the background populated and the screen displayed,
|
||||
// and the address that was active when it was raised — so a fee, a nonce or an
|
||||
// address that moved between approval and signing is refused rather than
|
||||
// signed.
|
||||
|
||||
const { Wallet } = require("ethers");
|
||||
const { Network, Wallet } = require("ethers");
|
||||
|
||||
const SIGNER_KEY =
|
||||
"0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d";
|
||||
const OTHER_KEY =
|
||||
"0x5de4111afa1a4b94908f83103eb1f1706367c2e68ca870fc3fb9a804cdab365a";
|
||||
const signer = new Wallet(SIGNER_KEY);
|
||||
const other = new Wallet(OTHER_KEY);
|
||||
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
|
||||
|
||||
const ORIGIN = "https://dapp.example";
|
||||
@@ -33,9 +41,16 @@ const TX_PARAMS = {
|
||||
data: "0x",
|
||||
};
|
||||
|
||||
// The fields the popup's populateTransaction() would fill in. The nonce is a
|
||||
// parameter because the duplicate case turns on the two artifacts differing
|
||||
// in exactly the field nothing constrains.
|
||||
// The nonce the stubbed node reports, and so the nonce the background
|
||||
// populates the approval with.
|
||||
const NONCE = 7;
|
||||
|
||||
// "Hello AutistMask" as the hex string a dApp passes to personal_sign.
|
||||
const MESSAGE = "0x48656c6c6f204175746973744d61736b";
|
||||
|
||||
// The transaction the background populates and the approval screen displays.
|
||||
// The nonce is a parameter because the duplicate case turns on two artifacts
|
||||
// differing in a field the dApp fixed nothing for.
|
||||
function populated(nonce) {
|
||||
return {
|
||||
type: 2,
|
||||
@@ -50,8 +65,26 @@ function populated(nonce) {
|
||||
};
|
||||
}
|
||||
|
||||
function signedAtNonce(nonce) {
|
||||
return signer.signTransaction(populated(nonce));
|
||||
function signedAtNonce(nonce, withWallet) {
|
||||
return (withWallet || signer).signTransaction(populated(nonce));
|
||||
}
|
||||
|
||||
// The node the background populates against. Its answers are the numbers the
|
||||
// approval screen shows, so they are also the numbers every artifact below is
|
||||
// signed at.
|
||||
function fakeProvider(broadcastTransaction, overrides) {
|
||||
return {
|
||||
broadcastTransaction,
|
||||
getNetwork: async () => Network.from(1),
|
||||
getTransactionCount: async () => NONCE,
|
||||
estimateGas: async () => 100000n,
|
||||
getFeeData: async () => ({
|
||||
gasPrice: 2000000000n,
|
||||
maxFeePerGas: 2000000000n,
|
||||
maxPriorityFeePerGas: 1000000000n,
|
||||
}),
|
||||
...(overrides || {}),
|
||||
};
|
||||
}
|
||||
|
||||
// A promise whose settlement the test controls, so a broadcast can be held in
|
||||
@@ -85,7 +118,7 @@ function loadBackground(options) {
|
||||
currentNetwork: () => ({ chainId: "0x1" }),
|
||||
}));
|
||||
jest.doMock("../src/shared/balances", () => ({
|
||||
getProvider: () => ({ broadcastTransaction }),
|
||||
getProvider: () => fakeProvider(broadcastTransaction, opts.provider),
|
||||
refreshBalances: jest.fn(async () => {}),
|
||||
}));
|
||||
jest.doMock("../src/shared/phishingDomains", () => ({
|
||||
@@ -171,7 +204,7 @@ function loadBackground(options) {
|
||||
|
||||
// Raise a pending transaction approval the way a dApp does, and dig the
|
||||
// approval id back out of the popup URL the background opened.
|
||||
function requestTx() {
|
||||
function requestTx(txParams) {
|
||||
let rpcResult = null;
|
||||
const sendResponse = jest.fn((r) => {
|
||||
rpcResult = r;
|
||||
@@ -180,7 +213,7 @@ function loadBackground(options) {
|
||||
{
|
||||
type: "AUTISTMASK_RPC",
|
||||
method: "eth_sendTransaction",
|
||||
params: [TX_PARAMS],
|
||||
params: [txParams || TX_PARAMS],
|
||||
},
|
||||
{ origin: ORIGIN },
|
||||
sendResponse,
|
||||
@@ -191,6 +224,30 @@ function loadBackground(options) {
|
||||
};
|
||||
}
|
||||
|
||||
// The same for a message-signing approval, which pins the signing address
|
||||
// at approval time in exactly the same way.
|
||||
function requestSign(from) {
|
||||
let rpcResult = null;
|
||||
messageListener(
|
||||
{
|
||||
type: "AUTISTMASK_RPC",
|
||||
method: "personal_sign",
|
||||
params: [MESSAGE, from || signer.address],
|
||||
},
|
||||
{ origin: ORIGIN },
|
||||
(r) => {
|
||||
rpcResult = r;
|
||||
},
|
||||
);
|
||||
return {
|
||||
id: () =>
|
||||
new URL(created[created.length - 1].url).searchParams.get(
|
||||
"approval",
|
||||
),
|
||||
result: () => rpcResult,
|
||||
};
|
||||
}
|
||||
|
||||
// The user closes the approval popup. `created` is index-aligned with the
|
||||
// ids the window stub hands back, so window 1 is the first popup opened.
|
||||
function closeWindow(windowId) {
|
||||
@@ -200,18 +257,27 @@ function loadBackground(options) {
|
||||
return {
|
||||
send,
|
||||
requestTx,
|
||||
requestSign,
|
||||
closeWindow,
|
||||
broadcastTransaction,
|
||||
loadState,
|
||||
created,
|
||||
removed,
|
||||
// The user switching account in the toolbar popup, as the background
|
||||
// sees it: the persisted active address changes underneath a pending
|
||||
// approval.
|
||||
setActiveAddress: (address) => {
|
||||
persisted.activeAddress = address;
|
||||
},
|
||||
fromPopup: { url: EXT_URL + "src/popup/index.html" },
|
||||
};
|
||||
}
|
||||
|
||||
// Let the handler's promise chain run to the next suspension point.
|
||||
// Let the handler's promise chain run to the next suspension point. Raising a
|
||||
// transaction approval now populates it against the node first, which is
|
||||
// several awaits deep before the window is opened.
|
||||
async function settle() {
|
||||
for (let i = 0; i < 10; i++) await Promise.resolve();
|
||||
for (let i = 0; i < 50; i++) await Promise.resolve();
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
@@ -244,9 +310,11 @@ describe("one approval, one broadcast", () => {
|
||||
await settle();
|
||||
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
|
||||
|
||||
// A reloaded approval window signs the same approval again. Nothing
|
||||
// in the approval fixes a nonce, so this artifact verifies just as
|
||||
// well as the first one.
|
||||
// A reloaded approval window signs the same approval again, at another
|
||||
// nonce. The claim is taken before anything is verified, so what this
|
||||
// asserts is the interlock and not the nonce comparison: the refusal
|
||||
// below is the claim's own message, which a verification failure does
|
||||
// not produce.
|
||||
const second = bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_TX_RESPONSE",
|
||||
@@ -376,6 +444,319 @@ describe("one approval, one broadcast", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// The approval carries the transaction the user was shown and the address it
|
||||
// was raised for, and the artifact is checked against both. Every case here is
|
||||
// one the old comparison — against the dApp's request, for the address that is
|
||||
// active now — would have broadcast.
|
||||
describe("what the approval is verified against", () => {
|
||||
// The approval screen showed the populated fee. An artifact at ten times
|
||||
// that fee, still far below the ceilings, is what the ceilings alone could
|
||||
// not catch.
|
||||
test("a fee differing from the displayed one is refused, not sent", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestTx();
|
||||
await settle();
|
||||
const id = pending.id();
|
||||
|
||||
const raw = await signer.signTransaction({
|
||||
...populated(NONCE),
|
||||
maxFeePerGas: 20000000000n,
|
||||
});
|
||||
const answer = bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_TX_RESPONSE",
|
||||
id,
|
||||
approved: true,
|
||||
rawSignedTx: raw,
|
||||
},
|
||||
{ url: bg.fromPopup.url },
|
||||
);
|
||||
await settle();
|
||||
|
||||
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
|
||||
expect(answer.sendResponse).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
error: expect.stringMatching(/approved maximum fee per gas/),
|
||||
retryable: false,
|
||||
stage: "verify",
|
||||
}),
|
||||
);
|
||||
expect(pending.result()).toEqual({
|
||||
error: {
|
||||
message: expect.stringMatching(/approved maximum fee per gas/),
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
test("a nonce differing from the displayed one is refused, not sent", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestTx();
|
||||
await settle();
|
||||
const id = pending.id();
|
||||
|
||||
const answer = bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_TX_RESPONSE",
|
||||
id,
|
||||
approved: true,
|
||||
rawSignedTx: await signedAtNonce(NONCE + 1),
|
||||
},
|
||||
{ url: bg.fromPopup.url },
|
||||
);
|
||||
await settle();
|
||||
|
||||
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
|
||||
expect(answer.sendResponse).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
error: expect.stringMatching(/approved nonce/),
|
||||
retryable: false,
|
||||
stage: "verify",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
// The address switch. The approval named one account; the wallet is on
|
||||
// another by the time the artifact arrives. Both halves are covered: the
|
||||
// popup signing as the account that is active now, and the popup correctly
|
||||
// signing as the approved account while the wallet has moved on.
|
||||
test("an artifact signed by the address that is active now is refused", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestTx();
|
||||
await settle();
|
||||
const id = pending.id();
|
||||
|
||||
bg.setActiveAddress(other.address);
|
||||
const answer = bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_TX_RESPONSE",
|
||||
id,
|
||||
approved: true,
|
||||
rawSignedTx: await signedAtNonce(NONCE, other),
|
||||
},
|
||||
{ url: bg.fromPopup.url },
|
||||
);
|
||||
await settle();
|
||||
|
||||
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
|
||||
expect(answer.sendResponse).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ retryable: false, stage: "verify" }),
|
||||
);
|
||||
expect(pending.result()).toEqual({
|
||||
error: { message: expect.stringMatching(/active address changed/) },
|
||||
});
|
||||
});
|
||||
|
||||
test("an address switch refuses even the correctly signed artifact", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestTx();
|
||||
await settle();
|
||||
const id = pending.id();
|
||||
|
||||
bg.setActiveAddress(other.address);
|
||||
const answer = bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_TX_RESPONSE",
|
||||
id,
|
||||
approved: true,
|
||||
rawSignedTx: await signedAtNonce(NONCE),
|
||||
},
|
||||
{ url: bg.fromPopup.url },
|
||||
);
|
||||
await settle();
|
||||
|
||||
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
|
||||
expect(answer.sendResponse).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
error: expect.stringMatching(/active address changed/),
|
||||
retryable: false,
|
||||
stage: "verify",
|
||||
}),
|
||||
);
|
||||
// A refusal, so the approval is spent: the same artifact offered again
|
||||
// finds nothing to answer.
|
||||
const retry = bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_TX_RESPONSE",
|
||||
id,
|
||||
approved: true,
|
||||
rawSignedTx: await signedAtNonce(NONCE),
|
||||
},
|
||||
{ url: bg.fromPopup.url },
|
||||
);
|
||||
await settle();
|
||||
expect(retry.sendResponse).not.toHaveBeenCalled();
|
||||
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("a switch back to the approved address still sends", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestTx();
|
||||
await settle();
|
||||
const id = pending.id();
|
||||
|
||||
bg.setActiveAddress(other.address);
|
||||
bg.setActiveAddress(signer.address);
|
||||
bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
|
||||
bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_TX_RESPONSE",
|
||||
id,
|
||||
approved: true,
|
||||
rawSignedTx: await signedAtNonce(NONCE),
|
||||
},
|
||||
{ url: bg.fromPopup.url },
|
||||
);
|
||||
await settle();
|
||||
|
||||
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
|
||||
expect(pending.result()).toEqual({ result: "0xfeed" });
|
||||
});
|
||||
|
||||
// The popup is handed the populated transaction and the address it is for,
|
||||
// and nothing else it would have to fetch or decide.
|
||||
test("the popup is given the transaction it is to sign", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestTx();
|
||||
await settle();
|
||||
|
||||
const details = bg.send(
|
||||
{ type: "AUTISTMASK_GET_APPROVAL", id: pending.id() },
|
||||
{ url: bg.fromPopup.url },
|
||||
);
|
||||
const shown = details.sendResponse.mock.calls[0][0];
|
||||
expect(shown.type).toBe("tx");
|
||||
expect(shown.approvedFrom).toBe(signer.address);
|
||||
expect(shown.approvedTx).toEqual({
|
||||
type: 2,
|
||||
from: signer.address,
|
||||
chainId: "0x1",
|
||||
nonce: "0x7",
|
||||
gasLimit: "0x186a0",
|
||||
maxFeePerGas: "0x77359400",
|
||||
maxPriorityFeePerGas: "0x3b9aca00",
|
||||
to: RECIPIENT,
|
||||
value: TX_PARAMS.value,
|
||||
data: "0x",
|
||||
accessList: [],
|
||||
});
|
||||
});
|
||||
|
||||
// A request naming an account the wallet is not on is refused outright
|
||||
// rather than signed as whichever account is active.
|
||||
test("a request from another address raises no approval at all", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestTx({ ...TX_PARAMS, from: other.address });
|
||||
await settle();
|
||||
|
||||
expect(pending.result()).toEqual({
|
||||
error: {
|
||||
code: 4100,
|
||||
message: expect.stringMatching(/not the active one/),
|
||||
},
|
||||
});
|
||||
expect(bg.created).toEqual([]);
|
||||
});
|
||||
|
||||
// Message signing pins the address the same way, and refuses the same way.
|
||||
// A signature is not a transaction, but a permit signed by an account the
|
||||
// approval did not name spends that account's tokens all the same.
|
||||
test("a sign approval refuses a signature after an address switch", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestSign();
|
||||
await settle();
|
||||
|
||||
bg.setActiveAddress(other.address);
|
||||
const answer = bg.send(
|
||||
{
|
||||
type: "AUTISTMASK_SIGN_RESPONSE",
|
||||
id: pending.id(),
|
||||
approved: true,
|
||||
signature: await signer.signMessage(
|
||||
Buffer.from(MESSAGE.slice(2), "hex"),
|
||||
),
|
||||
},
|
||||
{ url: bg.fromPopup.url },
|
||||
);
|
||||
await settle();
|
||||
|
||||
expect(answer.sendResponse).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
error: expect.stringMatching(/active address changed/),
|
||||
retryable: false,
|
||||
}),
|
||||
);
|
||||
expect(pending.result()).toEqual({
|
||||
error: { message: expect.stringMatching(/active address changed/) },
|
||||
});
|
||||
});
|
||||
|
||||
test("a sign request from another address raises no approval at all", async () => {
|
||||
const bg = loadBackground();
|
||||
const pending = bg.requestSign(other.address);
|
||||
await settle();
|
||||
|
||||
expect(pending.result()).toEqual({
|
||||
error: {
|
||||
code: 4100,
|
||||
message: expect.stringMatching(/not the active one/),
|
||||
},
|
||||
});
|
||||
expect(bg.created).toEqual([]);
|
||||
});
|
||||
|
||||
// Population is a network round trip with the user's hands free. An
|
||||
// approval raised for the address that was active when it started could
|
||||
// never be signed once the wallet has moved off it, so it is never raised.
|
||||
test("an address switch during population raises no approval", async () => {
|
||||
let bg;
|
||||
bg = loadBackground({
|
||||
provider: {
|
||||
// The user switches account in the toolbar popup while the
|
||||
// node is being asked for a gas estimate.
|
||||
estimateGas: async () => {
|
||||
bg.setActiveAddress(other.address);
|
||||
return 100000n;
|
||||
},
|
||||
},
|
||||
});
|
||||
const pending = bg.requestTx();
|
||||
await settle();
|
||||
|
||||
expect(pending.result()).toEqual({
|
||||
error: {
|
||||
message: expect.stringMatching(
|
||||
/active address changed while this transaction was being prepared/,
|
||||
),
|
||||
},
|
||||
});
|
||||
expect(bg.created).toEqual([]);
|
||||
});
|
||||
|
||||
// Population happens before the window exists, so its failure is a failure
|
||||
// of the request: no approval, no window, and the error goes back to the
|
||||
// page the click came from.
|
||||
test("a transaction that cannot be prepared opens no window", async () => {
|
||||
const bg = loadBackground({
|
||||
provider: {
|
||||
estimateGas: async () => {
|
||||
throw new Error("execution reverted");
|
||||
},
|
||||
},
|
||||
});
|
||||
const pending = bg.requestTx();
|
||||
await settle();
|
||||
|
||||
expect(pending.result()).toEqual({
|
||||
error: {
|
||||
message: expect.stringMatching(
|
||||
/could not be prepared.*execution reverted/,
|
||||
),
|
||||
},
|
||||
});
|
||||
expect(bg.created).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
// The interlock must not cost the retry the approval exists to allow.
|
||||
describe("the interlock releases a failed attempt", () => {
|
||||
test("a retryable failure before the broadcast leaves the approval usable", async () => {
|
||||
|
||||
@@ -297,17 +297,26 @@ async function handleRpc(route, postData, opts, report) {
|
||||
// ethers batches by default, so the body may be an array.
|
||||
const batch = Array.isArray(payload) ? payload : [payload];
|
||||
|
||||
// Anything that is not a JSON-RPC object, or a batch of them, is not
|
||||
// RPC at all and must be reported like any other unrecognised
|
||||
// outbound traffic rather than dereferenced. request.postData()
|
||||
// returns null both for a bodyless POST and for a body Playwright
|
||||
// cannot decode as UTF-8 (sendBeacon with a Blob, or any binary
|
||||
// payload), so this is not an empty-string special case: it rejects
|
||||
// every non-object payload, exactly as the catch above rejects every
|
||||
// unparseable one.
|
||||
// Anything that is not a JSON-RPC object, or a NON-EMPTY batch of
|
||||
// them, is not RPC at all and must be reported like any other
|
||||
// unrecognised outbound traffic rather than dereferenced.
|
||||
//
|
||||
// The length check is not decoration: every() is vacuously true on an
|
||||
// empty array, so without it a POST with body [] was answered 200 []
|
||||
// and escaped the guard entirely (issue #187). No real batch is empty,
|
||||
// so nothing legitimate is caught by it.
|
||||
//
|
||||
// Two distinct paths land a non-RPC body here, and neither is an
|
||||
// empty-string special case. playwright-core's postData() is
|
||||
// `buffer.toString("utf-8") || null`, so an absent or empty body
|
||||
// decodes to null, JSON.parse("null") yields null, and the type guard
|
||||
// below reports it. A binary body is instead decoded LOSSILY into
|
||||
// mojibake — not null — which is not valid JSON, so the catch above
|
||||
// reports that one. Both end up reported; only the route differs.
|
||||
if (
|
||||
payload === null ||
|
||||
typeof payload !== "object" ||
|
||||
batch.length === 0 ||
|
||||
!batch.every((req) => req !== null && typeof req === "object")
|
||||
) {
|
||||
report("unstubbed request: POST " + route.request().url());
|
||||
|
||||
@@ -247,7 +247,7 @@ describe("a reveal that is not interrupted", () => {
|
||||
|
||||
expect(node("export-privkey-value").textContent).toBe("");
|
||||
expect(node("export-privkey-flash").textContent).toBe(
|
||||
"That password is not correct. Please try again.",
|
||||
"That password is incorrect. Please try again.",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
213
tests/passwordMessages.test.js
Normal file
213
tests/passwordMessages.test.js
Normal file
@@ -0,0 +1,213 @@
|
||||
// One wording for one condition (issue #172).
|
||||
//
|
||||
// Every screen that asks for the password decrypts the vault itself, and
|
||||
// each one used to write its own sentence for the same failure: the send
|
||||
// confirmation and the delete-wallet confirmation said "Wrong password."
|
||||
// (a fragment, which RULES.md Language & Labeling forbids), the reveal
|
||||
// screens said "That password is not correct.", and the two dApp approval
|
||||
// paths said "That password is incorrect." A user hitting two of those
|
||||
// minutes apart had no way to tell whether the wallet meant the same
|
||||
// thing.
|
||||
//
|
||||
// This scans the source rather than driving six views, because the
|
||||
// invariant is about the set of call sites and not about any one of them:
|
||||
// a seventh screen that decrypts the vault has to join the set, and a
|
||||
// DOM test per view cannot notice one that was never written.
|
||||
//
|
||||
// The assertions are per CALL SITE, not per file. approval.js decrypts in
|
||||
// two places and is where the divergence came from; a per-file check that
|
||||
// only asks whether the canonical sentence appears somewhere in the file
|
||||
// passes while one of those two says something else entirely. So each
|
||||
// call site is read back to its own catch handler and the prose that
|
||||
// handler shows the user must be the canonical sentence and nothing else
|
||||
// — which fails on a novel wording, not only on a known-superseded one.
|
||||
|
||||
const fs = require("fs");
|
||||
const path = require("path");
|
||||
|
||||
const SRC = path.join(__dirname, "..", "src");
|
||||
|
||||
const CANONICAL = "That password is incorrect. Please try again.";
|
||||
|
||||
// Wordings this repo has actually shipped for the same condition. This is
|
||||
// a secondary, whole-file sweep for stragglers outside a decrypt handler;
|
||||
// divergence at a call site is caught by the exact-match assertion, which
|
||||
// needs no list of phrasings to guess at.
|
||||
const SUPERSEDED = [
|
||||
"Wrong password.",
|
||||
"That password is not correct. Please try again.",
|
||||
];
|
||||
|
||||
function jsFilesUnder(dir) {
|
||||
return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
|
||||
const full = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) return jsFilesUnder(full);
|
||||
return entry.name.endsWith(".js") ? [full] : [];
|
||||
});
|
||||
}
|
||||
|
||||
// Blank out the interior of every comment and string literal, keeping the
|
||||
// offsets and line breaks, so braces can be counted without a quote or a
|
||||
// commented-out block throwing the count off. The literals are returned
|
||||
// alongside with the offset of their opening quote, which is how a
|
||||
// message is later attributed to the handler it sits in.
|
||||
function scan(source) {
|
||||
const masked = source.split("");
|
||||
const strings = [];
|
||||
const blank = (from, to) => {
|
||||
for (let k = from; k < to; k++) if (masked[k] !== "\n") masked[k] = " ";
|
||||
};
|
||||
let i = 0;
|
||||
while (i < source.length) {
|
||||
const two = source.slice(i, i + 2);
|
||||
if (two === "//") {
|
||||
const nl = source.indexOf("\n", i);
|
||||
const stop = nl === -1 ? source.length : nl;
|
||||
blank(i, stop);
|
||||
i = stop;
|
||||
} else if (two === "/*") {
|
||||
const close = source.indexOf("*/", i + 2);
|
||||
const stop = close === -1 ? source.length : close + 2;
|
||||
blank(i, stop);
|
||||
i = stop;
|
||||
} else if (
|
||||
source[i] === '"' ||
|
||||
source[i] === "'" ||
|
||||
source[i] === "`"
|
||||
) {
|
||||
const quote = source[i];
|
||||
let j = i + 1;
|
||||
let value = "";
|
||||
while (j < source.length && source[j] !== quote) {
|
||||
if (source[j] === "\\") {
|
||||
value += source[j + 1];
|
||||
j += 2;
|
||||
continue;
|
||||
}
|
||||
value += source[j];
|
||||
j += 1;
|
||||
}
|
||||
blank(i + 1, j);
|
||||
strings.push({ offset: i, value });
|
||||
i = j + 1;
|
||||
} else {
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
return { masked: masked.join(""), strings };
|
||||
}
|
||||
|
||||
// Offset of the `{` that opens the block containing `at`, or -1.
|
||||
function enclosingBlockStart(masked, at) {
|
||||
let depth = 0;
|
||||
for (let i = at; i >= 0; i--) {
|
||||
if (masked[i] === "}") depth += 1;
|
||||
else if (masked[i] === "{") {
|
||||
if (depth === 0) return i;
|
||||
depth -= 1;
|
||||
}
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
|
||||
// Offset just past the `}` matching the `{` at `open`.
|
||||
function blockEnd(masked, open) {
|
||||
let depth = 0;
|
||||
for (let i = open; i < masked.length; i++) {
|
||||
if (masked[i] === "{") depth += 1;
|
||||
else if (masked[i] === "}") {
|
||||
depth -= 1;
|
||||
if (depth === 0) return i + 1;
|
||||
}
|
||||
}
|
||||
throw new Error("unterminated block");
|
||||
}
|
||||
|
||||
// The catch handler guarding a given decryptWithPassword call: walk out to
|
||||
// the try block the call sits in, then take the catch that follows it.
|
||||
function handlerSpan(masked, callOffset, label) {
|
||||
const tryOpen = enclosingBlockStart(masked, callOffset);
|
||||
if (tryOpen === -1 || !/\btry\s*$/.test(masked.slice(0, tryOpen)))
|
||||
throw new Error(`${label}: the decrypt is not inside a try block`);
|
||||
const rest = masked.slice(blockEnd(masked, tryOpen));
|
||||
const catchMatch = /^\s*catch\s*(\([^)]*\)\s*)?\{/.exec(rest);
|
||||
if (!catchMatch)
|
||||
throw new Error(`${label}: the decrypt's try block has no catch`);
|
||||
const catchOpen = blockEnd(masked, tryOpen) + catchMatch[0].length - 1;
|
||||
return [catchOpen, blockEnd(masked, catchOpen)];
|
||||
}
|
||||
|
||||
// The prose the handler puts in front of the user. Element ids, class
|
||||
// names and visibility keywords are single words; a sentence has a space
|
||||
// in it, and that is the whole distinction needed here.
|
||||
function handlerMessages(file, callOffset, label) {
|
||||
const { masked, strings } = scan(fs.readFileSync(file, "utf8"));
|
||||
const [from, to] = handlerSpan(masked, callOffset, label);
|
||||
return strings
|
||||
.filter((s) => s.offset >= from && s.offset < to)
|
||||
.map((s) => s.value)
|
||||
.filter((v) => v.includes(" "));
|
||||
}
|
||||
|
||||
// The call sites are found, not listed: the file layout moves (the private
|
||||
// key export was in addressDetail.js when #172 was filed and is its own
|
||||
// view now), and a hardcoded list would quietly stop covering a screen it
|
||||
// no longer names.
|
||||
function callSites() {
|
||||
const sites = [];
|
||||
for (const file of jsFilesUnder(SRC)) {
|
||||
if (file === path.join(SRC, "shared", "vault.js")) continue;
|
||||
const { masked } = scan(fs.readFileSync(file, "utf8"));
|
||||
const rel = path.relative(SRC, file).split(path.sep).join("/");
|
||||
let n = 0;
|
||||
let at = masked.indexOf("decryptWithPassword(");
|
||||
while (at !== -1) {
|
||||
n += 1;
|
||||
sites.push({ file, rel, offset: at, label: `${rel} #${n}` });
|
||||
at = masked.indexOf("decryptWithPassword(", at + 1);
|
||||
}
|
||||
}
|
||||
return sites.sort((a, b) => a.label.localeCompare(b.label));
|
||||
}
|
||||
|
||||
describe("password failure messages", () => {
|
||||
const sites = callSites();
|
||||
const files = [...new Set(sites.map((s) => s.file))].sort();
|
||||
|
||||
test("the call sites are found where they are expected", () => {
|
||||
const counts = {};
|
||||
for (const site of sites)
|
||||
counts[site.rel] = (counts[site.rel] ?? 0) + 1;
|
||||
expect(counts).toEqual({
|
||||
"popup/views/approval.js": 2,
|
||||
"popup/views/confirmTx.js": 1,
|
||||
"popup/views/deleteWallet.js": 1,
|
||||
"popup/views/exportPrivkey.js": 1,
|
||||
"popup/views/showPhrase.js": 1,
|
||||
});
|
||||
});
|
||||
|
||||
test("the canonical message is a full sentence", () => {
|
||||
expect(CANONICAL).toMatch(/^[A-Z][^]*\.$/);
|
||||
});
|
||||
|
||||
// Exact equality, per call site: a message that is merely different
|
||||
// rather than known-obsolete fails here too, which a scan for historic
|
||||
// wordings cannot do.
|
||||
test.each(sites.map((s) => [s.label, s]))(
|
||||
"%s answers a rejected password with the canonical sentence",
|
||||
(label, site) => {
|
||||
expect(handlerMessages(site.file, site.offset, label)).toEqual([
|
||||
CANONICAL,
|
||||
]);
|
||||
},
|
||||
);
|
||||
|
||||
test.each(files.map((f) => [path.relative(SRC, f), f]))(
|
||||
"%s carries no superseded wording",
|
||||
(_rel, file) => {
|
||||
const source = fs.readFileSync(file, "utf8");
|
||||
for (const old of SUPERSEDED) expect(source).not.toContain(old);
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -159,3 +159,113 @@ describe("hideSpoofedSymbols persistence", () => {
|
||||
expect(second.mod.state.hideSpoofedSymbols).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// restoreView() refuses to reopen ONTO a non-restorable view, but the stack
|
||||
// behind it was restored verbatim, so Back could still walk onto a screen
|
||||
// whose content is deliberately never re-rendered — and "show-phrase" has no
|
||||
// Back control of its own to leave by. The stack is filtered on load, at the
|
||||
// first entry the popup would not render, and everything above it goes too:
|
||||
// those entries were reached THROUGH the dropped one.
|
||||
describe("restored viewStack is filtered against RESTORABLE_VIEWS", () => {
|
||||
const NON_RESTORABLE = ["export-privkey", "show-phrase"];
|
||||
|
||||
function restoredStack(viewStack, currentView = "settings") {
|
||||
return loadModuleWith({
|
||||
wallets: oneWallet(),
|
||||
currentView,
|
||||
viewStack,
|
||||
});
|
||||
}
|
||||
|
||||
test("a non-restorable view at the top of the stack is dropped", async () => {
|
||||
const { mod } = restoredStack(["main", "address", "export-privkey"]);
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual(["main", "address"]);
|
||||
});
|
||||
|
||||
test("a non-restorable view in the middle truncates the stack there", async () => {
|
||||
const { mod } = restoredStack(["main", "show-phrase", "address"]);
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual(["main"]);
|
||||
});
|
||||
|
||||
// Truncating a stack rooted at a non-restorable view leaves nothing, and
|
||||
// the restored view still needs somewhere for Back to go.
|
||||
test("a non-restorable view at the bottom leaves main to go back to", async () => {
|
||||
const { mod } = restoredStack(["export-privkey", "address", "receive"]);
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual(["main"]);
|
||||
});
|
||||
|
||||
test("no restored stack retains a secret-bearing view", async () => {
|
||||
for (const view of NON_RESTORABLE) {
|
||||
const { mod } = restoredStack(["main", "address", view, "receive"]);
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).not.toContain(view);
|
||||
}
|
||||
});
|
||||
|
||||
// The rule is "views the popup will render", not a blocklist of the two
|
||||
// secret screens: a name no longer in the set (or never a view at all)
|
||||
// has to go the same way.
|
||||
test("a name that is not a restorable view at all is dropped", async () => {
|
||||
const { mod } = restoredStack(["main", "welcome", "address"]);
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual(["main"]);
|
||||
});
|
||||
|
||||
// Restorable entries are kept verbatim. That they are then unhidden
|
||||
// without being re-rendered is a separate defect, tracked in #268; this
|
||||
// filter is only about views the popup declined to restore.
|
||||
test("an ordinary restorable stack is restored unchanged", async () => {
|
||||
const stack = ["main", "address", "address-token"];
|
||||
const { mod } = restoredStack(stack);
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual(stack);
|
||||
});
|
||||
|
||||
test("restoring onto main keeps the stack empty", async () => {
|
||||
const { mod } = restoredStack(["show-phrase"], "main");
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual([]);
|
||||
});
|
||||
|
||||
// main is not the only view that gets no ["main"] beneath it: restoreView()
|
||||
// will not reopen onto a non-restorable view either, so nothing is left for
|
||||
// Back to sit under and the stack stays empty.
|
||||
test("restoring onto a view the popup will not reopen keeps the stack empty", async () => {
|
||||
const { mod } = restoredStack(["export-privkey"], "show-phrase");
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual([]);
|
||||
});
|
||||
|
||||
// Not an array means nothing survives, but the never-empty rule still
|
||||
// applies: a corrupt stack must not leave a restored view with no Back
|
||||
// target of its own.
|
||||
test("a stack that is not an array still gets main beneath a restored view", async () => {
|
||||
const { mod } = restoredStack("main");
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual(["main"]);
|
||||
});
|
||||
|
||||
test("a stack that is not an array loads as empty under main", async () => {
|
||||
const { mod } = restoredStack({ 0: "main" }, "main");
|
||||
await mod.loadState();
|
||||
expect(mod.state.viewStack).toEqual([]);
|
||||
});
|
||||
|
||||
// Filtering belongs on load, not on save: the live in-session stack is
|
||||
// legitimate — the user really is one Back away from a screen that is
|
||||
// rendered right now — and only a load-side filter also cleans the
|
||||
// stacks already sitting in storage.
|
||||
test("saveState persists the live stack verbatim", async () => {
|
||||
const { mod, set } = loadModuleWith(null);
|
||||
mod.state.viewStack = ["main", "address", "export-privkey"];
|
||||
await mod.saveState();
|
||||
expect(set).toHaveBeenCalledWith({
|
||||
autistmask: expect.objectContaining({
|
||||
viewStack: ["main", "address", "export-privkey"],
|
||||
}),
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user