Compare commits

...
Author SHA1 Message Date
sneak 9a5bd8d39c fix: estimate a request even when the site sends its gas limit (closes #554)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
ethers asks the node for a gas estimate only when the request has no gas
limit, and that estimate is the wallet's only check that a transaction
does not revert. Since a site's `gas` became the gas limit, a reverting
request carrying one opened the approval window and could be signed and
mined to fail. Such a request is now estimated first, with the site's
limit and under the same timeout; a failure goes back to the site with no
window, and the site's gas limit is still the one shown and signed. A
limit too low for the call now fails the same way.

Model: opus-5-5
2026-10-14 11:57:47 +00:00
clawbot 2442441830 fix: use the gas limit a site sends as gas (closes #207)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
Sites send the gas limit under its JSON-RPC name, `gas`. ethers only
reads `gasLimit`, so the field was dropped and the node's estimate was
shown and signed instead. The page's request now takes `gas` as the gas
limit, and the existing ceiling, display and comparison with the signed
transaction cover it as they already did `gasLimit`. A request that
gives both with different values is refused.

Model: opus-5-5
2026-10-14 13:48:52 +02:00
clawbot f1a13a50a6 fix: the defective xprv wallet notice names the keys that still reach its addresses (closes #255)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
The notice told the user that these addresses are reachable by software
that repeats the same derivation, while the same wallet blocks Export
Private Key, so there was no way to take that route from here. The
second paragraph now names private key export among the disabled
controls and says the addresses can be reached outside this wallet with
the extended private key the user imported, if they kept a copy of it,
or with the master extended private key or recovery phrase it came
from. No behaviour changes. A test checks the new wording.

Model: opus-5-5
2026-10-14 11:48:46 +02:00
clawbot b4059af827 fix: a listed site ending in a dot gets the phishing warning (closes #534)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
The blocklist check hashed the hostname as the page gave it, so
"myetheywallet.com." and every subdomain written that way hashed
differently from the listed "myetheywallet.com" and opened approval
prompts with no warning. The browser and DNS treat both spellings as
one host. hostnameVariants() now drops one trailing dot before
building the hostname and its parent domains, so the check sees the
name the list holds.

Model: opus-5-5
2026-10-14 10:48:41 +02:00
clawbot 8c08b3d467 docs: say what a closed window does to a tx or sign approval (closes #505)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
The approval port's disconnect handler leaves a transaction or signing
approval pending, and its comments said that was so the user could reopen
the toolbar popup. Those approvals open in their own window. When that
window closes, the windows.onRemoved listener rejects the approval, unless
an approved attempt is still running, in which case that attempt answers
the page. Both comments now say so. No code changed.

Model: opus-5-5
2026-10-14 09:48:41 +02:00
clawbot fd5921f62e fix: make fmt refuses to run without the repo's own prettier (closes #509)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
In a clone where script/bootstrap has not run, node_modules/ is empty and
yarn falls back to whatever prettier is on the PATH, which can format files
differently from the version yarn.lock pins. script/fmt and script/fmt-check
now stop with a message telling the user to run script/bootstrap when
node_modules/.bin/prettier is missing. After bootstrap they behave as
before. The README Entrypoints entries say so.

Model: opus-5-5
2026-10-14 09:31:47 +02:00
clawbot 83f3549d37 fix: typed data for another chain cannot be signed (closes #519)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
EIP-712 has a wallet refuse typed data whose domain chainId is not the
active chain, because the signature is valid on the chain it names. The
sign screen now shows such a request with the reason on its error line
and Sign disabled, the same way it refuses a mismatched primary type.
The background checks again when the signature comes back, against the
network active then, and answers the site with that refusal instead of
the signature. A chain id is read as ethers signs it, so 1, "1" and
"0x1" are one chain. Typed data with no chainId signs as before.

Model: opus-5-5
2026-10-14 08:31:44 +02:00
clawbot eb012b9eb3 fix: Chrome draws the popup's text at the 14px text-sm declares (closes #456)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
Chrome gives extension pages a stylesheet of its own that sets body to
75%, outside any cascade layer, so it beat Tailwind's text-sm and Chrome
drew the popup at 12px where Firefox draws 14px. body now carries
text-sm!, the important form, as it already carries font-mono!. Both
browser suites check that body computes to 14px.

At 14px the recovery phrase instruction on the add wallet screen takes a
third line, which pushed Import below the fold at 360x600. The die button
now shares the instruction's row instead of taking a row of its own.

Model: opus-5-5
2026-10-14 08:02:14 +02:00
clawbot adff1e0b9d docs: a change adds no Completed Steps entry to TODO.md (closes #222)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
Every change used to add an entry at the top of Completed Steps in its
own commit, so each merge into `next` left every other open PR
conflicting on that list. The Workflow section now says to add no
entry: what landed is recorded on the tracker and in `git log`, where
each commit title names the issue it closes. The step that moved Next
Step into Completed Steps and pulled the top of Future Steps into Next
Step goes with it. The entries already in Completed Steps stay.

Model: opus-5-5
2026-10-14 05:15:05 +02:00
clawbot 8e52528f8b test: wait until the page shows a screen, not only until it lays out (closes #502)
check / check (push) Canceled after 0s
e2e / e2e-chrome (push) Canceled after 0s
e2e / e2e-firefox (push) Canceled after 0s
The browser suites' wait for a screen passed as soon as the element laid
out. Until the page's stylesheet has applied, every view lays out, so an
approval test could read the prompt before the page's script had filled
it. visible() in the Chrome harness and waitVisible() in the Firefox
driver now also wait for the page to finish loading and for neither the
element nor anything around it to carry the hidden class that showView()
keeps on every view but the current one. Both also require the element's
computed visibility to be visible, as the browser library's own check in
Chrome did. No caller changed.

Model: opus-5-5
2026-10-08 12:49:43 +02:00
21 changed files with 455 additions and 51 deletions
+20 -4
View File
@@ -239,8 +239,12 @@ provide:
`make check` stays non-mutating. Linting runs only in the container, because
an ESLint result that depends on whichever ESLint the host happens to have is
not a result, so docker is required to lint.
- `script/fmt` — format all files (writes), on the host
- `script/fmt-check` — check formatting (read-only), on the host
- `script/fmt` — format all files (writes), on the host, with the `prettier`
that `script/bootstrap` installs in `node_modules/`; when that one is missing
it fails and says to run `script/bootstrap`, instead of using a `prettier`
from the `PATH`
- `script/fmt-check` — check formatting (read-only), on the host, with the same
`prettier` and the same refusal
- `script/check` — run `script/test`, `script/lint` and `script/fmt-check`
- `script/check-censored` — assert the competitor name RULES.md bars appears
nowhere in the working tree or under `dist/` outside its documented
@@ -2071,7 +2075,11 @@ view would leave a wallet one click from deletion.
artifact can be compared with it field for field. A nonce the site supplies is
ignored: the nonce is always the account's next nonce from the node, so a site
cannot replace one of the user's pending transactions or leave this one stuck
behind a gap. A request that cannot be populated — unreachable node, reverting
behind a gap. A gas limit the site supplies, as `gas` or `gasLimit`, is the
one shown and signed in place of the node's estimate, but the node still
estimates the transaction with that limit, as the check that it neither
reverts nor runs out of gas; a request that gives both with different values
is refused. A request that cannot be populated — unreachable node, reverting
gas estimate — opens no window and is failed back to the site. Only one
transaction approval exists at a time: populating fixes the nonce, so a second
`eth_sendTransaction` arriving while one is unanswered is refused with
@@ -2160,6 +2168,13 @@ view would leave a wallet one click from deletion.
- Typed data that states no primary type, or one other than the type it
would be signed as, or that cannot be read → shown with the error line
saying so and "Sign" disabled; only "Reject" remains
- Typed data whose domain `chainId` names a chain other than the active
network's → shown with the error line naming both chain ids and "Sign"
disabled; only "Reject" remains. EIP-712 asks for this, because the
signature is valid on the chain the domain names. Typed data with no
`chainId` is not refused. The background refuses such a signature too,
against the network active when the signature reaches it, and answers the
site with the same sentence
- A `personal_sign` or `eth_sign` message that is not hex (`0x` or `0X` and
an even number of hex digits, the form ethers' `getBytes` reads when
signing) → shown as plain text, with the error line "This message is plain
@@ -2652,7 +2667,8 @@ plaintext list of anyone's domain names.
When a dApp on a blocklisted domain requests a wallet connection, transaction
approval, or signature, the approval popup displays a prominent red warning
banner alerting the user. The domain checker matches exact hostnames and all
parent domains (subdomain matching).
parent domains (subdomain matching), and checks a hostname written with a
trailing dot (`name.`) as `name`.
#### Transaction Decoding
+13 -4
View File
@@ -2,10 +2,9 @@
- `git pull` `next` and cut a branch from it — one branch per issue, named
`issue-<N>-<slug>`. Never branch from `main`.
- Do the work as one commit whose title ends with ` (closes #N)`, with the
`TODO.md` update in that same commit.
- Move Next Step to the top of Completed Steps; move the top item of Future
Steps into Next Step.
- Do the work as one commit whose title ends with ` (closes #N)`.
- Add no entry to Completed Steps. What landed is recorded on the tracker and in
`git log`, where each commit title names the issue it closes.
- Run `make fmt`, then `make check`. A feature branch may be red; `next` and
`main` may not.
- Rebase onto current `next` immediately before pushing — other branches land on
@@ -44,6 +43,16 @@ then continue tagging as milestones land.
# Completed Steps
- 2026-10-08: The browser suites no longer read a screen before the page has
shown it ([#502](https://git.eeqj.de/sneak/AutistMask/issues/502)). Their wait
for a screen used to pass as soon as the element laid out, which every view
does until the page's stylesheet has applied, so an approval test could read
the prompt's fields before the page's script had filled them. `visible()` in
`tests/e2e/harness.js` and `waitVisible()` in `tests/e2e/firefox/driver.js`
now also wait for the page to finish loading and for neither the element nor
anything around it to carry the `hidden` class that `showView()` puts on every
view but the current one. No caller changed.
- 2026-10-08: Switching the network in Settings now tells open pages
([#500](https://git.eeqj.de/sneak/AutistMask/issues/500)). Once the switch is
saved, Settings asks the background to send `chainChanged` with the new chain
+7
View File
@@ -25,6 +25,13 @@ run_yarn() {
main() {
cd "$ROOT"
# Without the repo's own prettier, yarn runs whatever prettier is on
# the PATH, which may be a version that formats files differently.
if [ ! -x node_modules/.bin/prettier ]; then
echo "fmt: this repo's prettier is not installed in node_modules/," \
"so run script/bootstrap first." >&2
exit 1
fi
echo "Formatting..."
run_yarn run fmt
}
+7
View File
@@ -26,6 +26,13 @@ run_yarn() {
main() {
cd "$ROOT"
# Without the repo's own prettier, yarn runs whatever prettier is on
# the PATH, which may be a version that formats files differently.
if [ ! -x node_modules/.bin/prettier ]; then
echo "fmt-check: this repo's prettier is not installed in" \
"node_modules/, so run script/bootstrap first." >&2
exit 1
fi
echo "Checking formatting..."
run_yarn run fmt-check
}
+23 -4
View File
@@ -22,6 +22,7 @@ const { debugFetch, log } = require("../shared/log");
const {
verifySignedTx,
verifySignature,
typedDataChainRefusal,
failureIsRetryable,
describeTxFailure,
sameAddress,
@@ -632,8 +633,9 @@ function isExtensionSender(sender) {
// had refused.
//
// TX and sign approvals do not decide here. They stay pending across a
// disconnect — the user can reopen the toolbar popup — and are rejected by the
// windows.onRemoved listener below.
// disconnect. When their window closes, the windows.onRemoved listener below
// rejects them, unless an approved attempt is still running, in which case
// that attempt answers the page.
runtime.onConnect.addListener((port) => {
if (port.name.startsWith("approval:")) {
const id = port.name.split(":")[1];
@@ -661,7 +663,9 @@ runtime.onConnect.addListener((port) => {
const approval = pendingApprovals[id];
if (approval) {
if (approval.type === "tx" || approval.type === "sign") {
// Keep pending — user can reopen the toolbar popup
// When its window closes, windows.onRemoved rejects it,
// unless an approved attempt is still running, in which
// case that attempt answers the page.
return;
}
settleApproval(id, { approved: false, remember: false });
@@ -1764,7 +1768,8 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
(async () => {
try {
const activeAddress = await getActiveAddress();
const s = await getState();
const activeAddress = activeAddressOf(s);
// 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.
@@ -1783,6 +1788,20 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
signature,
approval.approvedFrom,
);
// The sign screen refuses typed data whose domain names
// another chain. A signature for it that arrives anyway is
// refused here, against the network active now.
const sp = approval.signParams;
if (
sp.method === "eth_signTypedData_v4" ||
sp.method === "eth_signTypedData"
) {
const refusal = typedDataChainRefusal(
JSON.parse(sp.typedData).domain,
networkById(s.networkId),
);
if (refusal) throw new ApprovalMismatchError(refusal);
}
settleApproval(msg.id, { signature }, { holdsClaim: true });
sendResponse({ signature });
} catch (e) {
+12 -8
View File
@@ -7,9 +7,10 @@
<link rel="stylesheet" href="styles.css" />
</head>
<!-- Chrome gives extension pages a stylesheet of its own that sets the
font on body, and a Tailwind class beats it only when marked
important: hence font-mono! rather than font-mono. -->
<body class="bg-bg text-fg font-mono! text-sm">
font and its size on body, and a Tailwind class beats it only when
marked important: hence font-mono! and text-sm! rather than
font-mono and text-sm. -->
<body class="bg-bg text-fg font-mono! text-sm!">
<div id="app" class="p-2 pr-5 overflow-x-hidden">
<!-- ============ GLOBAL TITLE BAR ============ -->
<div
@@ -87,11 +88,14 @@
<!-- Mnemonic form section -->
<div id="add-wallet-section-mnemonic">
<p class="mb-2">
Enter your 12 or 24 word recovery phrase below, or click
the button to roll the die for a new one.
</p>
<div class="mb-1 flex justify-end">
<!-- The die button shares the instruction's row: on a row
of its own it pushes Import below the fold at
360x600. -->
<div class="mb-2 flex justify-between items-start gap-2">
<p>
Enter your 12 or 24 word recovery phrase below, or
click the button to roll the die for a new one.
</p>
<button
id="btn-generate-phrase"
class="border border-border px-2 py-1 hover:bg-fg hover:text-bg cursor-pointer text-xs"
+8 -4
View File
@@ -12,7 +12,7 @@ const {
formatFee,
tokenLabel,
} = require("./helpers");
const { state, saveState } = require("../../shared/state");
const { state, saveState, currentNetwork } = require("../../shared/state");
const {
networkById,
networkByChainId,
@@ -46,7 +46,10 @@ const {
const { decryptWithPassword } = require("../../shared/vault");
const { getSignerForAddress } = require("../../shared/wallet");
const { walletDefect } = require("../../shared/walletDefects");
const { describeSigningFailure } = require("../../shared/approvalVerify");
const {
describeSigningFailure,
typedDataChainRefusal,
} = require("../../shared/approvalVerify");
const txStatus = require("./txStatus");
const uniswap = require("../../shared/uniswap");
const { notify, runtimeApi, sendMessage } = require("../../shared/browserApi");
@@ -443,7 +446,8 @@ function signedPrimaryType(types) {
// Why a signature request cannot be signed, as a sentence for the error line,
// or null when it can. Only typed data is refused: the `primaryType` the page
// states has to be the type ethers will sign, or this screen would name one
// message while another is signed.
// message while another is signed. Its domain may name no chain but the active
// network's.
function typedDataRefusal(sp) {
if (sp.method === "personal_sign" || sp.method === "eth_sign") return null;
let data;
@@ -466,7 +470,7 @@ function typedDataRefusal(sp) {
", so it cannot be signed."
);
}
return null;
return typedDataChainRefusal(data.domain, currentNetwork());
}
// The largest amount a Permit2 allowance can hold, a uint160. Permit2 treats
+53 -15
View File
@@ -1,13 +1,14 @@
// 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.
// A dApp's eth_sendTransaction normally fixes `to`, `value` and `data`, and
// sometimes the gas limit. The nonce, the fees, and the gas limit when the
// site gives none 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
@@ -31,6 +32,7 @@ const {
VoidSigner,
accessListify,
getAddress,
getBigInt,
getBytes,
hexlify,
toQuantity,
@@ -47,10 +49,11 @@ const {
// 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.
// The request fields taken from the page, plus `gas`, which requestFrom()
// takes as the gas limit. 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.
//
// The nonce is not taken from the page; it is always the account's next nonce
// from the network. A page that chose it could replace one of the user's
@@ -84,6 +87,16 @@ function present(v) {
return v !== null && v !== undefined && v !== "";
}
// Whether two quantities from the page are the same number. One that is not a
// number matches nothing.
function sameQuantity(a, b) {
try {
return getBigInt(a) === getBigInt(b);
} catch {
return false;
}
}
// 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) {
@@ -111,6 +124,20 @@ function requestFrom(txParams, from) {
for (const key of REQUEST_FIELDS) {
if (present(txParams[key])) request[key] = txParams[key];
}
// JSON-RPC calls the gas limit `gas`, and that is the name sites send.
// ethers only reads `gasLimit`: left as `gas` it would be dropped and an
// estimate signed in its place.
if (present(txParams.gas)) {
if (
present(request.gasLimit) &&
!sameQuantity(request.gasLimit, txParams.gas)
) {
throw fail(
"The site asked for two different gas limits for this transaction.",
);
}
request.gasLimit = txParams.gas;
}
if (
present(request.type) &&
!ALLOWED_TX_TYPES.includes(Number(request.type))
@@ -174,6 +201,7 @@ async function prepareApprovalTx(provider, from, txParams) {
throw fail("There is no active address to send this transaction from.");
}
const request = requestFrom(txParams || {}, from);
const signer = new VoidSigner(getAddress(from), provider);
let populated;
try {
@@ -181,10 +209,20 @@ async function prepareApprovalTx(provider, from, txParams) {
// 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.
//
// ethers asks the node for a gas estimate only when the request has
// no gas limit, and that estimate is the only check that the
// transaction does not revert. So a request carrying the site's gas
// limit is estimated first, with that limit: one that reverts, or
// needs more gas than the site gave it, fails here as well. The
// site's gas limit is still the one populated.
populated = await withTimeout(
new VoidSigner(getAddress(from), provider).populateTransaction(
request,
),
(async () => {
if (present(request.gasLimit)) {
await signer.estimateGas(request);
}
return signer.populateTransaction(request);
})(),
POPULATE_TIMEOUT_MS,
"The transaction could not be prepared: the network did not answer in time.",
);
+29
View File
@@ -53,6 +53,7 @@ const {
accessListify,
formatEther,
getAddress,
getBigInt,
getBytes,
toQuantity,
verifyMessage,
@@ -635,6 +636,33 @@ function verifySignature(signParams, signature, expectedFrom) {
return recovered;
}
// Why typed data cannot be signed while `network` is the active network, as a
// sentence, or null when it can. EIP-712 has a wallet refuse typed data whose
// domain names another chain, because the signature is valid on that chain.
// ethers signs no chain id when the domain's is null or absent, and reads any
// other with getBigInt(), so 1, "1" and "0x1" are the same chain here too.
function typedDataChainRefusal(domain, network) {
const chainId = domain?.chainId;
if (chainId === undefined || chainId === null) return null;
let named;
try {
named = getBigInt(chainId);
} catch {
return "This typed data cannot be read, so it cannot be signed.";
}
const active = getBigInt(network.chainId);
if (named === active) return null;
return (
"This typed data is for chain id " +
named +
", but the active network is " +
network.name +
" (chain id " +
active +
"), so it cannot be signed."
);
}
// The stage a transaction approval failed at. Which stage it is decides
// whether the approval survives the failure.
const TX_STAGE_SIGN = "sign";
@@ -796,6 +824,7 @@ function describeSigningFailure(response, fallbackMessage) {
module.exports = {
verifySignedTx,
verifySignature,
typedDataChainRefusal,
assertNoForbiddenFields,
assertNothingUnchecked,
assertCanonicalBytes,
+4 -1
View File
@@ -110,11 +110,14 @@ function hashListed(hash) {
* Generate hostname variants for subdomain matching.
* "sub.evil.com" yields ["sub.evil.com", "evil.com"].
*
* A trailing dot is dropped: "evil.com." is the fully qualified spelling of
* "evil.com", and the browser loads the same site from either.
*
* @param {string} hostname
* @returns {string[]}
*/
function hostnameVariants(hostname) {
const h = hostname.toLowerCase();
const h = hostname.toLowerCase().replace(/\.$/, "");
const variants = [h];
const parts = h.split(".");
// Parent domains: a.b.c.d -> b.c.d, c.d
+7 -5
View File
@@ -36,11 +36,13 @@ const DEFECTS = {
"derivation path beneath that key instead of from a master " +
"key, so the addresses listed here are not the ones that key " +
"produces under the standard path.",
"Signing and sending are disabled for this wallet. The addresses " +
"do descend from the extended private key you imported, so " +
"anything they hold is still reachable by software that " +
"repeats the same non-standard derivation. Check them in a " +
"block explorer before deciding what to do.",
"Signing, sending and private key export are disabled for this " +
"wallet. Anything these addresses hold can be reached outside " +
"this wallet with the extended private key you imported, if " +
"you kept a copy of it, or with the master extended private " +
"key or recovery phrase it came from, in software that " +
"repeats the same non-standard derivation. Check the " +
"addresses in a block explorer before deciding what to do.",
"To see the addresses this key produces under the standard path, " +
"import the master extended private key, or the recovery " +
"phrase it came from, as a new wallet. Nothing here has been " +
+69 -1
View File
@@ -9,7 +9,7 @@
// 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 { Network, Transaction, Wallet, toQuantity } = require("ethers");
const {
prepareApprovalTx,
serializeApprovedTx,
@@ -138,6 +138,49 @@ describe("prepareApprovalTx", () => {
expect(approved.maxFeePerGas).toBe("0x12a05f200");
});
// `gas` is the JSON-RPC name for the gas limit and the one sites send.
// The node is still asked for its estimate, with the site's gas limit,
// because the estimate is the check that the transaction does not revert.
// It estimates 21000; the site's 200000 is what is shown and what is
// signed.
test("uses the gas limit a site sends as gas", async () => {
const estimateGas = jest.fn(async () => 21000n);
const approved = await prepareApprovalTx(
providerWith({ estimateGas }),
signer.address,
{ ...TX_PARAMS, gas: "0x30d40" },
);
expect(estimateGas).toHaveBeenCalledWith(
expect.objectContaining({ gasLimit: 200000n }),
);
expect(approved.gasLimit).toBe("0x30d40");
const signed = Transaction.from(
await signer.signTransaction({ ...approved }),
);
expect(signed.gasLimit).toBe(200000n);
});
test("accepts gas and gasLimit that name the same number", async () => {
const approved = await prepareApprovalTx(
providerWith(),
signer.address,
{ ...TX_PARAMS, gas: "0x30d40", gasLimit: 200000 },
);
expect(approved.gasLimit).toBe("0x30d40");
});
test("refuses gas and gasLimit that disagree", async () => {
await expect(
prepareApprovalTx(providerWith(), signer.address, {
...TX_PARAMS,
gas: "0x30d40",
gasLimit: "0x5208",
}),
).rejects.toThrow(
"The site asked for two different gas limits for this transaction.",
);
});
test("carries an access list the request asked for", async () => {
const approved = await prepareApprovalTx(
providerWith(),
@@ -212,6 +255,15 @@ describe("prepareApprovalTx", () => {
).rejects.toThrow(/gas limit no network this wallet supports/);
});
test("refuses a gas limit the site sent above the ceiling", async () => {
await expect(
prepareApprovalTx(providerWith(), signer.address, {
...TX_PARAMS,
gas: toQuantity(MAX_GAS_LIMIT + 1n),
}),
).rejects.toThrow(/gas limit no network this wallet supports/);
});
// The combined bound at population: a gas limit and a fee that are each
// under their own ceiling but multiply to thousands of ETH is refused
// before the approval window opens, so the user is never shown a
@@ -251,6 +303,22 @@ describe("prepareApprovalTx", () => {
expect(thrown.message).toMatch(/^[A-Z].*\.$/);
});
test("fails back a reverting request that sends its own gas limit", async () => {
const reverting = providerWith({
estimateGas: async () => {
throw new Error("execution reverted: ERC20: transfer amount");
},
});
await expect(
prepareApprovalTx(reverting, signer.address, {
...TX_PARAMS,
gas: "0x30d40",
}),
).rejects.toThrow(
/^The transaction could not be prepared: execution reverted/,
);
});
// 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 () => {
-1
View File
@@ -58,7 +58,6 @@ const TX_PARAMS = {
to: RECIPIENT,
value: "0x2386f26fc10000",
data: "0xdeadbeef",
gas: "0x5208",
};
// The values populateTransaction() fills in when the dApp fixed none of them.
+56 -1
View File
@@ -397,7 +397,13 @@ function loadBackground(options) {
rpcResult = r;
},
);
return { result: () => rpcResult };
return {
id: () =>
new URL(created[created.length - 1].url).searchParams.get(
"approval",
),
result: () => rpcResult,
};
}
// A dApp asking to connect. The origin defaults to one the persisted
@@ -1484,6 +1490,55 @@ describe("what the approval is verified against", () => {
});
});
// Typed data whose domain names another chain is refused on the sign
// screen; a signature for it that reaches the background anyway is not
// handed to the page (https://git.eeqj.de/sneak/AutistMask/issues/519).
// TYPED_DATA names mainnet.
async function answerTypedData(bg, pending) {
const { domain, types, message } = JSON.parse(TYPED_DATA);
delete types.EIP712Domain;
const signature = await signer.signTypedData(domain, types, message);
const answer = bg.send(
{
type: "AUTISTMASK_SIGN_RESPONSE",
id: pending.id(),
approved: true,
signature,
},
{ url: bg.fromPopup.url },
);
await settle();
return { answer, signature };
}
test("typed data for the active chain is handed back signed", async () => {
const bg = loadBackground();
const pending = bg.requestTypedData();
await settle();
const { answer, signature } = await answerTypedData(bg, pending);
expect(answer.sendResponse).toHaveBeenCalledWith({ signature });
expect(pending.result()).toEqual({ result: signature });
});
test("typed data for another chain is refused, not handed back", async () => {
const bg = loadBackground();
bg.setNetwork(SEPOLIA);
const pending = bg.requestTypedData();
await settle();
const { answer } = await answerTypedData(bg, pending);
const refusal =
"This typed data is for chain id 1, but the active network is Sepolia Testnet (chain id 11155111), so it cannot be signed.";
expect(answer.sendResponse).toHaveBeenCalledWith({
error: refusal,
retryable: false,
});
expect(pending.result()).toEqual({ error: { message: refusal } });
});
test("a sign request from another address raises no approval at all", async () => {
const bg = loadBackground();
const pending = bg.requestSign(other.address);
+8 -2
View File
@@ -279,12 +279,18 @@ class Driver {
// Shown means shown: in the popup a view is switched by toggling a
// "hidden" class, and an element that is present but collapsed is not
// the thing a test means by visible.
// the thing a test means by visible. Until the page's stylesheet has
// applied that class hides nothing and every view lays out, so the page
// must also have finished loading, which it does only after its
// stylesheet, and neither the element nor anything enclosing it may
// carry the class (https://git.eeqj.de/sneak/AutistMask/issues/502).
async waitVisible(selector, timeout = DEFAULT_WAIT_MS) {
return this.waitFor(
"selector " + selector + " to be visible",
`const el = document.querySelector(arguments[0]);
if (!el) return false;
if (document.readyState !== "complete" || !el) return false;
if (el.closest(".hidden")) return false;
if (getComputedStyle(el).visibility !== "visible") return false;
const r = el.getBoundingClientRect();
return r.width > 0 && r.height > 0;`,
[selector],
+12
View File
@@ -128,6 +128,18 @@ step("the popup is drawn in the monospace font it declares", async (env) => {
);
});
// The same check as the Chrome suite's (#456), so both browsers are held to
// the same size.
step("the popup's text is drawn at the 14px text-sm declares", async (env) => {
const size = await env.driver.execute(
"return getComputedStyle(document.body).fontSize;",
);
assert(
size === "14px",
"the popup's text is drawn at " + size + ", not at the 14px of text-sm",
);
});
// The recovery screen (#361): the Chrome suite's four cases, run before any
// wallet exists for the same reason. With no wallet nothing saves on a timer,
// so no save can write a good record over the unreadable one. The last of them
+26 -1
View File
@@ -333,8 +333,33 @@ async function launch(routeOpts) {
const PASSWORD = "e2e-harness-password";
// Waits until the page shows `selector`, not only until it lays out. Until the
// page's stylesheet has applied, the `hidden` class that showView() keeps on
// every view but the current one hides nothing and every view lays out, so a
// test could read a screen before the page's script had filled it
// (https://git.eeqj.de/sneak/AutistMask/issues/502). So the page must also
// have finished loading, which it does only after its stylesheet, and neither
// the element nor anything enclosing it may carry `hidden`. Polled on a timer:
// animation frames are not guaranteed to a window that is not in front.
async function visible(page, selector, timeout = 15000) {
await page.waitForSelector(selector, { state: "visible", timeout });
await page
.waitForFunction(
(sel) => {
const el = document.querySelector(sel);
if (document.readyState !== "complete" || !el) return false;
if (el.closest(".hidden")) return false;
if (getComputedStyle(el).visibility !== "visible") return false;
const r = el.getBoundingClientRect();
return r.width > 0 && r.height > 0;
},
selector,
{ polling: 50, timeout },
)
.catch((e) => {
throw new Error(
"the page did not show " + selector + ": " + e.message,
);
});
}
// An empty WebAssembly module: magic number and version header, no
+12
View File
@@ -113,6 +113,18 @@ test("the popup is drawn in the monospace font it declares (#418)", async (env)
);
});
// The same stylesheet sets the size on body to 75%, which would draw the
// popup at 12px.
test("the popup's text is drawn at the 14px text-sm declares (#456)", async (env) => {
const size = await env.page.evaluate(
() => getComputedStyle(document.body).fontSize,
);
assert(
size === "14px",
"the popup's text is drawn at " + size + ", not at the 14px of text-sm",
);
});
// The empirical half of #182. The manifest change is only a claim about
// what the CSP permits; this is the observation. Two things have to hold
// together, and the run covers both: the popup realm compiles WASM (here),
+13
View File
@@ -89,6 +89,12 @@ describe("vendored blocklist", () => {
expect(isPhishingDomain(LISTED[0].toUpperCase())).toBe(true);
});
test("a hostname ending in a dot is the same host", () => {
expect(isPhishingDomain(LISTED[1] + ".")).toBe(true);
expect(isPhishingDomain("wallet." + LISTED[1] + ".")).toBe(true);
expect(isPhishingDomain(CLEAN[0] + ".")).toBe(false);
});
test("returns false for an empty or missing hostname", () => {
expect(isPhishingDomain("")).toBe(false);
expect(isPhishingDomain(null)).toBe(false);
@@ -133,6 +139,13 @@ describe("hostnameVariants", () => {
test("lowercases hostnames", () => {
expect(hostnameVariants("Evil.COM")).toEqual(["evil.com"]);
});
test("drops the trailing dot of a fully qualified hostname", () => {
expect(hostnameVariants("sub.evil.com.")).toEqual([
"sub.evil.com",
"evil.com",
]);
});
});
describe("domain hashing", () => {
+64
View File
@@ -240,6 +240,7 @@ function request(data) {
beforeEach(() => {
state.trackedTokens = [];
state.wallets = [];
state.networkId = "mainnet";
});
describe("a token permission is warned about, naming spender and amount", () => {
@@ -532,3 +533,66 @@ describe("the sign screen refuses a mismatched primary type", () => {
expect(node("btn-approve-sign").disabled).toBe(true);
});
});
// EIP-712: a wallet refuses typed data whose domain names a chain other than
// the active one, because the signature is valid on the chain it names
// (https://git.eeqj.de/sneak/AutistMask/issues/519). The wallet is on Sepolia
// and the site asks for a mainnet USDC Permit.
describe("the sign screen refuses typed data for another chain", () => {
const REFUSAL =
"This typed data is for chain id 1, but the active network is Sepolia Testnet (chain id 11155111), so it cannot be signed.";
function withChainId(chainId) {
return { ...PERMIT, domain: { ...PERMIT.domain, chainId } };
}
beforeEach(() => {
decryptWithPassword.mockClear();
state.networkId = "sepolia";
});
test("another chain shows the error, with Sign disabled", async () => {
await openSignScreen(PERMIT);
expect(node("approve-sign-error").textContent).toBe(REFUSAL);
expect(node("btn-approve-sign").disabled).toBe(true);
});
test("Sign clicked anyway decrypts and signs nothing", async () => {
const sent = await openSignScreen(PERMIT);
node("approve-sign-password").value = "any password";
await click("btn-approve-sign");
expect(decryptWithPassword).not.toHaveBeenCalled();
expect(sent.map((msg) => msg.type)).not.toContain(
"AUTISTMASK_SIGN_RESPONSE",
);
expect(node("approve-sign-error").textContent).toBe(REFUSAL);
});
// ethers signs 1, "1" and "0x1" as the same chain id.
test("the chain id is refused in each form ethers signs", () => {
for (const chainId of [1, "1", "0x1"]) {
expect(typedDataRefusal(request(withChainId(chainId)))).toBe(
REFUSAL,
);
}
});
test("a chain id that is not a number is refused", () => {
expect(typedDataRefusal(request(withChainId("mainnet")))).toBe(
"This typed data cannot be read, so it cannot be signed.",
);
});
test("the active chain leaves Sign enabled", async () => {
await openSignScreen(withChainId("0xaa36a7"));
expect(node("approve-sign-error").textContent).toBe("");
expect(node("btn-approve-sign").disabled).toBe(false);
});
test("no chain id leaves Sign enabled", async () => {
await openSignScreen(withChainId(undefined));
expect(node("approve-sign-error").textContent).toBe("");
expect(node("btn-approve-sign").disabled).toBe(false);
});
});
+12
View File
@@ -153,6 +153,18 @@ describe("the explanatory copy", () => {
expect(all).not.toMatch(/\blost\b|\bstolen\b|\bgone\b/i);
});
// The wallet holds the imported key but will not export it, so the copy
// must name every key the user may hold that reaches the addresses
// outside it.
test("it names the keys that reach the addresses outside this wallet", () => {
const body = defect.paragraphs.join(" ");
expect(body).toMatch(/private key export are disabled/);
expect(body).toMatch(
/with the extended private key you imported, if you kept a copy of it, or with the master extended private key or recovery phrase it came from, in software that repeats the same non-standard derivation/,
);
});
test("it says the wallet is not deleted and what the user can do", () => {
const body = defect.paragraphs.join(" ");