Compare commits

..

5 Commits

Author SHA1 Message Date
0434163ce8 test: containerized Firefox end-to-end harness (closes #184)
All checks were successful
check / check (push) Successful in 1m18s
Drives the real popup in a real Firefox with dist/firefox/ installed as an
unpacked MV2 temporary add-on, via geckodriver. Covers popup load, wallet
creation through the UI, and the Add Token screen. Outside make check, like
the Chrome suite.

Zero npm dependencies: tests/e2e/firefox/driver.js is a WebDriver client
over global fetch and child_process against geckodriver's HTTP API. The
Dockerfile pins the node base image, the Firefox 153.0.3 tarball and
geckodriver 0.36.0 by digest.

Errors are read from the privileged nsIConsoleService in Marionette's chrome
context, filtered to non-warning entries whose sourceName is the extension
origin. BiDi log.entryAdded delivers nothing at all for extension pages, so
a Playwright-BiDi or Puppeteer-BiDi harness would see nothing and report
success; the code says so where someone would be tempted to simplify it.
Errors logged during add-on install and background startup are drained and
folded into step 1, never discarded: a throw at the top of
src/background/index.js kills the background page and fails the run.
Content-script capture is left as unverified, because --network none leaves
no http:// page for a content script to be injected into.

Each drain reads the console and clears it in ONE chrome script. Splitting
the read from Services.console.reset() left a window between the two round
trips in which an error was logged into a buffer about to be discarded, and
destroyed unread rather than deferred to the next drain; a probe of 100
sequenced throws at 20ms spacing lost one. With the drain atomic the same
probe accounts for every throw that falls inside the observed window, on two
consecutive runs.

No driver layer is shared with the Chrome suite and the three UI steps are
written twice deliberately: the two backends have no common substrate, and
three steps do not pay for a shim.

Two limits are documented rather than papered over. Error capture is
poll-based, so an error is attributed to a step and not to a moment within
it, and the observed window ends ~1.5s after the last step returns, measured:
errors at +0.5s, +1.0s and +1.5s are reported and +1.6s and later never are,
because the browser is torn down. Nothing is stubbed; the container runs with
--network none instead, which proves no request escaped, cannot report which
were attempted, and runs only the failure branches of network-dependent code.
2026-08-12 09:23:11 +00:00
0a1786b406 harden: verify all approval fields and make failed signing retryable (closes #174)
All checks were successful
check / check (push) Successful in 30s
approvalVerify now compares every field of the signed artifact against the
approval, not a subset. Transaction types are allowlisted to 0/1/2 and any
field the module does not check is refused outright, so a future transaction
type cannot smuggle consequential fields past verification -- an EIP-7702
type-4 artifact that delegates the signer's own EOA while matching every
displayed field was accepted before this change. The serialized bytes handed
to broadcastTransaction are compared against the parsed artifact, so the
guarantee covers the bytes that actually go to the node.

Signing failures in the popup are retryable again. To make that safe, an
approval is claimed synchronously before the first await and every path that
resolves or removes one goes through a single chokepoint that refuses a claimed
approval. Without it, closing the approval window, switching the active address
or a late reject would report "User rejected the request." to the dApp while
the broadcast completed -- the user then redoes the transfer at a fresh nonce
and it sends twice.

Failure copy distinguishes the stage reached, so a user is never told to start
again from the site when the first attempt may already have reached the network.
2026-08-12 11:21:02 +02:00
937f699fb1 feat: remove an address from an HD wallet, behind a confirmation (closes #162)
All checks were successful
check / check (push) Successful in 36s
Address rows on Home gain an [x] control, on wallets that derive addresses from
an extended key and hold more than one, opening a DeleteAddress confirmation
screen.

Removal cannot destroy anything: the key material stays. Derivation indices are
not renumbered, so the next "+" derives the next unused index rather than
resurrecting the removed one. The confirmation states the real route back --
delete the whole wallet in Settings, which asks for the password and destroys
the stored recovery phrase, then import it again -- and notes that the scan
which follows only finds addresses with on-chain activity. The copy varies by
wallet type, since an xprv wallet has no recovery phrase.

Removing an address that holds a balance is allowed, with a warning naming no
figure; the funds are at the address on-chain and stay there either way.
Selection and active address move only when the removed address was the one
selected, and site permissions are dropped for it alone.

The state transition shares its address comparison, permission cleanup and
active-changed broadcast with the wallet-level removal.
2026-08-12 11:16:29 +02:00
1f41a07df2 fix: filter a fake ETH token from the balance list too (closes #235)
All checks were successful
check / check (push) Successful in 30s
2026-08-12 11:10:38 +02:00
78a1cb067e test: commit a verify-build failure-mode battery and run it from make check (closes #227)
All checks were successful
check / check (push) Successful in 51s
2026-08-12 11:05:08 +02:00
25 changed files with 4019 additions and 230 deletions

129
README.md
View File

@@ -91,13 +91,21 @@ provide:
- `script/lint` — run the linter
- `script/fmt` — format all files (writes)
- `script/fmt-check` — check formatting (read-only)
- `script/check` — run test, lint, and fmt-check
- `script/check` — run test, test-verify-build, lint, and fmt-check
- `script/verify-build` — assert the compiled `DEBUG` state of the bundles in
`dist/`: every bundle containing `src/shared/constants.js` must have `DEBUG`
off, or on when `AUTISTMASK_DEBUG=1`. Run automatically at the end of
`make build` and `make build-debug`; fails loudly rather than passing if it
cannot determine a bundle's state. Not part of `make check`, which does not
depend on build artifacts existing.
- `script/test-verify-build` — exercise every failure mode of
`script/verify-build` against a fixture tree in a temp dir, asserting the exit
status and the message of each. Part of `make check`; it reads no build
artifacts and writes nothing under `dist/`. The cases that depend on file
permissions cannot mean anything for a process that is not subject to them, so
the harness proves its runner against a mode-000 file before counting them,
dropping to an unprivileged user when run as root; if it cannot, it skips
those cases and says so in a banner rather than passing them.
- `script/docker` — build the Docker image tagged via `script/projectname`
- `script/cibuild` — CI entrypoint: plain `docker build .`
- `script/precommit` — run by the git pre-commit hook; runs `script/check`
@@ -141,8 +149,11 @@ transfer, and the recovery phrase screen — which wallet types are offered it,
that it holds nothing before the password is accepted, that a wrong password
reveals nothing, that leaving it by either route wipes it — including a leave
taken while the decrypt is still running — and that reopening the popup does not
land on it. All outbound network is intercepted at the browser level and served
from fixtures in `tests/e2e/network.js`, so the run is deterministic and fully
land on it. It also covers address removal: which wallets offer the control at
all, that the confirmation states the route back rather than showing an empty
paragraph, that leaving the confirmation removes nothing, and that confirming it
does. All outbound network is intercepted at the browser level and served from
fixtures in `tests/e2e/network.js`, so the run is deterministic and fully
offline; unrecognised outbound requests are reported as failures rather than
silently allowed.
@@ -232,10 +243,15 @@ Two limits are worth knowing, both real differences from the Chrome suite:
- **Error capture is poll-based, not event-streamed.** The console is drained at
each step boundary, so an error is attributed to the step it was drained
after, not to a moment within it. The window that is drained runs from add-on
install to one second after the last step returns; an error logged later than
that ~1s tail is missed entirely, because the browser is torn down first.
Within that window nothing is dropped, but an error cannot be located within a
step the way the Chrome suite's `pageerror` events can.
install to **≈1.5s** after the last step returns, then the browser is torn
down; measured with throws scheduled at fixed offsets, errors at +0.5s, +1.0s
and +1.5s are reported and +1.6s and later never are. Within that window
nothing is dropped — each drain reads and clears the console in a single
chrome round trip, so an error logged mid-drain lands in that batch or the
next one rather than being destroyed unread; a probe of 100 throws at 20ms
spacing accounts for every one that falls inside the window, twice running.
What poll-based costs is location, not coverage: an error cannot be placed
within a step the way the Chrome suite's `pageerror` events place it.
- **Nothing is stubbed, which inverts the coverage of network-dependent code.**
There is no fixture layer; the container runs with `--network none` instead,
so the run is offline and deterministic and no request can escape. The
@@ -297,6 +313,7 @@ src/
prices.js — ETH/USD and token/USD via CoinDesk API
scamlist.js — known fraud contract addresses
state.js — persisted state (extension storage)
symbolSpoof.js — the known-symbol spoof rule, shared by all surfaces
tokenList.js — top ERC-20 tokens by market cap (hardcoded)
transactions.js — tx history fetching + anti-poisoning filters
uniswap.js — Uniswap Universal Router calldata decoder
@@ -526,11 +543,12 @@ Which tokens an address shows is decided by `fetchTokenBalances()` in
tokens do appear without the user adding them. An ERC-20 is shown when its
balance is nonzero and it is in the bundled known-token list, is tracked by the
user, or has 1,000 or more holders; a token claiming a symbol from the bundled
list from any other contract address is always dropped. That filter is
unconditional — the "Hide tokens with fewer than 1,000 holders" setting governs
the transaction history and the send-screen token selector, not this list.
Tracked tokens with a zero balance are listed as well while "Show tracked tokens
with zero balance" is on.
list from any other contract address is always dropped, and so is any token
claiming a symbol that belongs to the native asset and therefore has no
legitimate contract at all (`"ETH"`). That filter is unconditional — the "Hide
tokens with fewer than 1,000 holders" setting governs the transaction history
and the send-screen token selector, not this list. Tracked tokens with a zero
balance are listed as well while "Show tracked tokens with zero balance" is on.
#### Navigation
@@ -597,8 +615,9 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
- Wallet list: each wallet shows its name (tap to rename inline) and a "+"
button for HD and xprv wallets, then one block per address with "Address
N" (bold when active), the ENS name if resolved, the full address, an
`[info]` button, the address USD total, and a balance line for ETH and for
each token shown for that address
`[info]` button, an `[x]` button (only on HD and xprv wallets holding more
than one address), the address USD total, and a balance line for ETH and
for each token shown for that address
- "Recent Transactions": up to 25 transactions merged across every address
of every wallet, deduplicated by hash and filtered
- "Add additional wallet..." link at bottom
@@ -608,6 +627,7 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
- Tap wallet name → inline rename field (no screen change)
- "+" on wallet → derives the next address inline (no screen change)
- `[info]` on address → **AddressDetail**
- `[x]` on address → **DeleteAddress**
- "Send" → **Send** (refuses with a flash message on a zero balance)
- "Receive" → **Receive** (shows active address QR)
- Tap home tx row → **TransactionDetail**
@@ -986,6 +1006,55 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
nothing deleted
- "Back" → previous screen (Settings)
#### DeleteAddress (`delete-address-confirm`)
- **When**: User tapped the `[x]` next to an address on Home. Offered only on HD
and xprv wallets holding more than one address: the last address of a wallet
is never removable, and a key wallet has exactly one.
- **Elements**:
- "Back" button, "Remove Address" heading
- The address's own label ("Address N") and its wallet's name
- The full address (color dot, etherscan link, tap to copy), with the ENS
name above it if resolved
- Explanation that this only stops the wallet tracking the address: nothing
is destroyed, no key is deleted, and funds stay where they are
- The route back, stated with its limit, because the obvious two are both
refused: "+" derives the next unused index (`nextIndex` is a high-water
mark), and re-importing the wallet's key material is rejected as a
duplicate by `findWalletByXpub` while the wallet is still present. What
works is deleting the whole wallet in Settings — password-gated, and it
destroys the stored secret — then importing again, whereupon
`scanForAddresses()` rediscovers the address **only if it has on-chain
activity**. An address that was never used is not found by that scan. The
text is written by `recoveryPathText()` rather than sitting in
`index.html`, so it can name the wallet's own kind of key material: an
xprv wallet has no recovery phrase to re-import.
- A warning when the address holds anything, ETH or any tracked ERC-20,
followed by the holdings themselves via `balanceLinesForAddress()` and the
USD total via `getAddressValueUsd()`. The sentence names no figure of its
own: the lines round to four decimals, so a sentence built from a rounded
number would report `0.0000 ETH` for an address holding real money. The
predicate is `addressHoldsFunds()` in `src/popup/views/helpers.js`,
unrounded and token-aware. A balance is a warning, never a refusal.
- The rule that a wallet always keeps at least one address, and that
removing the last one means deleting the wallet from Settings
- Error line
- "Remove Address" button
- **Transitions**:
- "Remove Address" → removes the address and its site permissions, then →
previous screen (Home) with an "Address removed." flash message
- "Back" → previous screen (Home), nothing removed
- **Deliberately not password-gated**, unlike DeleteWallet: a password gates the
disclosure or destruction of a secret, and this does neither. The address
stays derivable from key material the wallet still holds.
- The active address moves only if it was the address removed, and then to the
wallet's first remaining address, with `AUTISTMASK_ACTIVE_CHANGED` broadcast
so a connected site stops being told about an address the user removed
(`src/shared/walletDelete.js`). A selection in any other wallet is left alone;
one in this wallet follows the splice.
- The wallet's derivation counter (`nextIndex`) is not rewound, so "+" derives a
fresh address rather than handing back the one just removed.
#### SettingsAddToken (`settings-addtoken`)
- **When**: User tapped "+ Add token" in Settings. Tokens added here are tracked
@@ -1348,14 +1417,15 @@ indexes it as a real token transfer.
that is the only thing that populates it. In the transaction history the check
is the "Hide fake tokens impersonating a known symbol" setting, on by default;
with it off, spoofed transfers are shown and no new blocklist entries are
learned from them. The send-screen token selector applies the same check
unconditionally, because it decides which tokens the user can act on rather
than what the history displays. The balance list applies it unconditionally
too, but not identically: it exempts symbols that `KNOWN_SYMBOLS` maps to
`null`, and `"ETH"` is the only one. So the fake "Ethereum" token above is
filtered from the transaction history and from the send selector, but a
fake-`ETH` ERC-20 that clears the balance list's own 1,000-holder floor — or
that the user tracked manually — is still shown in the balance list.
learned from them. The send-screen token selector and the balance list apply
the same check unconditionally, because they decide which tokens the user can
act on and what the user believes they own rather than what the history
displays. All three surfaces read the rule from `src/shared/symbolSpoof.js`,
so they cannot answer the question differently. A symbol the list maps to no
contract at all — `"ETH"`, the native asset, is the only one — may be borne by
no contract, so every ERC-20 claiming it is a spoof on all three. The user's
real ETH balance is not an ERC-20 and is read over RPC, so the rule never sees
it.
- **Low-holder token filtering**: Token transfers from ERC-20 contracts with
fewer than 1,000 holders are hidden from transaction history by default.
@@ -1391,13 +1461,12 @@ indexes it as a real token transfer.
a sharp tool — users who understand the risks can configure the wallet to show
everything unfiltered, unix-style. All four settings govern the transaction
history; what else each one reaches varies. The known-symbol check also runs
unconditionally on the send-screen token selector, and on the balance list
except for symbols mapped to `null` (`"ETH"` alone), which the balance list
does not filter. The fraud contract blocklist is applied unconditionally on
that selector and is not consulted by the balance list at all. The low-holder
setting also gates the send selector, while the balance list's own
1,000-holder floor is unconditional (see Data Model). The dust threshold
applies to the transaction history alone.
unconditionally on the send-screen token selector and on the balance list, in
both cases identically to the history. The fraud contract blocklist is applied
unconditionally on that selector and is not consulted by the balance list at
all. The low-holder setting also gates the send selector, while the balance
list's own 1,000-holder floor is unconditional (see Data Model). The dust
threshold applies to the transaction history alone.
#### Phishing Domain Protection
@@ -1469,7 +1538,7 @@ Currently supported:
### Wallet Management
- [x] Delete wallet (with confirmation)
- [ ] Delete address from HD wallet (with confirmation)
- [x] Delete address from HD wallet (with confirmation)
- [x] Show wallet's recovery phrase (requires password)
### Transactions

44
TODO.md
View File

@@ -51,12 +51,44 @@ undefined identifiers, which is how
client over `fetch` against geckodriver — with `node`, Firefox 153.0.3 and
geckodriver 0.36.0 all pinned by digest. Uncaught errors are read from the
privileged console service in Marionette's chrome context, because BiDi
`log.entryAdded` reports nothing at all for extension pages, and errors logged
during add-on install and background startup are folded into step 1 instead of
being cleared; demonstrated discriminating by exiting 1 on a `throw` at the
top of `src/background/index.js` and on a build with one import removed, and 0
on the branch as it stands
([#184](https://git.eeqj.de/sneak/AutistMask/issues/184)).
`log.entryAdded` reports nothing at all for extension pages; each drain reads
and clears the console in one chrome round trip, so nothing logged between two
drains is destroyed unread, and errors logged during add-on install and
background startup are folded into step 1 instead of being cleared.
Demonstrated discriminating by exiting 1 on a `throw` at the top of
`src/background/index.js`, on a build with one import removed, on a
`setTimeout` throw whose UI assertions all pass, on an unhandled
`Promise.reject` and on an undefined identifier in `home.js`, and 0 on the
branch as it stands ([#184](https://git.eeqj.de/sneak/AutistMask/issues/184)).
- 2026-08-12: Approval verification became an allowlist — transaction type
restricted to 0/1/2 so an EIP-7702 delegation can no longer ride along on an
approved transfer, every consequential field compared, the artifact
re-serialized from the checked fields alone and its exact bytes required to be
the canonical encoding of what was broadcast. One approval now yields at most
one broadcast, and every path that retires a pending approval — popup close,
active-address change, a late reject — goes through a single chokepoint that
refuses to settle an attempt already claimed for signing and broadcast
([#174](https://git.eeqj.de/sneak/AutistMask/issues/174)).
- 2026-08-12: An address can be removed from an HD or xprv wallet behind a
confirmation screen that states nothing is destroyed, sharing the deletion
state transitions with wallet deletion so the selection, site permissions and
active-address broadcast follow the same rules
([#162](https://git.eeqj.de/sneak/AutistMask/issues/162)).
- 2026-08-12: The known-symbol spoof rule moved into `src/shared/symbolSpoof.js`
and is now the only copy. The balance list had exempted symbols the token list
maps to `null``"ETH"` alone — so a fake ETH ERC-20 was hidden from the
transaction history and the Send selector but listed as a holding named ETH. A
symbol with no legitimate contract may now be borne by no contract on any of
the three surfaces, and the native exemption is "has no contract address", so
a second null-mapped symbol needs no call-site change. The user's real ETH
balance is read over RPC and never passes through the rule
([#235](https://git.eeqj.de/sneak/AutistMask/issues/235)).
- 2026-08-12: `script/verify-build`'s failure modes are now a committed target,
`script/test-verify-build`, run by `make check`. It asserts the exit status
and the message of every case against a fixture tree in a temp dir, and drops
privileges (proving the runner against a mode-000 file first) for the cases
that only mean something when file permissions are in force
([#227](https://git.eeqj.de/sneak/AutistMask/issues/227)).
- 2026-08-12: WaitTx lifecycle: a receipt and the 60-second timeout can no
longer both render on one tick, no timer or in-flight lookup outlives its
wait, a failed receipt lookup no longer counts as a timeout (but six in a row

View File

@@ -1,12 +1,13 @@
#!/bin/sh
# script/check: run all checks (test, lint, fmt-check). Our own
# extension to scripts-to-rule-them-all. Must not modify any files.
# script/check: run all checks (test, test-verify-build, lint, fmt-check).
# Our own extension to scripts-to-rule-them-all. Must not modify any files.
set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
main() {
"$SCRIPT_DIR/test"
"$SCRIPT_DIR/test-verify-build"
"$SCRIPT_DIR/lint"
"$SCRIPT_DIR/fmt-check"
}

444
script/test-verify-build Executable file
View File

@@ -0,0 +1,444 @@
#!/bin/sh
# script/test-verify-build: exercise every failure mode of
# script/verify-build. Our own extension to scripts-to-rule-them-all, run
# from script/check so make check covers it.
#
# Why this exists: verify-build is the build-integrity guard, and three
# separate reviews of it each found a fresh vacuous pass — the grep exit-2
# conflation, the discarded find status, the line-delimited walk. Every one
# was caught by someone building a tree by hand, because nothing in make check
# could catch it. This is that hand battery, committed and automated.
#
# Each case asserts the exit status AND a substring of the message. A guard
# that fails for the wrong reason (right status, different fault) is itself a
# defect, so matching the status alone would not be a test of anything.
#
# The fixture is a temp tree containing script/verify-build as a SYMLINK to
# the real script: verify-build takes its ROOT from dirname "$0"/.., so it
# operates on the fixture's dist/ and never reads or writes the repo's build
# output. The symlink rather than a copy is what makes a deliberate break in
# the real script fail here.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
VERIFY_BUILD="$ROOT/script/verify-build"
MARKER_ON="autistmask-build-debug=on"
MARKER_OFF="autistmask-build-debug=off"
NEWLINE='
'
PASSED=0
FAILED=0
SKIPPED=0
SKIPPED_NAMES=""
# The command prefix that runs the permission-dependent cases as a user who
# is actually subject to file permissions, and whether those cases can run at
# all. Both are decided by probe_permission_runner, never assumed.
UNPRIV=""
PERM_ENABLED=no
PERM_HOW=""
WORK=""
cleanup() {
[ -n "$WORK" ] || return 0
# The cases chmod 000 files and directories on purpose.
chmod -R u+rwX "$WORK" 2>/dev/null || true
rm -rf "$WORK"
}
trap cleanup EXIT INT TERM
WORK="$(mktemp -d "${TMPDIR:-/tmp}/autistmask-test-verify-build.XXXXXX")"
FIXTURE="$WORK/fixture"
# verify-build mktemps its dist/ listing under TMPDIR. Pointing that inside
# our work dir keeps the run leaving no residue, and keeps it writable for the
# unprivileged user the permission cases run as.
TMPDIR="$WORK/tmp"
export TMPDIR
mkdir -p "$TMPDIR"
chmod 1777 "$TMPDIR"
chmod 755 "$WORK"
# --- fixture ---------------------------------------------------------------
# A stand-in for an emitted bundle: some text plus one marker literal, which
# is all verify-build reads out of the real thing.
write_bundle() {
printf 'var a=1;/* %s */\nvar b=2;\n' "$2" >"$1"
}
# A dist/ shaped like a real build: two listed bundles under different
# browsers, an unlisted subtree to make unwalkable, and unlisted files that
# carry no marker and must not be objected to.
build_fixture() {
chmod -R u+rwX "$FIXTURE" 2>/dev/null || true
rm -rf "$FIXTURE"
mkdir -p "$FIXTURE/script"
ln -s "$VERIFY_BUILD" "$FIXTURE/script/verify-build"
mkdir -p "$FIXTURE/dist/chrome/src/popup" \
"$FIXTURE/dist/chrome/src/content" \
"$FIXTURE/dist/firefox/src/popup"
write_bundle "$FIXTURE/dist/chrome/src/popup/index.js" "$MARKER_OFF"
write_bundle "$FIXTURE/dist/firefox/src/popup/index.js" "$MARKER_OFF"
printf 'body{color:#000}\n' >"$FIXTURE/dist/styles.css"
printf 'var c=3;\n' >"$FIXTURE/dist/chrome/src/content/content.js"
{
echo "dist/chrome/src/popup/index.js"
echo "dist/firefox/src/popup/index.js"
} >"$FIXTURE/dist/constants-bundles.txt"
# Readable and traversable by the unprivileged user the permission cases
# run as, before those cases take that away again on purpose.
chmod -R a+rX "$FIXTURE"
}
# --- permission runner ------------------------------------------------------
# Run a command through the current unprivileged runner. Unquoted on purpose:
# UNPRIV is a command prefix that has to word-split.
run_unpriv() {
# shellcheck disable=SC2086
$UNPRIV "$@"
}
# Decide whether the permission-dependent cases can run, and prove it rather
# than assuming it.
#
# The problem: the CI image declares no USER, so CI runs as root, and root is
# not subject to file permissions — chmod 000 stops neither find nor grep. A
# permission case run as root passes vacuously, which is worse than no case at
# all because it reads as coverage.
#
# So the runner is validated with two probes before any permission case is
# counted:
#
# - a mode-644 file MUST be readable through it. If not, the runner itself
# is broken (missing helper, no such user, sandbox), and every case run
# through it would fail for the wrong reason.
# - a mode-000 file MUST NOT be readable through it. If it is, permissions
# are not in force and the cases would pass without proving anything.
#
# Unprivileged: the runner is empty and both probes are about this process,
# which is the honest answer. Root: setpriv and runuser are tried, both
# present in the pinned CI base image. Only when no candidate passes both
# probes are the cases skipped, and a skipped run says so unmistakably.
probe_permission_runner() {
_probe="$WORK/probe"
mkdir -p "$_probe"
printf 'readable\n' >"$_probe/public"
printf 'secret\n' >"$_probe/private"
chmod 755 "$_probe"
chmod 644 "$_probe/public"
chmod 000 "$_probe/private"
if [ "$(id -u)" -eq 0 ]; then
_candidates="setpriv|setpriv --reuid=65534 --regid=65534 --clear-groups --
runuser|runuser -u nobody --"
else
_candidates="direct|"
fi
_tried=""
_saved_ifs="$IFS"
IFS="$NEWLINE"
for _line in $_candidates; do
IFS="$_saved_ifs"
_label="${_line%%|*}"
_cmd="${_line#*|}"
_tried="${_tried:+$_tried, }$_label"
if [ -n "$_cmd" ]; then
_bin="${_cmd%% *}"
command -v "$_bin" >/dev/null 2>&1 || continue
fi
UNPRIV="$_cmd"
# Broken or unusable runner: the cases would fail for the wrong
# reason. Reaching the script under test is part of usable.
run_unpriv cat "$_probe/public" >/dev/null 2>&1 || continue
run_unpriv cat "$VERIFY_BUILD" >/dev/null 2>&1 || continue
# Permissions not in force through this runner: the cases would pass
# without testing anything.
if run_unpriv cat "$_probe/private" >/dev/null 2>&1; then
continue
fi
PERM_ENABLED=yes
PERM_HOW="$_label"
IFS="$_saved_ifs"
return 0
done
IFS="$_saved_ifs"
UNPRIV=""
PERM_ENABLED=no
PERM_HOW="$_tried"
}
# --- case runner ------------------------------------------------------------
# check_case <name> <perm:yes|no> <mode:release|debug> <status> <text> <setup>
#
# Rebuilds the fixture, applies <setup> inside it, runs verify-build, and
# requires both the exit status and the message. <perm> marks a case that only
# means anything when file permissions are in force.
check_case() {
_name="$1"
_perm="$2"
_mode="$3"
_want_status="$4"
_want_text="$5"
_setup="$6"
if [ "$_perm" = yes ] && [ "$PERM_ENABLED" != yes ]; then
SKIPPED=$((SKIPPED + 1))
SKIPPED_NAMES="$SKIPPED_NAMES## - $_name$NEWLINE"
echo " SKIP (permissions not in force): $_name"
return 0
fi
build_fixture
if ! (cd "$FIXTURE" && "$_setup") >/dev/null 2>&1; then
FAILED=$((FAILED + 1))
echo " FAIL: $_name"
echo " the case's own setup failed, so nothing was tested."
return 0
fi
if [ "$_mode" = debug ]; then
_debug=1
else
_debug=""
fi
# Exported rather than set as a command prefix: run_unpriv is a function,
# and an assignment prefixed to a function call is not portable.
AUTISTMASK_DEBUG="$_debug"
export AUTISTMASK_DEBUG
_status=0
if [ "$_perm" = yes ]; then
_out="$(run_unpriv "$FIXTURE/script/verify-build" 2>&1)" || _status=$?
else
_out="$("$FIXTURE/script/verify-build" 2>&1)" || _status=$?
fi
_ok=yes
_why=""
if [ "$_status" -ne "$_want_status" ]; then
_ok=no
_why="exit status $_status, wanted $_want_status"
fi
# Same discipline verify-build itself applies to grep: 0 and 1 are
# answers, anything else is not, and must not be read as "no match".
_g=0
printf '%s\n' "$_out" | grep -q -F -e "$_want_text" || _g=$?
case "$_g" in
0) ;;
1)
_ok=no
_why="${_why:+$_why; }message did not contain: $_want_text"
;;
*)
_ok=no
_why="${_why:+$_why; }grep exited $_g matching the message, so the
message was never checked"
;;
esac
if [ "$_ok" = yes ]; then
PASSED=$((PASSED + 1))
echo " ok: $_name"
return 0
fi
FAILED=$((FAILED + 1))
echo " FAIL: $_name"
echo " $_why"
echo " --- verify-build output ---"
printf '%s\n' "$_out" | sed 's/^/ /'
echo " --- end output ---"
}
# --- cases ------------------------------------------------------------------
#
# Each runs with the fixture as its working directory.
c_control() { :; }
c_trailing_space() {
cp dist/chrome/src/popup/index.js "dist/chrome/src/popup/index.js "
}
c_embedded_newline() {
cp dist/chrome/src/popup/index.js "dist/chrome/src/popup/index.js$NEWLINE"
}
c_dist_symlink() {
mv dist dist.real
ln -s dist.real dist
}
c_unwalkable_subtree() { chmod 000 dist/chrome/src/content; }
c_dangling_symlink() {
ln -s /nonexistent-target-for-test-verify-build dist/chrome/dangling.js
}
c_dir_symlink() { ln -s src dist/chrome/link-to-dir; }
c_alias_symlink() { ln -s popup/index.js dist/chrome/src/aliased.js; }
c_manifest_missing() { rm dist/constants-bundles.txt; }
c_manifest_empty() { : >dist/constants-bundles.txt; }
c_manifest_unreadable() { chmod 000 dist/constants-bundles.txt; }
c_bundle_missing() { rm dist/chrome/src/popup/index.js; }
c_bundle_empty() { : >dist/chrome/src/popup/index.js; }
c_bundle_unreadable() { chmod 000 dist/chrome/src/popup/index.js; }
c_unlisted_extension() {
cp dist/chrome/src/popup/index.js dist/chrome/src/popup/extra.mjs
}
c_no_marker() { printf 'var d=4;\n' >dist/chrome/src/popup/index.js; }
c_both_markers() {
printf '/* %s */\n' "$MARKER_ON" >>dist/chrome/src/popup/index.js
}
run_cases() {
check_case "control: untouched dist passes" \
no release 0 "2 bundle(s) verified $MARKER_OFF" c_control
check_case "unlisted marker-carrying file, trailing space in name" \
no release 1 "carries a debug marker but is absent from" \
c_trailing_space
check_case "unlisted marker-carrying file, newline in name" \
no release 1 "carries a debug marker but is absent from" \
c_embedded_newline
check_case "dist/ replaced by a symlink" \
no release 1 "dist is a symlink, not a directory." c_dist_symlink
check_case "unwalkable subtree under dist/" \
yes release 1 "enumerating dist/, so part of the tree" \
c_unwalkable_subtree
check_case "dangling symlink under dist/" \
no release 1 \
"reading dist/chrome/dangling.js, so the file could not be" \
c_dangling_symlink
check_case "symlink to a directory under dist/" \
no release 1 \
"reading dist/chrome/link-to-dir, so the file could not be" \
c_dir_symlink
check_case "symlink to a listed bundle under an unlisted path" \
no release 1 \
"dist/chrome/src/aliased.js carries a debug marker but is absent" \
c_alias_symlink
check_case "manifest missing" \
no release 1 "dist/constants-bundles.txt is missing." \
c_manifest_missing
check_case "manifest empty" \
no release 1 "is empty, so no emitted bundle was found to contain" \
c_manifest_empty
check_case "manifest unreadable" \
yes release 1 "is not readable, so nothing was inspected." \
c_manifest_unreadable
check_case "listed bundle missing" \
no release 1 \
"lists dist/chrome/src/popup/index.js, which does not exist." \
c_bundle_missing
check_case "listed bundle empty" \
no release 1 "which is empty. An empty bundle" c_bundle_empty
check_case "listed bundle unreadable" \
yes release 1 \
"reading dist/chrome/src/popup/index.js, so the file could not be" \
c_bundle_unreadable
check_case "unlisted extension carrying a marker" \
no release 1 \
"dist/chrome/src/popup/extra.mjs carries a debug marker but is" \
c_unlisted_extension
check_case "listed bundle carries no marker" \
no release 1 "carries no debug marker, so its DEBUG state cannot be" \
c_no_marker
check_case "listed bundle carries both markers" \
no release 1 "carries both debug markers, so DEBUG was not resolved" \
c_both_markers
check_case "wrong marker for the requested mode" \
no debug 1 "is $MARKER_OFF but this build expects $MARKER_ON" \
c_control
}
# --- main --------------------------------------------------------------------
main() {
cd "$ROOT"
[ -x "$VERIFY_BUILD" ] || {
echo "test-verify-build: $VERIFY_BUILD is missing or not executable" >&2
exit 1
}
echo "Testing script/verify-build failure modes..."
probe_permission_runner
if [ "$PERM_ENABLED" = yes ]; then
echo " permission cases: enabled (runner: $PERM_HOW, proved against" \
"a mode-000 file)"
fi
run_cases
if [ "$FAILED" -ne 0 ]; then
echo "test-verify-build: $FAILED case(s) FAILED," \
"$PASSED passed, $SKIPPED skipped" >&2
exit 1
fi
if [ "$SKIPPED" -ne 0 ]; then
cat <<EOF
################################################################################
## WARNING: $SKIPPED PERMISSION CASE(S) DID NOT RUN, AND THIS RUN DOES NOT
## PROVE THEM. This process is uid $(id -u), and no runner subject to file
## permissions was available. Tried: $PERM_HOW.
## Under root, chmod 000 stops neither find nor grep, so these cases would
## have passed without testing anything. They were skipped, not counted:
$SKIPPED_NAMES################################################################################
EOF
echo "test-verify-build: $PASSED case(s) passed," \
"$SKIPPED SKIPPED AND NOT PROVEN (see the warning above)"
return 0
fi
echo "test-verify-build: $PASSED case(s) passed"
}
main "$@"

View File

@@ -13,7 +13,16 @@ const {
} = require("../shared/state");
const { refreshBalances, getProvider } = require("../shared/balances");
const { debugFetch, log } = require("../shared/log");
const { verifySignedTx, verifySignature } = require("../shared/approvalVerify");
const {
verifySignedTx,
verifySignature,
failureIsRetryable,
describeTxFailure,
TX_STAGE_SIGN,
TX_STAGE_VERIFY,
TX_STAGE_BROADCAST,
TX_STAGE_INFLIGHT,
} = require("../shared/approvalVerify");
const {
isPhishingDomain,
refreshPhishingListOnSchedule,
@@ -107,6 +116,55 @@ function resetPopupUrl() {
}
}
// Settle a pending approval: hand `result` to the promise the requesting page
// is waiting on and retire the approval. This is the ONLY place an approval is
// resolved or removed — the popup closing, an active-address switch, a reject
// from the popup and the attempt that signs and broadcasts all come through
// here — because a settlement that bypasses the claim below is a fund-loss bug
// and enumerating the call sites has repeatedly missed one.
//
// A claimed approval belongs to the attempt holding the claim, and only that
// attempt may settle it. Anything else settling first would leave the attempt
// running to completion against an already-settled promise: the transaction
// reaches the chain while the page is told "User rejected the request", and the
// user's natural response is to send it again at a fresh nonce.
//
// Returns false when the approval is gone or claimed by someone else, so the
// caller can refuse instead of assuming it settled.
function settleApproval(id, result, options) {
const approval = pendingApprovals[id];
if (!approval) return false;
const holdsClaim = !!(options && options.holdsClaim);
if (approval.attemptInFlight && !holdsClaim) return false;
delete pendingApprovals[id];
approval.resolve(result);
resetPopupUrl();
return true;
}
// Take exclusive hold of a pending approval for one attempt, or refuse.
//
// An approval that failed retryably has to stay in pendingApprovals, so its
// presence cannot be the interlock against a second attempt; this flag is. It
// is set synchronously, before the handler's first await, so a second response
// carrying the same id — a reloaded approval window re-rendering a live
// Approve button, a popup that emits the message twice — finds the attempt
// already running instead of starting an independent verify and broadcast.
// Without it one approval can put two transactions on the chain: with the
// ordinary dApp approval shape the page fixes no nonce, so two artifacts
// signed at different nonces both verify.
function claimApproval(approval) {
if (approval.attemptInFlight) return false;
approval.attemptInFlight = true;
return true;
}
// Release an approval whose attempt failed in a way the user can retry.
// Nothing was broadcast, so the next attempt may claim it.
function releaseApproval(approval) {
approval.attemptInFlight = false;
}
// Open approval in a separate popup window.
// This is the primary mechanism for tx/sign approvals (triggered programmatically,
// not from a user gesture) and the fallback for site-connection approvals.
@@ -215,8 +273,7 @@ runtime.onConnect.addListener((port) => {
// Keep pending — user can reopen the toolbar popup
return;
}
approval.resolve({ approved: false, remember: false });
delete pendingApprovals[id];
settleApproval(id, { approved: false, remember: false });
}
resetPopupUrl();
});
@@ -547,15 +604,21 @@ async function broadcastAccountsChanged() {
for (const key of Object.keys(connectedSites)) {
delete connectedSites[key];
}
// Reject and close any pending approval popups so they don't hang
// Reject and close any pending approval popups so they don't hang. An
// approval an attempt has already claimed is left alone entirely: it is
// being signed and broadcast right now, and neither rejecting it to the
// page nor closing the window it is reporting into is survivable.
for (const [id, approval] of Object.entries(pendingApprovals)) {
if (approval.type === "tx" || approval.type === "sign") {
approval.resolve({
error: { code: 4001, message: "User rejected the request." },
});
} else {
approval.resolve({ approved: false, remember: false });
const rejection =
approval.type === "tx" || approval.type === "sign"
? {
error: {
code: 4001,
message: "User rejected the request.",
},
}
: { approved: false, remember: false };
if (!settleApproval(id, rejection)) continue;
if (approval.windowId) {
windowsApi.remove(approval.windowId, () => {
if (runtime.lastError) {
@@ -563,7 +626,6 @@ async function broadcastAccountsChanged() {
}
});
}
delete pendingApprovals[id];
}
resetPopupUrl();
const s = await getState();
@@ -679,23 +741,26 @@ if (runtime.onStartup) {
}
startBackgroundJobs();
// When approval window is closed without a response, treat as rejection
// When approval window is closed without a response, treat as rejection.
// "Without a response" is the operative part: the popup stays open across the
// verify and broadcast it is waiting on, so a user closing an apparently-hung
// window is an ordinary event with an attempt already in flight behind it.
// settleApproval() refuses those, which leaves the attempt to report its real
// outcome to the page.
if (windowsApi && windowsApi.onRemoved) {
windowsApi.onRemoved.addListener((windowId) => {
for (const [id, approval] of Object.entries(pendingApprovals)) {
if (approval.windowId === windowId) {
if (approval.type === "tx" || approval.type === "sign") {
approval.resolve({
if (approval.windowId !== windowId) continue;
const rejection =
approval.type === "tx" || approval.type === "sign"
? {
error: {
code: 4001,
message: "User rejected the request.",
},
});
} else {
approval.resolve({ approved: false, remember: false });
}
delete pendingApprovals[id];
}
: { approved: false, remember: false };
settleApproval(id, rejection);
}
});
}
@@ -761,14 +826,10 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
}
if (msg.type === "AUTISTMASK_APPROVAL_RESPONSE") {
const approval = pendingApprovals[msg.id];
if (approval) {
approval.resolve({
settleApproval(msg.id, {
approved: msg.approved,
remember: msg.remember,
});
delete pendingApprovals[msg.id];
}
resetPopupUrl();
return false;
}
@@ -776,21 +837,50 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
if (msg.type === "AUTISTMASK_TX_RESPONSE") {
const approval = pendingApprovals[msg.id];
if (!approval) return false;
delete pendingApprovals[msg.id];
resetPopupUrl();
// A reject arriving while an attempt holds the approval is refused,
// not honoured: the attempt is on its way to broadcasting the
// transaction, and resolving 4001 here would tell the page the request
// was rejected while it goes out.
if (!msg.approved) {
approval.resolve({
error: { code: 4001, message: "User rejected the request." },
if (
!settleApproval(msg.id, {
error: {
code: 4001,
message: "User rejected the request.",
},
})
) {
sendResponse({
error: "This transaction is already being sent.",
retryable: false,
stage: TX_STAGE_BROADCAST,
});
return false;
}
return true;
}
// The popup signs; it reports back here when it could not. Fail the
// request the same way this handler used to when it did the signing.
// The popup signs; it reports back here when it could not. Keep the
// approval so the user can correct the problem and try again with the
// transaction they already saw.
if (msg.error) {
approval.resolve({ error: { message: msg.error } });
sendResponse({ error: msg.error });
const outcome = describeTxFailure(TX_STAGE_SIGN, msg.error);
sendResponse({
error: outcome.error,
retryable: outcome.retryable,
stage: TX_STAGE_SIGN,
});
return false;
}
// Exactly one broadcast per approval, whatever the popup sends.
if (!claimApproval(approval)) {
sendResponse({
error: "This transaction is already being sent.",
retryable: false,
stage: TX_STAGE_BROADCAST,
});
return false;
}
@@ -800,22 +890,63 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
const activeAddress = await getActiveAddress();
// The popup holds the secret, but the background stays the
// authority on what is broadcast: the raw transaction must be
// the approved one, signed by the approved address.
// the approved one, signed by the approved address, on the
// network that is selected.
verifySignedTx(
msg.rawSignedTx,
approval.txParams,
activeAddress,
currentNetwork().chainId,
);
} catch (e) {
// A signed transaction that is not the approved one is not
// retried against that approval; it is refused outright.
// Anything else that failed before the check ran is the
// user's to retry.
const outcome = describeTxFailure(TX_STAGE_VERIFY, e);
if (outcome.spendApproval) {
settleApproval(
msg.id,
{ error: { message: outcome.error } },
{ holdsClaim: true },
);
} else {
releaseApproval(approval);
}
sendResponse({
error: outcome.error,
retryable: outcome.retryable,
stage: TX_STAGE_VERIFY,
});
return;
}
try {
const provider = getProvider(state.rpcUrl);
const tx = await provider.broadcastTransaction(msg.rawSignedTx);
approval.resolve({ txHash: tx.hash });
settleApproval(
msg.id,
{ txHash: tx.hash },
{ holdsClaim: true },
);
sendResponse({ txHash: tx.hash });
} catch (e) {
const errMsg = e.shortMessage || e.message;
approval.resolve({
error: { message: errMsg },
// 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.
const outcome = describeTxFailure(TX_STAGE_BROADCAST, e);
settleApproval(
msg.id,
{ error: { message: outcome.error } },
{ holdsClaim: true },
);
sendResponse({
error: outcome.error,
retryable: outcome.retryable,
stage: TX_STAGE_BROADCAST,
});
sendResponse({ error: errMsg });
}
})();
return true;
@@ -824,21 +955,43 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
if (msg.type === "AUTISTMASK_SIGN_RESPONSE") {
const approval = pendingApprovals[msg.id];
if (!approval) return false;
delete pendingApprovals[msg.id];
resetPopupUrl();
// Same as the transaction path: a reject cannot retire an approval an
// attempt already holds.
if (!msg.approved) {
approval.resolve({
error: { code: 4001, message: "User rejected the request." },
if (
!settleApproval(msg.id, {
error: {
code: 4001,
message: "User rejected the request.",
},
})
) {
sendResponse({
error: "This request is already being signed.",
retryable: false,
stage: TX_STAGE_INFLIGHT,
});
return false;
}
return true;
}
// The popup signs; it reports back here when it could not. Fail the
// request the same way this handler used to when it did the signing.
// The popup signs; it reports back here when it could not. Keep the
// approval so the user can correct the problem and try again with the
// message they already saw.
if (msg.error) {
approval.resolve({ error: { message: msg.error } });
sendResponse({ error: msg.error });
sendResponse({ error: msg.error, retryable: true });
return false;
}
// Exactly one signature handed back per approval.
if (!claimApproval(approval)) {
sendResponse({
error: "This request is already being signed.",
retryable: false,
stage: TX_STAGE_INFLIGHT,
});
return false;
}
@@ -851,14 +1004,21 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
// address.
const signature = msg.signature;
verifySignature(approval.signParams, signature, activeAddress);
approval.resolve({ signature });
settleApproval(msg.id, { signature }, { holdsClaim: true });
sendResponse({ signature });
} catch (e) {
const errMsg = e.shortMessage || e.message;
approval.resolve({
error: { message: errMsg },
});
sendResponse({ error: errMsg });
const retryable = failureIsRetryable(e);
if (!retryable) {
settleApproval(
msg.id,
{ error: { message: errMsg } },
{ holdsClaim: true },
);
} else {
releaseApproval(approval);
}
sendResponse({ error: errMsg, retryable });
}
})();
return true;

View File

@@ -1142,6 +1142,62 @@
</button>
</div>
<!-- ============ DELETE ADDRESS CONFIRM ============ -->
<div id="view-delete-address-confirm" class="view hidden">
<button
id="btn-delete-address-back"
class="border border-border px-2 py-1 hover:bg-fg hover:text-bg cursor-pointer mb-2"
>
&lt; Back
</button>
<h2 class="font-bold mb-3">Remove Address</h2>
<p class="text-xs mb-2">
You are about to remove
<strong id="delete-address-label"></strong> from
<strong id="delete-address-wallet-name"></strong>.
</p>
<div
id="delete-address-value"
class="text-xs mb-2 break-all min-h-[1rem]"
></div>
<div
class="text-xs mb-2 border border-border border-dashed p-2"
>
This only stops this wallet from tracking the address.
Nothing is destroyed and no key is deleted. Any funds at the
address stay exactly where they are, and the address remains
yours. Any site permissions granted to this address are
forgotten.
</div>
<!-- Filled by src/popup/views/deleteAddress.js: the route
back names the wallet's own kind of key material. -->
<div
id="delete-address-recovery"
class="text-xs mb-2 border border-border border-dashed p-2"
></div>
<div
id="delete-address-balance"
class="text-xs mb-2 min-h-[1.25rem] pointer-events-none"
>
&nbsp;
</div>
<p class="text-xs text-muted mb-3">
A wallet always keeps at least one address. To remove the
last one, delete the whole wallet from Settings instead.
</p>
<div
id="delete-address-flash"
class="text-xs text-red-500 mb-2 min-h-[1.25rem]"
style="visibility: hidden"
></div>
<button
id="btn-delete-address-confirm"
class="border border-border text-red-500 px-2 py-1 hover:bg-fg hover:text-bg cursor-pointer"
>
Remove Address
</button>
</div>
<!-- ============ SHOW RECOVERY PHRASE ============ -->
<div id="view-show-phrase" class="view hidden">
<button

View File

@@ -33,6 +33,7 @@ const receive = require("./views/receive");
const addToken = require("./views/addToken");
const settings = require("./views/settings");
const settingsAddToken = require("./views/settingsAddToken");
const deleteAddress = require("./views/deleteAddress");
const approval = require("./views/approval");
function renderWalletList() {
@@ -101,6 +102,10 @@ const ctx = {
pushCurrentView();
settingsAddToken.show();
},
showDeleteAddress: (walletIdx, addrIdx) => {
pushCurrentView();
deleteAddress.show(walletIdx, addrIdx);
},
};
function needsAddress(view) {
@@ -256,6 +261,7 @@ async function init() {
addToken.init(ctx);
settings.init(ctx);
settingsAddToken.init(ctx);
deleteAddress.init(ctx);
if (!state.hasWallet) {
showView("welcome");

View File

@@ -24,6 +24,7 @@ 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");
const runtime =
@@ -589,10 +590,20 @@ function init(ctx) {
runtime.sendMessage(payload, (response) => {
if (response && response.txHash) {
txStatus.showWait(pendingTxDetails, response.txHash);
return;
}
// A retryable failure leaves the approval pending in the
// background, so stay on this screen with a live button rather
// than sending the user to a dead end.
const outcome = describeSigningFailure(
response,
"The transaction could not be sent.",
);
if (outcome.retryable) {
showError("approve-tx-error", outcome.message);
setTxButtonBusy(false);
} else {
const msg =
(response && response.error) || "Transaction failed.";
txStatus.showError(pendingTxDetails, null, msg);
txStatus.showError(pendingTxDetails, null, outcome.message);
}
});
});
@@ -695,11 +706,18 @@ function init(ctx) {
runtime.sendMessage(payload, (response) => {
if (response && response.signature) {
window.close();
} else {
const msg = (response && response.error) || "Signing failed.";
showError("approve-sign-error", msg);
setSignButtonBusy(false);
return;
}
// The button comes back only when the approval is still pending in
// the background; otherwise it stays disabled and the message says
// why, because a control that cannot succeed must not look like it
// can.
const outcome = describeSigningFailure(
response,
"The message could not be signed.",
);
showError("approve-sign-error", outcome.message);
if (outcome.retryable) setSignButtonBusy(false);
});
});

View File

@@ -0,0 +1,176 @@
// Confirmation screen for removing one address from a wallet that derives
// its addresses from an extended key.
//
// No password is asked for, unlike delete-wallet. A password gates the
// disclosure or destruction of a secret, and this does neither: the address
// is derived from key material the wallet still holds, so removing it only
// stops the wallet tracking it. An explicit confirmation screen is the
// proportionate treatment.
const {
$,
showView,
showFlash,
goBack,
renderAddressHtml,
attachCopyHandlers,
addressHoldsFunds,
balanceLinesForAddress,
} = require("./helpers");
const { formatUsd, getAddressValueUsd } = require("../../shared/prices");
const { walletHasRecoveryPhrase } = require("../../shared/wallet");
const { state, saveState } = require("../../shared/state");
const {
canRemoveAddress,
removeAddressFromState,
broadcastActiveChanged,
} = require("../../shared/walletDelete");
// The wallet and address indices this screen is confirming, or null when it
// is not confirming anything.
let target = null;
let ctx = null;
function setFlash(msg) {
const el = $("delete-address-flash");
el.textContent = msg;
el.style.visibility = msg ? "visible" : "hidden";
}
// What it actually takes to get the address back, which is not what the
// screen used to claim.
//
// Neither obvious route works: "+" derives the next unused index, because
// wallet.nextIndex is a high-water mark and is deliberately not rewound; and
// re-importing this wallet's key material is refused as a duplicate by
// findWalletByXpub() for as long as the wallet is here. What remains is to
// delete the whole wallet in Settings — which asks for the password and
// destroys the stored secret — and import again, after which
// scanForAddresses() rediscovers the address only if it has on-chain
// activity. An address that was never used is not found by that scan, and
// the copy must not imply otherwise.
//
// The noun follows the wallet: an xprv wallet holds no recovery phrase, and
// this screen is offered on xprv wallets too.
function recoveryPathText(wallet) {
const secret = walletHasRecoveryPhrase(wallet)
? "recovery phrase"
: "extended private key";
return (
"Getting the address back into this list is not easy, so be sure. " +
"Adding an address derives the next unused one, not this one, and " +
"importing this " +
secret +
" again is refused while this wallet is still here. The way back is " +
"to delete the whole wallet in Settings, which asks for your " +
"password and destroys the stored " +
secret +
", and then import that " +
secret +
" again. The scan that follows only finds addresses that have " +
"on-chain activity, so an address that has never been used is not " +
"found by it."
);
}
// The balance warning, or a blank line when the address holds nothing.
//
// A balance is a reason to be careful, not a reason to refuse: the funds are
// at the address, not in this list, and stay there either way.
//
// "Holds" means ETH or any ERC-20 the wallet knows about — an address with no
// ETH and a five-figure stablecoin position must not get the blank line on
// the one screen whose job is to warn. The sentence names no figure of its
// own: the rendered lines round to four decimals, so a sentence built from a
// rounded number would report "0.0000 ETH" for an address holding real money.
// The lines below it carry the amounts, in the same format as Home and
// AddressDetail, followed by the USD total when prices are known (null on
// testnet and before the first price fetch, where the line is left off rather
// than printed as $0.00).
function balanceWarningHtml(addr) {
if (!addressHoldsFunds(addr)) return "&nbsp;";
const usd = getAddressValueUsd(addr);
const total =
usd === null
? ""
: `<div class="text-xs text-muted mt-1">Total: ${formatUsd(usd)}</div>`;
return (
`<p class="mb-1">This address holds a balance. Removing it does not ` +
`move or spend anything; the balance stays at the address.</p>` +
balanceLinesForAddress(addr, state.trackedTokens, false) +
total
);
}
function show(walletIdx, addrIdx) {
const wallet = state.wallets[walletIdx];
const addr = wallet && wallet.addresses[addrIdx];
if (!addr) return;
target = { walletIdx, addrIdx };
$("delete-address-label").textContent = "Address " + (addrIdx + 1);
$("delete-address-wallet-name").textContent =
wallet.name || "Wallet " + (walletIdx + 1);
const value = $("delete-address-value");
value.innerHTML = renderAddressHtml(addr.address, {
ensName: addr.ensName,
});
attachCopyHandlers(value);
$("delete-address-recovery").textContent = recoveryPathText(wallet);
$("delete-address-balance").innerHTML = balanceWarningHtml(addr);
setFlash("");
showView("delete-address-confirm");
}
function init(_ctx) {
ctx = _ctx;
$("btn-delete-address-back").addEventListener("click", () => {
target = null;
goBack();
});
$("btn-delete-address-confirm").addEventListener("click", async () => {
if (target === null) {
setFlash("No address is selected for removal.");
return;
}
const { walletIdx, addrIdx } = target;
if (!canRemoveAddress(state.wallets[walletIdx])) {
setFlash(
"This address cannot be removed, because a wallet always " +
"keeps at least one address.",
);
return;
}
const { removed, activeAddressChanged } = removeAddressFromState(
state,
walletIdx,
addrIdx,
);
if (!removed) {
setFlash("This address could not be removed.");
return;
}
target = null;
// Save before broadcasting: the background reads the active address
// back out of storage to build accountsChanged.
await saveState();
if (activeAddressChanged) broadcastActiveChanged();
ctx.renderWalletList();
goBack();
showFlash("Address removed.");
});
}
// recoveryPathText and balanceWarningHtml are exported so the two pieces of
// copy that carry the screen's substance can be tested without a DOM; show()
// is a one-line assignment for each.
module.exports = { init, show, recoveryPathText, balanceWarningHtml };

View File

@@ -25,6 +25,7 @@ const VIEWS = [
"add-token",
"settings",
"delete-wallet-confirm",
"delete-address-confirm",
"settings-addtoken",
"transaction",
"approve-site",
@@ -217,6 +218,20 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
return html;
}
// Whether an address holds anything at all: ETH or any ERC-20 the wallet
// knows about. Deliberately unrounded — the rendered lines round to four
// decimals, so a dust balance displays as 0.0000 while still being real
// money at a real address. Callers that warn about holdings must ask this,
// not the rendered figure.
function addressHoldsFunds(addr) {
if (!addr) return false;
if (parseFloat(addr.balance || "0") > 0) return true;
for (const t of addr.tokenBalances || []) {
if (parseFloat(t.balance || "0") > 0) return true;
}
return false;
}
// Truncate the middle of a string, replacing removed characters with "…".
// Safety: refuses to truncate more than 10 characters, which is the maximum
// that still prevents address spoofing attacks (see Display Consistency in
@@ -463,6 +478,7 @@ module.exports = {
flashCopyFeedback,
balanceLine,
balanceLinesForAddress,
addressHoldsFunds,
addressColor,
addressDotHtml,
escapeHtml,

View File

@@ -21,6 +21,7 @@ const {
resetSendValidation,
} = require("./send");
const { deriveAddressFromXpub } = require("../../shared/wallet");
const { canRemoveAddress } = require("../../shared/walletDelete");
const {
walletDefect,
walletDefectHtml,
@@ -240,6 +241,12 @@ function walletListHtml() {
html += `<div class="address-row py-1 border-b border-border-light cursor-pointer hover:bg-hover" data-wallet="${wi}" data-address="${ai}">`;
const isActive = state.activeAddress === addr.address;
const infoBtn = `<span class="btn-addr-info text-xs cursor-pointer border border-border hover:bg-fg hover:text-bg" style="padding:0" data-wallet="${wi}" data-address="${ai}">[info]</span>`;
// Only where a wallet can spare the address: a wallet holding a
// single address has no remove control, because its last address
// is never removable.
const removeBtn = canRemoveAddress(wallet)
? `<span class="btn-remove-address text-xs cursor-pointer border border-border hover:bg-fg hover:text-bg ml-1" style="padding:0" data-wallet="${wi}" data-address="${ai}" title="Remove this address from the wallet">[x]</span>`
: "";
const dot = addressDotHtml(addr.address);
const titleBold = isActive ? "font-bold" : "";
html += `<div class="text-xs ${titleBold}">Address ${ai + 1}</div>`;
@@ -248,7 +255,7 @@ function walletListHtml() {
}
html += `<div class="flex text-xs items-center justify-between">`;
html += `<span class="flex items-center break-all">${addr.ensName ? "" : dot}${addr.address}</span>`;
html += `<span class="flex-shrink-0 ml-1">${infoBtn}</span>`;
html += `<span class="flex-shrink-0 ml-1">${infoBtn}${removeBtn}</span>`;
html += `</div>`;
const addrUsd = formatUsd(getAddressValueUsd(addr));
html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrUsd || "&nbsp;"}</div>`;
@@ -304,6 +311,16 @@ function render(ctx) {
});
});
container.querySelectorAll(".btn-remove-address").forEach((btn) => {
btn.addEventListener("click", (e) => {
e.stopPropagation();
ctx.showDeleteAddress(
parseInt(btn.dataset.wallet, 10),
parseInt(btn.dataset.address, 10),
);
});
});
container.querySelectorAll(".btn-add-address").forEach((btn) => {
btn.addEventListener("click", async (e) => {
e.stopPropagation();

View File

@@ -12,8 +12,9 @@ const {
const { state, currentAddress } = require("../../shared/state");
let ctx;
const { getProvider } = require("../../shared/balances");
const { KNOWN_SYMBOLS, resolveSymbol } = require("../../shared/tokenList");
const { resolveSymbol } = require("../../shared/tokenList");
const { isLowHolderCount } = require("../../shared/holders");
const { isSpoofedSymbol } = require("../../shared/symbolSpoof");
const { getAddress } = require("ethers");
const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000";
@@ -116,14 +117,6 @@ function updateToValidation() {
}
}
function isSpoofedToken(t) {
const upper = (t.symbol || "").toUpperCase();
if (!KNOWN_SYMBOLS.has(upper)) return false;
const legit = KNOWN_SYMBOLS.get(upper);
if (legit === null) return true;
return t.address.toLowerCase() !== legit;
}
function renderSendTokenSelect(addr) {
const sel = $("send-token");
sel.innerHTML = '<option value="ETH">ETH</option>';
@@ -131,7 +124,7 @@ function renderSendTokenSelect(addr) {
(state.fraudContracts || []).map((a) => a.toLowerCase()),
);
for (const t of addr.tokenBalances || []) {
if (isSpoofedToken(t)) continue;
if (isSpoofedSymbol(t.symbol, t.address)) continue;
if (fraudSet.has(t.address.toLowerCase())) continue;
// An unknown holder count does not withhold a token the user holds:
// only a count the explorer actually reported as below the threshold

View File

@@ -7,17 +7,144 @@
// the signer from the artifact and checks it against the approval it is
// holding before acting on it. All recovery is delegated to ethers.
//
// The check is an allowlist, in both directions, because a denylist cannot be
// correct against a transaction format that keeps gaining fields:
//
// - only transaction types 0, 1 and 2 are accepted. Every later EIP-2718 type
// adds a field with consequences of its own — EIP-7702's authorizationList
// rewrites the code at the signer's own account, EIP-4844's blob
// commitments carry a separate fee — and a check that enumerates the fields
// it refuses admits every one of them by default.
// - after the per-field comparisons, the artifact is rebuilt from those
// checked fields and nothing else, and the two are compared byte for byte.
// Anything the artifact carries that this module does not name is absent
// from the rebuild and changes the bytes, so the final assertion is that
// the artifact *is* the approved transaction, not merely that it is not one
// of the tampered shapes that were thought of.
// - every comparison runs against the decode, but the string handed to
// broadcastTransaction() is the artifact. So the artifact is also required
// to be the canonical re-encoding of its own decode, which is what makes
// the checked transaction and the broadcast bytes the same object rather
// than two things that merely decode alike.
//
// Every consequential field is compared, and a mismatch is a refusal to act,
// never a warning: what the user approved is what gets broadcast, or nothing
// does.
//
// Fields the approval does not carry are not treated as zero. The popup
// populates nonce, gas limit, fee and chain id through populateTransaction()
// when the requesting page did not fix them, so there is no approved value to
// compare against; treating absent as zero would refuse every legitimate
// transaction. Those fields are instead held to the absolute ceilings below,
// and the chain id is always checked against the selected network rather than
// against the approval alone, which is what makes a cross-chain replay
// impossible.
//
// Every failure message is a full sentence, because these strings are shown to
// the user and returned to the dApp.
const {
Transaction,
accessListify,
getAddress,
getBytes,
verifyMessage,
verifyTypedData,
} = require("ethers");
// The only transaction types this wallet signs: legacy, EIP-2930 and
// EIP-1559. populateTransaction() produces nothing else, so nothing else can
// be an artifact of an approval this wallet raised.
const ALLOWED_TX_TYPES = [0, 1, 2];
// The serialized fields of each allowed type, which is also the complete set
// of fields the checks below compare or bound. The artifact is rebuilt from
// exactly these at the end of verification and compared byte for byte, so a
// field outside this table cannot ride along unexamined.
const SERIALIZED_FIELDS = {
0: ["chainId", "nonce", "gasPrice", "gasLimit", "to", "value", "data"],
1: [
"chainId",
"nonce",
"gasPrice",
"gasLimit",
"to",
"value",
"data",
"accessList",
],
2: [
"chainId",
"nonce",
"maxPriorityFeePerGas",
"maxFeePerGas",
"gasLimit",
"to",
"value",
"data",
"accessList",
],
};
// Fields no allowed type may carry. The type allowlist already excludes every
// type that defines them, and the structural check at the end of verification
// would catch them anyway; they are named here so that an artifact carrying
// one is refused with a message that says what it was.
const FORBIDDEN_FIELDS = [
{
key: "authorizationList",
message:
"The signed transaction would hand the signing account over to another contract, which was not approved.",
},
{
key: "blobVersionedHashes",
message:
"The signed transaction carries blob commitments, which were not approved.",
},
{
key: "blobs",
message:
"The signed transaction carries blobs, which were not approved.",
},
{
key: "maxFeePerBlobGas",
message:
"The signed transaction carries a blob gas fee, which was not approved.",
},
];
// Above the block gas limit of every supported network (see networks.js), so
// no transaction that could ever be included is refused by it.
const MAX_GAS_LIMIT = 100000000n;
// 100,000 gwei per gas: orders of magnitude above the highest fee either
// supported network has produced, and low enough to catch a fee that would
// hand the validator the balance.
const MAX_FEE_PER_GAS = 100000000000000n;
// A refusal to act on an artifact: it is not the thing that was approved, so
// the approval it was offered against is spent and must not be retried. Every
// throw in this module is one of these; the background distinguishes them from
// transient failures (a busy node, a failed broadcast), which leave the
// approval standing so the user can try again.
class ApprovalMismatchError extends Error {
constructor(message) {
super(message);
this.name = "ApprovalMismatchError";
this.approvalMismatch = true;
}
}
function refuse(message) {
return new ApprovalMismatchError(message);
}
// Whether a signing failure leaves the approval usable. Anything that is not a
// mismatch is the user's to correct and retry.
function failureIsRetryable(err) {
return !(err && err.approvalMismatch === true);
}
// Case-insensitive address comparison that tolerates absent values on either
// side. Two absent addresses compare equal (contract creation has no `to`).
function sameAddress(a, b) {
@@ -31,11 +158,64 @@ function sameAddress(a, b) {
}
}
// Normalize a transaction value (hex string, decimal string, number or
// bigint) to a bigint. An absent value is zero, matching ethers.
function normalizeValue(v) {
if (v === null || v === undefined || v === "") return 0n;
// Whether the approval fixed a value for a field at all.
function present(v) {
return v !== null && v !== undefined && v !== "";
}
// Whether a field carries anything at all. An empty array is nothing: ethers
// reports an absent access list on a type 2 transaction as `[]`.
function carriesValue(v) {
if (!present(v)) return false;
if (Array.isArray(v)) return v.length > 0;
return true;
}
// Normalize a quantity that must be present, refusing anything that is not a
// number: an approval carrying junk in a fee field cannot be compared, and an
// uncomparable field is a refusal rather than a pass.
function normalizeQuantity(v, label) {
try {
return BigInt(v);
} catch {
throw refuse(
"The approved " +
label +
" is not a number, so it cannot be" +
" compared with the signed transaction.",
);
}
}
// Normalize a transaction value (hex string, decimal string, number or
// bigint) to a bigint. An absent value is zero, matching ethers. The value is
// page-controlled, so it goes through the same refusal as every other
// quantity rather than throwing a raw BigInt conversion error.
function normalizeValue(v) {
if (!present(v)) return 0n;
return normalizeQuantity(v, "value");
}
// Normalize an access list to a comparable string. An absent or empty list is
// the empty string, so absent and `[]` are the same thing.
function normalizeAccessList(v) {
if (!carriesValue(v)) return "";
let list;
try {
list = accessListify(v);
} catch {
throw refuse(
"The approved access list is not a valid access list, so it cannot be compared with the signed transaction.",
);
}
return list
.map(
(entry) =>
String(entry.address).toLowerCase() +
":" +
entry.storageKeys.map((k) => String(k).toLowerCase()).join(","),
)
.join(";");
}
// Normalize call data to a lowercase hex string. Absent data is "0x".
@@ -44,44 +224,216 @@ function normalizeData(v) {
return String(v).toLowerCase();
}
// Quantity fields the requesting page may fix in the approval. Each is
// compared exactly when the approval carries it, and left to the ceilings
// above when it does not.
const APPROVED_QUANTITIES = [
{
key: "nonce",
label: "nonce",
message: "The signed transaction does not carry the approved nonce.",
},
{
key: "gasLimit",
label: "gas limit",
message:
"The signed transaction does not carry the approved gas limit.",
},
{
key: "gasPrice",
label: "gas price",
message:
"The signed transaction does not carry the approved gas price.",
},
{
key: "maxFeePerGas",
label: "maximum fee per gas",
message:
"The signed transaction does not carry the approved maximum fee per gas.",
},
{
key: "maxPriorityFeePerGas",
label: "maximum priority fee per gas",
message:
"The signed transaction does not carry the approved maximum priority fee per gas.",
},
];
// Refuse a field only a transaction type this wallet does not sign can carry.
// The type allowlist keeps these unreachable in production, which is exactly
// what they are for; it also means nothing else exercises them, so this is
// exported and tested on its own rather than left to be believed.
function assertNoForbiddenFields(parsed) {
for (const field of FORBIDDEN_FIELDS) {
if (carriesValue(parsed[field.key])) throw refuse(field.message);
}
}
// Closing structural check. Rebuild the transaction from the fields the
// comparisons cover, and nothing else, then compare the unsigned bytes. Every
// field carried by the artifact but absent from the rebuild changes the
// serialization, so this refuses anything this module does not account for —
// including a field a future ethers learns to parse onto an allowed type —
// instead of waving it through by not naming it. Also exported for its own
// test: nothing reachable today can make the bytes differ.
function assertNothingUnchecked(parsed) {
let rebuilt;
try {
const fields = { type: parsed.type };
for (const key of SERIALIZED_FIELDS[parsed.type]) {
fields[key] = parsed[key];
}
rebuilt = Transaction.from(fields);
} catch {
throw refuse(
"The signed transaction could not be rebuilt from the fields that were checked, so it cannot be shown to be the approved transaction.",
);
}
if (rebuilt.unsignedSerialized !== parsed.unsignedSerialized) {
throw refuse(
"The signed transaction carries data beyond the fields that were checked against the approval.",
);
}
}
// The other half of the closing check, and the one that makes it bind on the
// bytes that actually leave: every comparison above runs against the decode,
// so on its own the rebuild proves only that the transaction ethers understood
// is the approved one. What the background hands to broadcastTransaction() is
// the artifact string itself. Requiring the artifact to be exactly the
// canonical re-encoding of its own decode closes the gap between the two —
// no encoding the decoder normalizes away (a leading zero byte on an RLP
// quantity, say) can differ from what was checked. Hex case is not part of the
// encoding, so only that is normalized before comparing.
function assertCanonicalBytes(parsed, rawSignedTx) {
if (parsed.serialized !== String(rawSignedTx).toLowerCase()) {
throw refuse(
"The signed transaction is not encoded canonically, so the bytes that would be broadcast are not the bytes that were checked.",
);
}
}
// Assert that a raw signed transaction is the transaction the user approved,
// signed by the address the approval was raised for. Returns the parsed
// ethers Transaction on success, throws otherwise.
function verifySignedTx(rawSignedTx, txParams, expectedFrom) {
// signed by the address the approval was raised for, on the network that is
// selected. Returns the parsed ethers Transaction on success, throws
// otherwise.
function verifySignedTx(rawSignedTx, txParams, expectedFrom, selectedChainId) {
if (typeof rawSignedTx !== "string" || !rawSignedTx.startsWith("0x")) {
throw new Error("The signed transaction is missing or malformed.");
throw refuse("The signed transaction is missing or malformed.");
}
let parsed;
try {
parsed = Transaction.from(rawSignedTx);
} catch {
throw new Error("The signed transaction could not be decoded.");
throw refuse("The signed transaction could not be decoded.");
}
if (!parsed.from) {
throw new Error("The signed transaction carries no valid signature.");
throw refuse("The signed transaction carries no valid signature.");
}
if (!sameAddress(parsed.from, expectedFrom)) {
throw new Error(
throw refuse(
"The signed transaction was signed by a different address than the one that was approved.",
);
}
// Before any field is looked at: the type decides which fields exist at
// all, so an unrecognised type is refused outright rather than compared
// field by field against an approval that cannot describe it.
if (!ALLOWED_TX_TYPES.includes(parsed.type)) {
throw refuse(
"The signed transaction is of a type this wallet does not sign, so what it would do beyond the approved transfer cannot be checked.",
);
}
assertNoForbiddenFields(parsed);
// The selected network, not the artifact, is the authority on which chain
// this may be broadcast to; without it nothing can be verified.
if (!present(selectedChainId)) {
throw refuse(
"The selected network is unknown, so the signed transaction cannot be checked against it.",
);
}
if (parsed.chainId !== normalizeQuantity(selectedChainId, "network")) {
throw refuse(
"The signed transaction is for a different network than the one that is selected.",
);
}
if (
present(txParams.chainId) &&
parsed.chainId !== normalizeQuantity(txParams.chainId, "network")
) {
throw refuse(
"The signed transaction is for a different network than the one that was approved.",
);
}
if (!sameAddress(parsed.to, txParams.to)) {
throw new Error(
throw refuse(
"The signed transaction does not go to the approved recipient.",
);
}
if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) {
throw new Error(
throw refuse(
"The signed transaction does not carry the approved value.",
);
}
if (normalizeData(parsed.data) !== normalizeData(txParams.data)) {
throw new Error(
throw refuse(
"The signed transaction does not carry the approved call data.",
);
}
if (
normalizeAccessList(parsed.accessList) !==
normalizeAccessList(txParams.accessList)
) {
throw refuse(
"The signed transaction does not carry the approved access list.",
);
}
// An approval that fixed EIP-1559 fees must not be signed as a legacy
// transaction, and vice versa: the fee the user agreed to is only
// meaningful under the mechanism it was quoted in.
const approvedEip1559 =
present(txParams.maxFeePerGas) ||
present(txParams.maxPriorityFeePerGas);
const approvedLegacy = present(txParams.gasPrice);
const signedEip1559 = parsed.type === 2;
if (
(approvedEip1559 && !signedEip1559) ||
(approvedLegacy && signedEip1559)
) {
throw refuse(
"The signed transaction does not use the approved fee mechanism.",
);
}
for (const field of APPROVED_QUANTITIES) {
if (!present(txParams[field.key])) continue;
const approved = normalizeQuantity(txParams[field.key], field.label);
if (normalizeQuantity(parsed[field.key], field.label) !== approved) {
throw refuse(field.message);
}
}
if (parsed.gasLimit > MAX_GAS_LIMIT) {
throw refuse(
"The signed transaction sets a gas limit no network this wallet supports can accept.",
);
}
for (const key of ["gasPrice", "maxFeePerGas", "maxPriorityFeePerGas"]) {
const fee = parsed[key];
if (fee !== null && fee !== undefined && fee > MAX_FEE_PER_GAS) {
throw refuse(
"The signed transaction sets a fee per gas far above any plausible value.",
);
}
}
assertNothingUnchecked(parsed);
assertCanonicalBytes(parsed, rawSignedTx);
return parsed;
}
@@ -91,7 +443,7 @@ function verifySignedTx(rawSignedTx, txParams, expectedFrom) {
// address on success, throws otherwise.
function verifySignature(signParams, signature, expectedFrom) {
if (typeof signature !== "string" || !signature.startsWith("0x")) {
throw new Error("The signature is missing or malformed.");
throw refuse("The signature is missing or malformed.");
}
let recovered;
@@ -109,11 +461,11 @@ function verifySignature(signParams, signature, expectedFrom) {
recovered = verifyTypedData(domain, types, message, signature);
}
} catch {
throw new Error("The signature could not be verified.");
throw refuse("The signature could not be verified.");
}
if (!sameAddress(recovered, expectedFrom)) {
throw new Error(
throw refuse(
"The signature was produced by a different address than the one that was approved.",
);
}
@@ -121,4 +473,98 @@ function verifySignature(signParams, signature, expectedFrom) {
return recovered;
}
module.exports = { verifySignedTx, verifySignature, sameAddress };
// The stage a transaction approval failed at. Which stage it is decides
// whether the approval survives the failure.
const TX_STAGE_SIGN = "sign";
const TX_STAGE_VERIFY = "verify";
const TX_STAGE_BROADCAST = "broadcast";
// Not a failure of this request at all: a second response arrived for an
// approval an attempt already holds. The first attempt is still running and
// may yet succeed, so the one thing the popup must not say is "start again
// from the site".
const TX_STAGE_INFLIGHT = "inflight";
function errorText(err) {
if (typeof err === "string" && err !== "") return err;
if (err && (err.shortMessage || err.message)) {
return err.shortMessage || err.message;
}
return "The transaction could not be sent.";
}
// What the background does with a pending transaction approval after a failed
// attempt: what it tells the popup, and whether the approval is spent
// (resolved to the requesting page as an error and deleted) or left standing
// so the user can try the transaction they already saw again.
//
// - sign: the popup could not produce an artifact, almost always a wrong
// password. Nothing left the extension, so the approval stands.
// - verify: a mismatch is a refusal and spends the approval — an artifact
// that is not the approved transaction must never be retried against that
// approval. Anything else failed before the check ran and is retryable.
// - broadcast: always terminal. A broadcast that throws after the node
// accepted the transaction is routine (a timeout, a dropped response, a
// node answering "already known"), and the popup's retry does not
// re-broadcast these bytes — it re-runs populateTransaction() and signs
// again at a freshly fetched pending-tag nonce. Retrying would therefore
// put a second transaction on the chain for one approval.
function describeTxFailure(stage, err) {
const error = errorText(err);
const retryable =
stage === TX_STAGE_SIGN ||
(stage === TX_STAGE_VERIFY && failureIsRetryable(err));
return { error, retryable, spendApproval: !retryable };
}
// What the popup shows and does after the background reports a failed signing
// attempt. A retryable failure leaves the approval pending in the background,
// so the button goes back to being usable; a refusal spent the approval, and
// the popup says so rather than offering a button that cannot succeed.
//
// A failed broadcast gets its own wording: the transaction may already be on
// the network, so telling the user to start again from the site is exactly the
// wrong instruction.
function describeSigningFailure(response, fallbackMessage) {
let message = (response && response.error) || fallbackMessage;
if (!/[.!?]$/.test(message)) message += ".";
const retryable = !!(response && response.retryable);
const stage = response && response.stage;
if (!retryable) {
if (stage === TX_STAGE_BROADCAST) {
message +=
" The transaction may still have reached the network." +
" Check the account before sending it again.";
} else if (stage === TX_STAGE_INFLIGHT) {
message +=
" The first attempt is still running and may still succeed." +
" Wait for it rather than starting again.";
} else {
message +=
" This request can no longer be signed. Please start it" +
" again from the site.";
}
}
return { message, retryable };
}
module.exports = {
verifySignedTx,
verifySignature,
assertNoForbiddenFields,
assertNothingUnchecked,
assertCanonicalBytes,
sameAddress,
failureIsRetryable,
describeTxFailure,
describeSigningFailure,
ApprovalMismatchError,
ALLOWED_TX_TYPES,
SERIALIZED_FIELDS,
FORBIDDEN_FIELDS,
TX_STAGE_SIGN,
TX_STAGE_VERIFY,
TX_STAGE_BROADCAST,
TX_STAGE_INFLIGHT,
MAX_GAS_LIMIT,
MAX_FEE_PER_GAS,
};

View File

@@ -11,8 +11,9 @@ const {
const { ERC20_ABI } = require("./constants");
const { log, debugFetch } = require("./log");
const { deriveAddressFromXpub } = require("./wallet");
const { KNOWN_SYMBOLS, TOKEN_BY_ADDRESS } = require("./tokenList");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { LOW_HOLDER_THRESHOLD, parseHoldersCount } = require("./holders");
const { isSpoofedSymbol } = require("./symbolSpoof");
// Use a static network to skip auto-detection (which can fail and cause
// "could not coalesce error" on some RPC endpoints like Cloudflare).
@@ -89,15 +90,11 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
// Skip spam tokens the user never asked to see
if (!isKnown && !isTracked && !hasEnoughHolders) continue;
// Skip tokens spoofing a known symbol from a different address
const sym = (item.token.symbol || "").toUpperCase();
const legitAddr = KNOWN_SYMBOLS.get(sym);
if (
legitAddr !== undefined &&
legitAddr !== null &&
tokenAddr !== legitAddr
)
continue;
// Skip tokens spoofing a known symbol from a different address.
// Every row here is an ERC-20 the explorer reported, so it has a
// contract address; the native ETH balance is fetched over RPC in
// refreshBalances and never passes through this loop.
if (isSpoofedSymbol(item.token.symbol, tokenAddr)) continue;
balances.push({
address: item.token.address_hash,

43
src/shared/symbolSpoof.js Normal file
View File

@@ -0,0 +1,43 @@
// The known-symbol spoof rule, in one place.
//
// A token that borrows a known symbol from a contract that is not the one
// that symbol belongs to is a spoof, and the wallet hides it. Three surfaces
// ask that question — the transaction history, the Send token selector and
// the balance list — and they must answer it identically: a token the history
// calls fake while the balance list lists it as a holding is worse than
// either verdict alone, because the balance list is where the user forms
// their belief about what they own (issue #235).
//
// KNOWN_SYMBOLS maps a symbol to the lowercased contract address that may
// bear it, or to null. Null means the symbol belongs to the native asset,
// which has no contract at all, so no contract may bear it and every one
// that does is a spoof. "ETH" is the only such entry today; the rule is
// written so that a second one needs no change here or at any call site.
const { KNOWN_SYMBOLS } = require("./tokenList");
// Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum
// over the address, not part of its identity.
function normalizeAddress(addr) {
return (addr || "").toLowerCase();
}
// True when a token bearing `symbol` from contract `contractAddress` is
// impersonating a known symbol.
//
// An empty contract address is the native asset, which is never a spoof:
// this is what keeps the user's real ETH out of the rule, and it holds for
// any symbol that becomes null-mapped later, not just for ETH.
function isSpoofedSymbol(symbol, contractAddress) {
const contract = normalizeAddress(contractAddress);
if (!contract) return false;
const sym = (symbol || "").toUpperCase();
if (!KNOWN_SYMBOLS.has(sym)) return false;
const legit = KNOWN_SYMBOLS.get(sym);
if (legit === null) return true;
return contract !== normalizeAddress(legit);
}
module.exports = {
isSpoofedSymbol,
};

View File

@@ -8,8 +8,9 @@
const { formatEther, formatUnits } = require("ethers");
const { log, debugFetch } = require("./log");
const { KNOWN_SYMBOLS, TOKEN_BY_ADDRESS } = require("./tokenList");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { parseHoldersCount, isLowHolderCount } = require("./holders");
const { isSpoofedSymbol } = require("./symbolSpoof");
// Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum
// over the address, not part of its identity. Every address comparison in
@@ -245,18 +246,6 @@ async function fetchRecentTransactions(address, blockscoutUrl, count = 25) {
return result;
}
// Check if a token transfer is spoofing a known symbol.
// Returns true if the symbol matches a known token but the contract
// address doesn't match the legitimate one.
function isSpoofedSymbol(tx) {
if (!tx.contractAddress) return false;
const symbol = (tx.symbol || "").toUpperCase();
if (!KNOWN_SYMBOLS.has(symbol)) return false;
const legit = KNOWN_SYMBOLS.get(symbol);
if (legit === null) return true; // "ETH" as ERC-20 is always fake
return normalizeAddress(tx.contractAddress) !== normalizeAddress(legit);
}
// Pure filter function. Takes raw transactions and filter settings,
// returns { transactions, newFraudContracts }.
function filterTransactions(txs, filters = {}) {
@@ -283,7 +272,7 @@ function filterTransactions(txs, filters = {}) {
const contract = normalizeAddress(tx.contractAddress);
// Filter spoofed known symbols and record the fraud contract
if (hideSpoofed && isSpoofedSymbol(tx)) {
if (hideSpoofed && isSpoofedSymbol(tx.symbol, tx.contractAddress)) {
if (contract && !fraudSet.has(contract)) {
fraudSet.add(contract);
newFraud.push(contract);

View File

@@ -1,5 +1,22 @@
// Wallet deletion state transition, kept out of the view so the selection
// and broadcast rules are testable without a DOM.
// Wallet and address deletion state transitions, kept out of the views so the
// selection and broadcast rules are testable without a DOM.
// Two records of the same address can be stored in different cases, so
// address equality is never a literal string comparison.
function sameAddress(a, b) {
if (a === null || a === undefined || b === null || b === undefined) {
return false;
}
return String(a).toLowerCase() === String(b).toLowerCase();
}
// Forget every site permission held against the given addresses.
function dropSitePermissions(state, addresses) {
for (const addr of addresses) {
delete state.allowedSites[addr];
delete state.deniedSites[addr];
}
}
// Remove wallet `walletIdx` from `state` and repair the derived state.
//
@@ -18,19 +35,13 @@ function removeWalletFromState(state, walletIdx) {
const wallet = state.wallets[walletIdx];
const addresses = (wallet.addresses || []).map((a) => a.address);
const previousActive = state.activeAddress;
const activeWasDeleted =
previousActive !== null &&
previousActive !== undefined &&
addresses.some(
(a) => a.toLowerCase() === String(previousActive).toLowerCase(),
const activeWasDeleted = addresses.some((a) =>
sameAddress(a, previousActive),
);
state.wallets.splice(walletIdx, 1);
for (const addr of addresses) {
delete state.allowedSites[addr];
delete state.deniedSites[addr];
}
dropSitePermissions(state, addresses);
state.hasWallet = state.wallets.length > 0;
@@ -58,6 +69,77 @@ function removeWalletFromState(state, walletIdx) {
return { activeAddressChanged: state.activeAddress !== previousActive };
}
// Whether a wallet may be offered a per-address remove control, and the same
// gate the removal itself is held behind.
//
// Only a wallet that derives its addresses from an extended key can hold more
// than one, so only those get the control — a key wallet has exactly one
// address and no "+" button either. The last address of any wallet is never
// removable: a wallet with no addresses is what delete-wallet is for.
function canRemoveAddress(wallet) {
if (!wallet) return false;
if (wallet.type !== "hd" && wallet.type !== "xprv") return false;
return (wallet.addresses || []).length > 1;
}
// Remove address `addrIdx` of wallet `walletIdx` and repair the derived state.
//
// Nothing is destroyed here. The address stays derivable from the wallet's own
// key material and any funds at it are untouched; this only stops the wallet
// tracking it. `nextIndex` is deliberately left alone — it is a derivation
// high-water mark, so "+" derives a fresh index rather than handing back the
// address just removed, and the gap it leaves is within what
// `scanForAddresses()` re-discovers on a later import.
//
// The rules mirror removeWalletFromState() one level down:
// - The call is refused unless canRemoveAddress() allows it, so the last
// address of a wallet always survives.
// - Site permissions are dropped for the removed address.
// - `selectedAddress` follows the splice, but only within the wallet that
// lost the address: it is decremented when an earlier address was
// removed, and falls back to that wallet's first address when the
// selection itself was removed. `selectedWallet` never moves, because the
// wallet list does not.
// - `activeAddress` moves only when it was the removed address, and then to
// the wallet's first remaining address.
//
// Returns whether the address was removed and whether `activeAddress`
// changed, so the caller can broadcast it.
function removeAddressFromState(state, walletIdx, addrIdx) {
const wallet = state.wallets[walletIdx];
const refused = { removed: false, activeAddressChanged: false };
if (!canRemoveAddress(wallet)) return refused;
if (!wallet.addresses[addrIdx]) return refused;
const address = wallet.addresses[addrIdx].address;
const previousActive = state.activeAddress;
const activeWasRemoved = sameAddress(address, previousActive);
wallet.addresses.splice(addrIdx, 1);
dropSitePermissions(state, [address]);
if (state.selectedWallet === walletIdx) {
if (state.selectedAddress === addrIdx) {
state.selectedAddress = 0;
} else if (
typeof state.selectedAddress === "number" &&
state.selectedAddress > addrIdx
) {
state.selectedAddress -= 1;
}
}
if (activeWasRemoved) {
state.activeAddress = wallet.addresses[0].address;
}
return {
removed: true,
activeAddressChanged: state.activeAddress !== previousActive,
};
}
// Tell the background the active address changed, so it re-emits
// accountsChanged to connected sites. Same call shape as the address
// switch in the home view.
@@ -67,4 +149,9 @@ function broadcastActiveChanged() {
runtime.sendMessage({ type: "AUTISTMASK_ACTIVE_CHANGED" });
}
module.exports = { removeWalletFromState, broadcastActiveChanged };
module.exports = {
canRemoveAddress,
removeAddressFromState,
removeWalletFromState,
broadcastActiveChanged,
};

View File

@@ -1,8 +1,28 @@
const { Network, Transaction, Wallet } = require("ethers");
const {
Network,
Transaction,
Wallet,
decodeRlp,
encodeRlp,
} = require("ethers");
const {
verifySignedTx,
verifySignature,
assertNoForbiddenFields,
assertNothingUnchecked,
assertCanonicalBytes,
sameAddress,
failureIsRetryable,
describeTxFailure,
describeSigningFailure,
ALLOWED_TX_TYPES,
SERIALIZED_FIELDS,
FORBIDDEN_FIELDS,
TX_STAGE_SIGN,
TX_STAGE_VERIFY,
TX_STAGE_BROADCAST,
MAX_GAS_LIMIT,
MAX_FEE_PER_GAS,
} = require("../src/shared/approvalVerify");
const { getSignerForAddress } = require("../src/shared/wallet");
@@ -18,6 +38,10 @@ const other = new Wallet(OTHER_KEY);
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const OTHER_RECIPIENT = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
// The chain id of the selected network, as networks.js carries it.
const SELECTED = "0x1";
const SEPOLIA = "0xaa36a7";
// Approved parameters as a dApp would supply them over eth_sendTransaction.
const TX_PARAMS = {
from: signer.address,
@@ -27,25 +51,38 @@ const TX_PARAMS = {
gas: "0x5208",
};
// Build a signable transaction from approved params. The popup does the same
// thing through populateTransaction(); here the fields are fixed so the test
// needs no provider.
function txFor(params) {
return {
// The values populateTransaction() fills in when the dApp fixed none of them.
const POPULATED = {
chainId: 1,
nonce: 7,
gasLimit: 100000n,
maxFeePerGas: 2000000000n,
maxPriorityFeePerGas: 1000000000n,
type: 2,
};
// Build a signable transaction from approved params. The popup does the same
// thing through populateTransaction(); here the fields are fixed so the test
// needs no provider. `overrides` stands in for what a tampered or misbuilt
// popup would put on the wire.
function txFor(params, overrides) {
return {
...POPULATED,
to: params.to,
value: params.value === undefined ? 0n : BigInt(params.value),
data: params.data || "0x",
...(overrides || {}),
};
}
async function signedFor(params, withWallet) {
return (withWallet || signer).signTransaction(txFor(params));
async function signedFor(params, withWallet, overrides) {
return (withWallet || signer).signTransaction(txFor(params, overrides));
}
// Sign the approved transaction with one field changed from what was
// populated, which is the shape of every tamper case below.
async function signedWith(overrides) {
return signedFor(TX_PARAMS, signer, overrides);
}
describe("sameAddress", () => {
@@ -71,7 +108,7 @@ describe("sameAddress", () => {
describe("verifySignedTx", () => {
test("accepts the approved transaction signed by the approved address", async () => {
const raw = await signedFor(TX_PARAMS);
const parsed = verifySignedTx(raw, TX_PARAMS, signer.address);
const parsed = verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
expect(parsed.from).toBe(signer.address);
expect(parsed.hash).toBe(Transaction.from(raw).hash);
});
@@ -79,14 +116,16 @@ describe("verifySignedTx", () => {
test("accepts a contract creation with no recipient", async () => {
const params = { to: undefined, value: "0x0", data: "0x600160005500" };
const raw = await signedFor(params);
expect(() => verifySignedTx(raw, params, signer.address)).not.toThrow();
expect(() =>
verifySignedTx(raw, params, signer.address, SELECTED),
).not.toThrow();
});
test("accepts an absent value as zero", async () => {
const approved = { to: RECIPIENT, data: "0x" };
const raw = await signedFor(approved);
expect(() =>
verifySignedTx(raw, approved, signer.address),
verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow();
});
@@ -94,7 +133,7 @@ describe("verifySignedTx", () => {
const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" };
const raw = await signedFor(approved);
expect(() =>
verifySignedTx(raw, approved, signer.address),
verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow();
});
@@ -103,9 +142,9 @@ describe("verifySignedTx", () => {
...TX_PARAMS,
to: OTHER_RECIPIENT,
});
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow(
/approved recipient/,
);
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
).toThrow(/approved recipient/);
});
test("rejects an inflated value", async () => {
@@ -113,48 +152,48 @@ describe("verifySignedTx", () => {
...TX_PARAMS,
value: "0x4563918244f40000",
});
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow(
/approved value/,
);
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
).toThrow(/approved value/);
});
test("rejects substituted call data", async () => {
const raw = await signedFor({ ...TX_PARAMS, data: "0xc0ffee" });
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow(
/approved call data/,
);
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
).toThrow(/approved call data/);
});
test("rejects a transaction signed by a different address", async () => {
const raw = await signedFor(TX_PARAMS, other);
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow(
/different address/,
);
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
).toThrow(/different address/);
});
test("rejects an unsigned transaction", () => {
const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized;
expect(() =>
verifySignedTx(unsigned, TX_PARAMS, signer.address),
verifySignedTx(unsigned, TX_PARAMS, signer.address, SELECTED),
).toThrow(/no valid signature/);
});
test("rejects a missing or malformed payload", () => {
expect(() =>
verifySignedTx(undefined, TX_PARAMS, signer.address),
verifySignedTx(undefined, TX_PARAMS, signer.address, SELECTED),
).toThrow(/missing or malformed/);
expect(() => verifySignedTx("nope", TX_PARAMS, signer.address)).toThrow(
/missing or malformed/,
);
expect(() =>
verifySignedTx("0xc0ffee", TX_PARAMS, signer.address),
verifySignedTx("nope", TX_PARAMS, signer.address, SELECTED),
).toThrow(/missing or malformed/);
expect(() =>
verifySignedTx("0xc0ffee", TX_PARAMS, signer.address, SELECTED),
).toThrow(/could not be decoded/);
});
test("every rejection message is a full sentence", async () => {
const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
try {
verifySignedTx(raw, TX_PARAMS, signer.address);
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
throw new Error("expected a rejection");
} catch (e) {
expect(e.message).toMatch(/^[A-Z].*\.$/);
@@ -162,6 +201,606 @@ describe("verifySignedTx", () => {
});
});
// One case per consequential field: the field alone differs from what was
// approved, and that alone must refuse the signature.
describe("verifySignedTx field comparison", () => {
test("rejects a chain id that is not the selected network", async () => {
const raw = await signedWith({ chainId: 11155111 });
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
).toThrow(/different network than the one that is selected/);
});
test("rejects a chain id that is not the approved one", async () => {
// Selected network and signed chain id agree; the dApp asked for a
// different chain, so the artifact is not what was approved.
const approved = { ...TX_PARAMS, chainId: SEPOLIA };
const raw = await signedWith({});
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).toThrow(/different network than the one that was approved/);
});
test("refuses when the selected network is unknown", async () => {
const raw = await signedWith({});
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, undefined),
).toThrow(/selected network is unknown/);
});
test("rejects a substituted nonce", async () => {
const approved = { ...TX_PARAMS, nonce: 7 };
const raw = await signedWith({ nonce: 8 });
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).toThrow(/approved nonce/);
});
test("rejects a substituted gas limit", async () => {
const approved = { ...TX_PARAMS, gasLimit: "0x186a0" };
const raw = await signedWith({ gasLimit: 250000n });
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).toThrow(/approved gas limit/);
});
test("rejects a substituted maximum fee per gas", async () => {
const approved = { ...TX_PARAMS, maxFeePerGas: "0x77359400" };
const raw = await signedWith({ maxFeePerGas: 900000000000n });
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).toThrow(/approved maximum fee per gas/);
});
test("rejects a substituted maximum priority fee per gas", async () => {
const approved = { ...TX_PARAMS, maxPriorityFeePerGas: "0x3b9aca00" };
const raw = await signedWith({ maxPriorityFeePerGas: 1500000000n });
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).toThrow(/approved maximum priority fee per gas/);
});
test("rejects a substituted legacy gas price", async () => {
const approved = { ...TX_PARAMS, gasPrice: "0x77359400" };
const legacy = {
type: 0,
gasPrice: 9000000000n,
maxFeePerGas: null,
maxPriorityFeePerGas: null,
};
const raw = await signedWith(legacy);
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).toThrow(/approved gas price/);
});
test("rejects an approved legacy fee signed as an EIP-1559 fee", async () => {
const approved = { ...TX_PARAMS, gasPrice: "0x77359400" };
const raw = await signedWith({});
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).toThrow(/approved fee mechanism/);
});
test("rejects an approved EIP-1559 fee signed as a legacy fee", async () => {
const approved = { ...TX_PARAMS, maxFeePerGas: "0x77359400" };
const raw = await signedWith({
type: 0,
gasPrice: 2000000000n,
maxFeePerGas: null,
maxPriorityFeePerGas: null,
});
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).toThrow(/approved fee mechanism/);
});
test("rejects a gas limit above anything a supported network accepts", async () => {
const raw = await signedWith({ gasLimit: MAX_GAS_LIMIT + 1n });
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
).toThrow(/gas limit no network this wallet supports/);
});
test("rejects an absurd fee per gas the approval never fixed", async () => {
const raw = await signedWith({
maxFeePerGas: MAX_FEE_PER_GAS + 1n,
maxPriorityFeePerGas: MAX_FEE_PER_GAS + 1n,
});
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
).toThrow(/fee per gas far above any plausible value/);
});
test("every field mismatch is a refusal, not a warning", async () => {
const raw = await signedWith({ nonce: 8 });
try {
verifySignedTx(
raw,
{ ...TX_PARAMS, nonce: 7 },
signer.address,
SELECTED,
);
throw new Error("expected a rejection");
} catch (e) {
expect(e.approvalMismatch).toBe(true);
expect(e.message).toMatch(/^[A-Z].*\.$/);
}
});
});
// The 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.
describe("verifySignedTx transaction type", () => {
const BARE_APPROVAL = {
from: signer.address,
to: RECIPIENT,
value: "0x2386f26fc10000",
data: "0x",
};
// An EIP-7702 artifact that pays the approved amount to the approved
// recipient and, in the same transaction, installs the attacker's code at
// the signer's own account for good. Every field the approval screen shows
// matches; only the type and the authorization list do not.
test("refuses a type 4 artifact that delegates the signer's own account", async () => {
const authorization = await signer.authorize({
address: OTHER_RECIPIENT,
chainId: 1,
nonce: 8,
});
const raw = await signedFor(BARE_APPROVAL, signer, {
type: 4,
authorizationList: [authorization],
});
const parsed = Transaction.from(raw);
expect(parsed.type).toBe(4);
expect(parsed.authorizationList[0].address).toBe(OTHER_RECIPIENT);
expect(() =>
verifySignedTx(raw, BARE_APPROVAL, signer.address, SELECTED),
).toThrow(/type this wallet does not sign/);
});
test("refuses a type 3 blob artifact", async () => {
const raw = await signedFor(BARE_APPROVAL, signer, {
type: 3,
maxFeePerBlobGas: 1000000000n,
blobVersionedHashes: ["0x01" + "ab".repeat(31)],
});
expect(Transaction.from(raw).type).toBe(3);
expect(() =>
verifySignedTx(raw, BARE_APPROVAL, signer.address, SELECTED),
).toThrow(/type this wallet does not sign/);
});
test("refuses every type outside the allowlist, not just the known ones", async () => {
for (const type of [3, 4]) {
expect(ALLOWED_TX_TYPES).not.toContain(type);
}
expect(ALLOWED_TX_TYPES).toEqual([0, 1, 2]);
});
test("a type refusal is a refusal, not a warning", async () => {
const authorization = await signer.authorize({
address: OTHER_RECIPIENT,
chainId: 1,
nonce: 8,
});
const raw = await signedFor(BARE_APPROVAL, signer, {
type: 4,
authorizationList: [authorization],
});
try {
verifySignedTx(raw, BARE_APPROVAL, signer.address, SELECTED);
throw new Error("expected a rejection");
} catch (e) {
expect(e.approvalMismatch).toBe(true);
expect(e.message).toMatch(/^[A-Z].*\.$/);
}
});
test("accepts a legacy type 0 transaction", async () => {
const approved = { ...BARE_APPROVAL, gasPrice: "0x77359400" };
const raw = await signedFor(approved, signer, {
type: 0,
gasPrice: 2000000000n,
maxFeePerGas: null,
maxPriorityFeePerGas: null,
});
expect(() =>
verifySignedTx(raw, approved, 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, {
type: 1,
gasPrice: 2000000000n,
maxFeePerGas: null,
maxPriorityFeePerGas: null,
accessList,
});
expect(Transaction.from(raw).type).toBe(1);
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow();
});
test("refuses an access list the approval never carried", async () => {
const raw = await signedFor(BARE_APPROVAL, signer, {
accessList: [{ address: OTHER_RECIPIENT, storageKeys: [] }],
});
expect(() =>
verifySignedTx(raw, BARE_APPROVAL, signer.address, SELECTED),
).toThrow(/approved access list/);
});
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, {});
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow();
});
});
// The allowlist is only exhaustive while it accounts for every field an
// artifact can carry. These tests are what makes that claim checkable rather
// than asserted.
describe("verifySignedTx exhaustiveness", () => {
// Every accessor ethers exposes on a parsed transaction, and where this
// module deals with it. If an ethers upgrade adds a transaction field,
// this fails and forces a decision about it instead of letting it default
// to unchecked.
test("every field ethers can parse is accounted for", () => {
const derived = [
// Recovered from the signature or computed from the payload, not
// independent content: covered by the signer check and by the
// fields below.
"from",
"fromPublicKey",
"hash",
"serialized",
"signature",
"type",
"typeName",
"unsignedHash",
"unsignedSerialized",
// Blob sidecar machinery, meaningful only alongside `blobs`,
// which is refused outright.
"kzg",
"blobWrapperVersion",
];
const accounted = new Set([
...derived,
...FORBIDDEN_FIELDS.map((f) => f.key),
...Object.values(SERIALIZED_FIELDS).flat(),
]);
const exposed = Object.getOwnPropertyNames(Transaction.prototype)
.filter((name) => {
const d = Object.getOwnPropertyDescriptor(
Transaction.prototype,
name,
);
return d && typeof d.get === "function";
})
.sort();
expect(exposed.filter((name) => !accounted.has(name))).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
// on trust.
test("a forbidden field is refused even on an allowed type", async () => {
const authorization = await signer.authorize({
address: OTHER_RECIPIENT,
chainId: 1,
nonce: 8,
});
const carriers = {
authorizationList: [authorization],
blobVersionedHashes: ["0x01" + "ab".repeat(31)],
blobs: ["0x00"],
maxFeePerBlobGas: 1n,
};
for (const key of Object.keys(carriers)) {
expect(FORBIDDEN_FIELDS.map((f) => f.key)).toContain(key);
let thrown;
try {
assertNoForbiddenFields({ type: 2, [key]: carriers[key] });
throw new Error("expected a rejection");
} catch (e) {
thrown = e;
}
expect(thrown.approvalMismatch).toBe(true);
expect(thrown.message).toMatch(/^[A-Z].*\.$/);
}
expect(() => assertNoForbiddenFields({ type: 2 })).not.toThrow();
});
// Stands in for a future ethers that parses a field this module does not
// know about onto an allowed type: every field the module checks is
// identical, and the bytes are not.
test("an artifact carrying more than the checked fields is refused", async () => {
const parsed = Transaction.from(await signedWith({}));
const smuggled = { type: parsed.type };
for (const key of SERIALIZED_FIELDS[parsed.type]) {
smuggled[key] = parsed[key];
}
smuggled.unsignedSerialized = parsed.unsignedSerialized + "ff";
expect(() => assertNothingUnchecked(smuggled)).toThrow(
/beyond the fields that were checked/,
);
expect(() => assertNothingUnchecked(parsed)).not.toThrow();
});
// The closing check rebuilds the artifact from the fields the module
// compared and compares the bytes, so an artifact carrying anything else
// is refused without the module having to name it. Assert the rebuild is
// faithful for every accepted shape, since a rebuild that dropped a
// legitimate field would refuse honest transactions.
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,
},
},
{
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)],
},
],
},
},
{ approved: TX_PARAMS, overrides: {} },
];
for (const shape of shapes) {
const raw = await signedFor(
shape.approved,
signer,
shape.overrides,
);
const parsed = verifySignedTx(
raw,
shape.approved,
signer.address,
SELECTED,
);
const fields = { type: parsed.type };
for (const key of SERIALIZED_FIELDS[parsed.type]) {
fields[key] = parsed[key];
}
expect(Transaction.from(fields).unsignedSerialized).toBe(
parsed.unsignedSerialized,
);
}
});
});
// Every comparison above runs against the decode, but the string that is
// handed to broadcastTransaction() is the artifact. An encoding the decoder
// normalizes away therefore checks as one transaction and broadcasts as
// different bytes, so the artifact must be the canonical encoding of itself.
describe("verifySignedTx canonical encoding", () => {
// Re-encode a signed type-2 artifact with a leading zero byte on the RLP
// value field. It decodes to exactly the approved transaction — same
// value, same signer, same everything the field comparisons look at — and
// it is not the same string.
async function nonCanonical() {
const raw = await signedWith({});
const items = decodeRlp("0x" + raw.slice(4));
// type 2 payload order: chainId, nonce, maxPriorityFeePerGas,
// maxFeePerGas, gasLimit, to, value, data, accessList, then the
// signature.
const padded = items.slice();
padded[6] = "0x00" + items[6].slice(2);
return "0x02" + encodeRlp(padded).slice(2);
}
test("the mutation decodes to the approved transaction and is not it", async () => {
const raw = await signedWith({});
const mutated = await nonCanonical();
const parsed = Transaction.from(mutated);
expect(mutated).not.toBe(raw);
expect(mutated.length).toBeGreaterThan(raw.length);
expect(parsed.value).toBe(BigInt(TX_PARAMS.value));
expect(parsed.from).toBe(signer.address);
expect(parsed.serialized).not.toBe(mutated);
});
test("refuses an artifact that is not its own canonical encoding", async () => {
const mutated = await nonCanonical();
expect(() =>
verifySignedTx(mutated, TX_PARAMS, signer.address, SELECTED),
).toThrow(/not encoded canonically/);
});
test("assertCanonicalBytes accepts what ethers itself produced", async () => {
const raw = await signedWith({});
expect(() =>
assertCanonicalBytes(Transaction.from(raw), raw),
).not.toThrow();
});
test("hex case is not part of the encoding", async () => {
const raw = await signedWith({});
const upper = "0x" + raw.slice(2).toUpperCase();
expect(() =>
verifySignedTx(upper, TX_PARAMS, signer.address, SELECTED),
).not.toThrow();
});
});
// The approval and the artifact spell the same values differently. None of
// these differences is tampering, so none may refuse the signature.
describe("verifySignedTx normalization", () => {
test("accepts a decimal chain id against a hex selected network", async () => {
const raw = await signedWith({});
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, 1),
).not.toThrow();
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, "1"),
).not.toThrow();
});
test("accepts an approved chain id written in hex", async () => {
const raw = await signedWith({});
const approved = { ...TX_PARAMS, chainId: "0x1" };
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow();
});
test("accepts a hex nonce against a numeric one", async () => {
const raw = await signedWith({ nonce: 7 });
expect(() =>
verifySignedTx(
raw,
{ ...TX_PARAMS, nonce: "0x7" },
signer.address,
SELECTED,
),
).not.toThrow();
});
test("accepts a decimal gas limit against a hex one", async () => {
const raw = await signedWith({ gasLimit: 100000n });
expect(() =>
verifySignedTx(
raw,
{ ...TX_PARAMS, gasLimit: "100000" },
signer.address,
SELECTED,
),
).not.toThrow();
});
test("accepts fee fields spelled as hex, decimal, number and bigint", async () => {
const raw = await signedWith({});
for (const maxFee of [
"0x77359400",
"2000000000",
2000000000,
2000000000n,
]) {
expect(() =>
verifySignedTx(
raw,
{ ...TX_PARAMS, maxFeePerGas: maxFee },
signer.address,
SELECTED,
),
).not.toThrow();
}
});
test("accepts an approval that fixes no nonce, gas or fee at all", async () => {
const raw = await signedWith({});
expect(() =>
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
).not.toThrow();
});
test("accepts an approval whose recipient case differs", async () => {
const raw = await signedWith({});
const approved = { ...TX_PARAMS, to: RECIPIENT.toLowerCase() };
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow();
});
test("accepts absent call data against 0x", async () => {
const approved = { to: RECIPIENT, value: "0x0" };
const raw = await signedFor({ ...approved, data: "0x" });
expect(() =>
verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow();
});
test("refuses an approved quantity that is not a number", async () => {
const raw = await signedWith({});
expect(() =>
verifySignedTx(
raw,
{ ...TX_PARAMS, maxFeePerGas: "cheap" },
signer.address,
SELECTED,
),
).toThrow(/is not a number/);
});
// The value is page-controlled. A refusal is correct; a raw BigInt
// conversion error is not, because it is not a mismatch, so it would be
// reported retryable and leave the approval unspent behind a live button
// that can never succeed.
test("refuses an approved value that is not a number, as a mismatch", async () => {
const raw = await signedWith({});
for (const value of ["cheap", 1.5, "1e18", {}]) {
let thrown;
try {
verifySignedTx(
raw,
{ ...TX_PARAMS, value },
signer.address,
SELECTED,
);
throw new Error("expected a rejection");
} catch (e) {
thrown = e;
}
expect(thrown.approvalMismatch).toBe(true);
expect(thrown.message).toMatch(/approved value is not a number/);
expect(failureIsRetryable(thrown)).toBe(false);
}
});
test("refuses an approved access list that is not an access list", async () => {
const raw = await signedWith({});
expect(() =>
verifySignedTx(
raw,
{ ...TX_PARAMS, accessList: ["nope"] },
signer.address,
SELECTED,
),
).toThrow(/not a valid access list/);
});
});
const TYPED_DATA = JSON.stringify({
domain: {
name: "AutistMask Test",
@@ -281,6 +920,147 @@ describe("verifySignature", () => {
});
});
// What happens after a signing attempt fails: the background keeps the
// approval for anything the user can correct, and the popup only offers the
// button again when it did.
describe("signing failure and retry", () => {
test("a failure that is not a mismatch leaves the approval retryable", () => {
expect(failureIsRetryable(new Error("The node is unreachable."))).toBe(
true,
);
expect(failureIsRetryable(undefined)).toBe(true);
});
test("a mismatch spends the approval", async () => {
const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
try {
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
throw new Error("expected a rejection");
} catch (e) {
expect(failureIsRetryable(e)).toBe(false);
}
});
test("a retryable failure keeps the button usable and says only what failed", () => {
const outcome = describeSigningFailure(
{ error: "The node rejected the transaction.", retryable: true },
"The transaction could not be sent.",
);
expect(outcome.retryable).toBe(true);
expect(outcome.message).toBe("The node rejected the transaction.");
});
test("a refusal tells the user to start again from the site", () => {
const outcome = describeSigningFailure(
{
error: "The signed transaction does not go to the approved recipient.",
retryable: false,
},
"The transaction could not be sent.",
);
expect(outcome.retryable).toBe(false);
expect(outcome.message).toMatch(/start it again from the site\.$/);
});
test("a refusal for an attempt already running does not say to start again", () => {
const outcome = describeSigningFailure(
{
error: "This request is already being signed.",
retryable: false,
stage: "inflight",
},
"The message could not be signed.",
);
expect(outcome.retryable).toBe(false);
expect(outcome.message).not.toMatch(/start it again from the site/);
expect(outcome.message).toMatch(/first attempt is still running/);
});
test("a response the background never sent is treated as a spent approval", () => {
const outcome = describeSigningFailure(
undefined,
"The transaction could not be sent.",
);
expect(outcome.retryable).toBe(false);
expect(outcome.message).toMatch(/^The transaction could not be sent\./);
});
test("every failure message is a full sentence", () => {
const outcome = describeSigningFailure(
{ error: "The node is on fire", retryable: true },
"The transaction could not be sent.",
);
expect(outcome.message).toMatch(/^[A-Z].*\.$/);
});
test("a popup that could not sign leaves the approval standing", () => {
const outcome = describeTxFailure(
TX_STAGE_SIGN,
"That password is incorrect. Please try again.",
);
expect(outcome.retryable).toBe(true);
expect(outcome.spendApproval).toBe(false);
expect(outcome.error).toMatch(/password is incorrect/);
});
test("a mismatch found at verification spends the approval", async () => {
const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
let outcome;
try {
verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
} catch (e) {
outcome = describeTxFailure(TX_STAGE_VERIFY, e);
}
expect(outcome.retryable).toBe(false);
expect(outcome.spendApproval).toBe(true);
});
test("a failure before the check ran is still retryable", () => {
const outcome = describeTxFailure(
TX_STAGE_VERIFY,
new Error("The wallet state could not be read."),
);
expect(outcome.retryable).toBe(true);
expect(outcome.spendApproval).toBe(false);
});
// A broadcast that throws after the node took the transaction is routine:
// a timeout, a dropped response, a node answering "already known". The
// popup's retry does not re-broadcast the same bytes — it re-populates and
// re-signs at a freshly fetched nonce — so a retryable broadcast failure
// would put the approved transfer on the chain twice.
test("a failed broadcast is terminal, whatever the node said", () => {
for (const message of [
"already known",
"timeout of 30000ms exceeded",
"could not coalesce error",
"replacement transaction underpriced",
]) {
const outcome = describeTxFailure(
TX_STAGE_BROADCAST,
new Error(message),
);
expect(outcome.retryable).toBe(false);
expect(outcome.spendApproval).toBe(true);
expect(outcome.error).toBe(message);
}
});
test("a failed broadcast does not tell the user to send it again", () => {
const outcome = describeSigningFailure(
{
error: "The node did not answer.",
retryable: false,
stage: TX_STAGE_BROADCAST,
},
"The transaction could not be sent.",
);
expect(outcome.retryable).toBe(false);
expect(outcome.message).toMatch(/may still have reached the network/);
expect(outcome.message).not.toMatch(/start it again from the site/);
});
});
// End-to-end over the messaging boundary, without a browser: run the exact
// sequence the approval popup runs, then hand the artifact to the exact check
// the background runs before it broadcasts or resolves. Only what the popup
@@ -314,7 +1094,12 @@ describe("popup signing sequence to background verification", () => {
test("a populated, signed transaction is accepted and broadcastable", async () => {
const rawSignedTx = await popupSignsTx(TX_PARAMS);
const parsed = verifySignedTx(rawSignedTx, TX_PARAMS, signer.address);
const parsed = verifySignedTx(
rawSignedTx,
TX_PARAMS,
signer.address,
SELECTED,
);
expect(parsed.nonce).toBe(7);
expect(parsed.chainId).toBe(1n);
expect(parsed.gasLimit).toBe(21000n);
@@ -349,7 +1134,14 @@ describe("popup signing sequence to background verification", () => {
to: OTHER_RECIPIENT,
});
expect(() =>
verifySignedTx(rawSignedTx, TX_PARAMS, signer.address),
verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SELECTED),
).toThrow(/approved recipient/);
});
test("the background rejects a transaction populated on another network", async () => {
const rawSignedTx = await popupSignsTx(TX_PARAMS);
expect(() =>
verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SEPOLIA),
).toThrow(/different network than the one that is selected/);
});
});

View File

@@ -0,0 +1,679 @@
// The background's approval message wiring, driven end to end: a dApp
// eth_sendTransaction raises a pending approval, and the popup answers it with
// AUTISTMASK_TX_RESPONSE / AUTISTMASK_SIGN_RESPONSE.
//
// What this exists for is the duplicate response. The handler verifies and
// broadcasts asynchronously, and the approval deliberately survives a
// retryable failure so the user can try again with the transaction they
// 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.
const { Wallet } = require("ethers");
const SIGNER_KEY =
"0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d";
const signer = new Wallet(SIGNER_KEY);
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const ORIGIN = "https://dapp.example";
const HOSTNAME = "dapp.example";
const EXT_URL = "chrome-extension://autistmask/";
// What the dApp asks for: no nonce, no gas, no fees. This is the shape that
// makes a duplicate broadcast possible at all.
const TX_PARAMS = {
from: signer.address,
to: RECIPIENT,
value: "0x2386f26fc10000",
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.
function populated(nonce) {
return {
type: 2,
chainId: 1,
nonce,
gasLimit: 100000n,
maxFeePerGas: 2000000000n,
maxPriorityFeePerGas: 1000000000n,
to: TX_PARAMS.to,
value: BigInt(TX_PARAMS.value),
data: TX_PARAMS.data,
};
}
function signedAtNonce(nonce) {
return signer.signTransaction(populated(nonce));
}
// A promise whose settlement the test controls, so a broadcast can be held in
// flight while the second response arrives.
function deferred() {
let resolve;
let reject;
const promise = new Promise((res, rej) => {
resolve = res;
reject = rej;
});
return { promise, resolve, reject };
}
// Load the background worker against stubbed browser and network APIs and
// return the handles the tests drive it through. Everything that would touch
// the network or the browser's own schedulers is mocked; the approval
// verification is the real module, because that is what the handler under
// test is wired to.
function loadBackground(options) {
const opts = options || {};
jest.resetModules();
const broadcastTransaction = jest.fn();
const loadState = jest.fn(opts.loadState || (async () => {}));
jest.doMock("../src/shared/state", () => ({
state: { rpcUrl: "https://rpc.invalid", wallets: [] },
loadState,
saveState: jest.fn(async () => {}),
currentNetwork: () => ({ chainId: "0x1" }),
}));
jest.doMock("../src/shared/balances", () => ({
getProvider: () => ({ broadcastTransaction }),
refreshBalances: jest.fn(async () => {}),
}));
jest.doMock("../src/shared/phishingDomains", () => ({
isPhishingDomain: () => false,
refreshPhishingListOnSchedule: jest.fn(async () => {}),
initPhishingList: jest.fn(async () => {}),
}));
jest.doMock("../src/shared/alarms", () => ({
BALANCE_REFRESH_ALARM: "balance",
PHISHING_REFRESH_ALARM: "phishing",
BALANCE_REFRESH_PERIOD_MINUTES: 1,
ensureRecurringAlarms: jest.fn(async () => {}),
registerAlarmHandlers: jest.fn(),
}));
const persisted = {
wallets: [
{ name: "Wallet 1", type: "hd", addresses: [signer.address] },
],
rpcUrl: "https://rpc.invalid",
activeAddress: signer.address,
allowedSites: { [signer.address]: [HOSTNAME] },
deniedSites: {},
};
let messageListener = null;
let windowRemovedListener = null;
const created = [];
const removed = [];
global.chrome = {
storage: {
local: {
get: jest.fn(async () => ({ autistmask: persisted })),
set: jest.fn(async () => {}),
},
},
runtime: {
getURL: (path) => EXT_URL + path,
onMessage: {
addListener: (fn) => {
messageListener = fn;
},
},
onConnect: { addListener: () => {} },
lastError: null,
},
windows: {
getLastFocused: (cb) => cb(null),
create: (options2, cb) => {
created.push(options2);
cb({ id: created.length });
},
remove: (id, cb) => {
removed.push(id);
if (cb) cb();
},
// Captured, not swallowed: closing the approval window is the
// event that used to retire an approval out from under a live
// broadcast, and a no-op stub here hides exactly that.
onRemoved: {
addListener: (fn) => {
windowRemovedListener = fn;
},
},
},
tabs: {
query: (q, cb) => cb([]),
sendMessage: () => {},
},
action: { setPopup: () => {} },
};
require("../src/background/index");
// Send a message the way the browser would, and hand back whatever the
// handler passed to sendResponse.
function send(msg, sender) {
const sendResponse = jest.fn();
const kept = messageListener(msg, sender || {}, sendResponse);
return { sendResponse, kept };
}
// 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() {
let rpcResult = null;
const sendResponse = jest.fn((r) => {
rpcResult = r;
});
messageListener(
{
type: "AUTISTMASK_RPC",
method: "eth_sendTransaction",
params: [TX_PARAMS],
},
{ origin: ORIGIN },
sendResponse,
);
return {
id: () => new URL(created[0].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) {
windowRemovedListener(windowId);
}
return {
send,
requestTx,
closeWindow,
broadcastTransaction,
loadState,
created,
removed,
fromPopup: { url: EXT_URL + "src/popup/index.html" },
};
}
// Let the handler's promise chain run to the next suspension point.
async function settle() {
for (let i = 0; i < 10; i++) await Promise.resolve();
}
afterEach(() => {
delete global.chrome;
jest.resetModules();
});
describe("one approval, one broadcast", () => {
test("a second AUTISTMASK_TX_RESPONSE for the same id does not broadcast again", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
expect(id).toBeTruthy();
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
// The popup answers. Verification passes and the broadcast is held
// open, which is the whole window the second message arrives in.
const first = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
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.
const second = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(8),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
expect(second.sendResponse).toHaveBeenCalledWith(
expect.objectContaining({
error: expect.stringMatching(/already being sent/),
retryable: false,
}),
);
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
});
test("the same artifact sent twice broadcasts once", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
const raw = await signedAtNonce(7);
const msg = {
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: raw,
};
bg.send(msg, { url: bg.fromPopup.url });
bg.send(msg, { url: bg.fromPopup.url });
await settle();
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
});
test("a response arriving after the broadcast finished finds nothing to send", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
const late = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(8),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
expect(late.sendResponse).not.toHaveBeenCalled();
});
test("a second AUTISTMASK_SIGN_RESPONSE for the same id is refused", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
// Hold the transaction approval in flight, then answer it a second
// time as if it were a sign approval: the sign handler must apply the
// same interlock rather than running its own verification.
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
const second = bg.send(
{
type: "AUTISTMASK_SIGN_RESPONSE",
id,
approved: true,
signature: "0x00",
},
{ url: bg.fromPopup.url },
);
await settle();
expect(second.sendResponse).toHaveBeenCalledWith(
expect.objectContaining({
error: expect.stringMatching(/already being signed/),
retryable: false,
}),
);
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
});
});
// 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 () => {
let failNext = true;
const bg = loadBackground({
loadState: async () => {
if (failNext) {
failNext = false;
throw new Error("storage unavailable");
}
},
});
const pending = bg.requestTx();
await settle();
const id = pending.id();
const first = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
expect(first.sendResponse).toHaveBeenCalledWith(
expect.objectContaining({ retryable: true }),
);
bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
const retry = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
expect(retry.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
expect(pending.result()).toEqual({ result: "0xfeed" });
});
test("a mismatched artifact spends the approval outright", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
// Signed for a different recipient than the one that was approved.
const wrong = await signer.signTransaction({
...populated(7),
to: "0xdAC17F958D2ee523a2206206994597C13D831ec7",
});
const first = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: wrong,
},
{ url: bg.fromPopup.url },
);
await settle();
expect(first.sendResponse).toHaveBeenCalledWith(
expect.objectContaining({ retryable: false, stage: "verify" }),
);
const retry = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
expect(retry.sendResponse).not.toHaveBeenCalled();
});
});
// The claim is what makes one approval one broadcast, so it has to hold
// against everything else that retires an approval, not just against a second
// AUTISTMASK_TX_RESPONSE. Each of these paths used to resolve the waiting
// promise 4001 while the attempt behind it ran to completion: the transaction
// reached the chain and the page was told the user rejected it, which invites
// the user to send it a second time at a fresh nonce.
describe("a claimed approval outlives every other retirement path", () => {
// The approval popup stays open across the broadcast it is waiting on, so
// a user closing an apparently-hung window needs no adversary at all.
test("closing the approval window mid-broadcast still reports the result", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
const first = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
// The user closes the window while the broadcast is still open.
bg.closeWindow(1);
await settle();
expect(pending.result()).toBeNull();
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
});
test("switching the active address mid-broadcast still reports the result", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
// The user switches account in the toolbar popup, which rejects and
// force-closes every pending approval.
bg.send(
{ type: "AUTISTMASK_ACTIVE_CHANGED" },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toBeNull();
// The window an in-flight attempt reports into is left standing too.
expect(bg.removed).toEqual([]);
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
});
test("a reject arriving mid-broadcast is refused, not honoured", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
const reject = bg.send(
{ type: "AUTISTMASK_TX_RESPONSE", id, approved: false },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toBeNull();
expect(reject.sendResponse).toHaveBeenCalledWith(
expect.objectContaining({
retryable: false,
stage: "broadcast",
}),
);
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
});
// The refusals above must not cost the rejection its ordinary meaning.
test("with no attempt running, closing the window still rejects", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
bg.closeWindow(1);
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
});
test("with no attempt running, an active-address switch still rejects and closes", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
bg.send(
{ type: "AUTISTMASK_ACTIVE_CHANGED" },
{ url: bg.fromPopup.url },
);
await settle();
expect(pending.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
expect(bg.removed).toEqual([1]);
});
// A sign approval held by a running verification is the same shape, and
// the refusal must not tell the user to start again from the site while
// the first attempt may still hand back a signature.
test("a reject during a sign attempt is refused with the in-flight stage", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
const inFlight = deferred();
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: bg.fromPopup.url },
);
await settle();
const reject = bg.send(
{ type: "AUTISTMASK_SIGN_RESPONSE", id, approved: false },
{ url: bg.fromPopup.url },
);
await settle();
expect(reject.sendResponse).toHaveBeenCalledWith(
expect.objectContaining({ retryable: false, stage: "inflight" }),
);
inFlight.resolve({ hash: "0xfeed" });
await settle();
expect(pending.result()).toEqual({ result: "0xfeed" });
});
});
describe("popup-only messages", () => {
test("a page sender cannot answer an approval", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
const id = pending.id();
const spoof = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id,
approved: true,
rawSignedTx: await signedAtNonce(7),
},
{ url: ORIGIN + "/index.html" },
);
await settle();
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
expect(spoof.sendResponse).toHaveBeenCalledWith({
error: "Unauthorized sender",
});
});
});

158
tests/deleteAddress.test.js Normal file
View File

@@ -0,0 +1,158 @@
// Tests for the copy on the address-removal confirmation (issue #162).
//
// The screen's whole job is to warn before a destructive-looking action, so
// the copy is the substance and is tested as such. Two things it must not
// get wrong: what it takes to get the address back — the app refuses both
// obvious routes — and what counts as holding something, which is any
// ERC-20 as well as ETH, at any size, including a balance that rounds to
// zero at the four decimals the balance lines render. The DOM behaviour
// around them is driven against the real popup by tests/e2e/run.js.
// helpers.js pulls in state.js, which reads chrome.storage.local at load.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
const { addressHoldsFunds } = require("../src/popup/views/helpers");
const {
recoveryPathText,
balanceWarningHtml,
} = require("../src/popup/views/deleteAddress");
const { prices, clearPrices } = require("../src/shared/prices");
const USDC = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
const EMPTY = { address: "0x1", balance: "0.0000", tokenBalances: [] };
const ETH_ONLY = { address: "0x1", balance: "1.5", tokenBalances: [] };
const DUST = { address: "0x1", balance: "0.00001", tokenBalances: [] };
const TOKEN_ONLY = {
address: "0x1",
balance: "0.0000",
tokenBalances: [{ address: USDC, symbol: "USDC", balance: "2500.0" }],
};
const ZERO_TOKEN = {
address: "0x1",
balance: "0",
tokenBalances: [{ address: USDC, symbol: "USDC", balance: "0" }],
};
afterEach(() => {
clearPrices();
});
describe("what the screen says it takes to get the address back", () => {
// The screen used to promise the address "can be brought back at any
// time by importing this wallet's recovery phrase again". That import is
// refused as a duplicate for as long as the wallet is present, which it
// always is here — a wallet never gives up its last address.
test("it does not promise a re-import while the wallet is here", () => {
const text = recoveryPathText({ type: "hd" });
expect(text).not.toMatch(/at any time/);
expect(text).toContain("is refused while this wallet is still here");
});
test("it names deleting the whole wallet as the route back", () => {
expect(recoveryPathText({ type: "hd" })).toContain(
"delete the whole wallet in Settings",
);
});
// The scan after a re-import finds used addresses only, so an address
// that never saw a transaction does not come back at all. Saying so is
// the difference between a warning and a false reassurance.
test("it states the limit: only on-chain activity is found", () => {
const text = recoveryPathText({ type: "hd" });
expect(text).toContain("only finds addresses that have on-chain");
expect(text).toContain("never been used is not found by it");
});
// The screen is offered on xprv wallets too, and an xprv wallet holds no
// recovery phrase — telling its owner to import one would send them
// looking for words that do not exist.
test("an xprv wallet is told about its extended private key", () => {
const text = recoveryPathText({ type: "xprv" });
expect(text).toContain("extended private key");
expect(text).not.toContain("recovery phrase");
});
test("an HD wallet is told about its recovery phrase", () => {
const text = recoveryPathText({ type: "hd" });
expect(text).toContain("recovery phrase");
expect(text).not.toContain("extended private key");
});
});
describe("whether an address holds anything", () => {
test("ETH counts", () => {
expect(addressHoldsFunds(ETH_ONLY)).toBe(true);
});
// The case that decides the screen: no ETH at all, and $2500 of a
// stablecoin sitting at the address.
test("an ERC-20 balance counts even with no ETH", () => {
expect(addressHoldsFunds(TOKEN_ONLY)).toBe(true);
});
// 0.00001 ETH renders as "0.0000" at four decimals. It is still money.
test("an ETH balance below the displayed precision counts", () => {
expect(addressHoldsFunds(DUST)).toBe(true);
});
test("an address holding nothing does not", () => {
expect(addressHoldsFunds(EMPTY)).toBe(false);
expect(addressHoldsFunds(ZERO_TOKEN)).toBe(false);
});
test("a missing address or missing fields do not", () => {
expect(addressHoldsFunds(undefined)).toBe(false);
expect(addressHoldsFunds({ address: "0x1" })).toBe(false);
});
});
describe("the balance warning on the removal confirmation", () => {
test("an address holding nothing gets a blank line, not a warning", () => {
expect(balanceWarningHtml(EMPTY)).toBe("&nbsp;");
expect(balanceWarningHtml(ZERO_TOKEN)).toBe("&nbsp;");
});
test("an ERC-20-only address is warned about, and its token listed", () => {
const html = balanceWarningHtml(TOKEN_ONLY);
expect(html).toContain("This address holds a balance.");
expect(html).toContain("does not move or spend anything");
expect(html).toContain("USDC");
expect(html).toContain("2500.0000");
});
// The rendered line says 0.0000 for this address — that is the display
// format, shared with Home and AddressDetail — and the warning is shown
// all the same, because the balance is not zero.
test("an ETH balance that renders as 0.0000 is warned about", () => {
const html = balanceWarningHtml(DUST);
expect(html).toContain("This address holds a balance.");
expect(html).toContain("<span>0.0000</span>");
});
// The sentence must not assert an amount, because any amount it could
// assert has been rounded: "This address holds 0.0000 ETH." is what the
// rounded form produces for an address that holds real money.
test("the warning sentence asserts no rounded amount", () => {
for (const addr of [DUST, ETH_ONLY, TOKEN_ONLY]) {
expect(balanceWarningHtml(addr)).not.toMatch(
/holds [\d.]+ (ETH|USDC)/,
);
}
});
test("the USD total is shown when prices are known", () => {
prices.ETH = 2000;
prices.USDC = 1;
expect(balanceWarningHtml(TOKEN_ONLY)).toContain("Total: $2,500.00");
expect(balanceWarningHtml(ETH_ONLY)).toContain("Total: $3,000.00");
});
// getAddressValueUsd() returns null on testnet and before the first
// price fetch. A "Total: $0.00" there would be a lie about the holdings.
test("no USD total is shown when prices are not known", () => {
expect(balanceWarningHtml(TOKEN_ONLY)).not.toContain("Total:");
});
});

View File

@@ -356,7 +356,14 @@ class Driver {
//
// Warnings are excluded so the semantics match Playwright's pageerror:
// uncaught errors only.
const READ_ERRORS_SCRIPT = `
//
// The read and the clear are ONE chrome script on purpose. Splitting them
// into two round trips leaves a blind window between them in which an
// error is logged into a buffer that is about to be discarded, and is
// destroyed unread rather than deferred to the next drain. That was not
// theoretical: with a separate reset() call, a probe of 100 sequenced
// throws at 20ms spacing lost one of them outright.
const DRAIN_ERRORS_SCRIPT = `
const origin = arguments[0];
const out = [];
for (const raw of Services.console.getMessageArray() || []) {
@@ -376,6 +383,7 @@ const READ_ERRORS_SCRIPT = `
cat: e.category,
});
}
Services.console.reset();
return out;
`;
@@ -385,25 +393,18 @@ class ConsoleErrors {
this.originPrefix = originPrefix;
}
// Everything logged since the last reset, then clear. Poll-based, so
// an error is attributed to the step that was running when it was
// drained, not to the moment inside that step at which it happened —
// see the limitation note in run.js.
// Everything logged since the last take, read and cleared atomically
// in a single chrome round trip. Poll-based, so an error is attributed
// to the step that was running when it was drained, not to the moment
// inside that step at which it happened — see the limitation note in
// run.js. Nothing between two takes is lost, though: an error that
// arrives mid-drain either makes this batch or the next one.
async take() {
const found = await this.driver.executeChrome(READ_ERRORS_SCRIPT, [
const found = await this.driver.executeChrome(DRAIN_ERRORS_SCRIPT, [
this.originPrefix,
]);
await this.reset();
return found || [];
}
// Discards the console outright. Anything logged and not yet taken is
// destroyed unread, so this is only ever correct straight after a
// take(); to move past a window of errors, take() them and report
// them somewhere.
async reset() {
await this.driver.executeChrome("Services.console.reset(); return 0;");
}
}
// ------------------------------------------------------------- startup

View File

@@ -22,12 +22,14 @@
// capture here is POLL-BASED, not event-streamed. The console service is
// drained at each step boundary, so an error is attributed to the step it
// was drained after, never to a moment within that step. What is drained
// covers the whole run from add-on install to one second after the last
// step returns — but only that far: an error logged more than that ~1s
// tail after the last step is never observed at all, because the browser
// is torn down first. The Chrome harness receives pageerror events as they
// happen and can say more. Do not read a green Firefox run as the same
// claim.
// covers the whole run from add-on install to the last drain below, which
// measures out at ~1.5s after the last step returns — errors at +0.5s,
// +1.0s and +1.5s are reported, +1.6s and later never are, because the
// browser is torn down first. Nothing inside that window is dropped: the
// drain reads and clears in one chrome round trip, so there is no gap for
// an error to be destroyed unread in. The Chrome harness receives
// pageerror events as they happen and can say more. Do not read a green
// Firefox run as the same claim.
"use strict";

View File

@@ -398,6 +398,101 @@ test("reopening the popup never lands on the phrase screen (#161)", async (env)
assertWiped(st, env.phrase, "after reopening the popup");
});
// -------------------------------------------- address removal (#162)
// Number of address rows across every wallet in the list, counted in the DOM
// whether or not Home is the screen on top.
function addressRowCount(page) {
return page.locator("#wallet-list .btn-addr-info").count();
}
function waitForAddressRows(page, n) {
return page.waitForFunction(
(want) =>
document.querySelectorAll("#wallet-list .btn-addr-info").length ===
want,
n,
{ timeout: 60000 },
);
}
// The suite arrives here with two wallets, an HD one and a key one, holding
// one address each.
test("only a wallet that can spare an address offers to remove one (#162)", async (env) => {
await visible(env.page, "#view-main");
const rows = await addressRowCount(env.page);
assert(rows === 2, "expected two address rows, got " + rows);
const offered = await env.page
.locator("#wallet-list .btn-remove-address")
.count();
assert(
offered === 0,
"a wallet holding its last address offered to remove it",
);
await env.page.click("#wallet-list .btn-add-address");
await waitForAddressRows(env.page, 3);
// Only the HD wallet's two rows; the key wallet still holds one address.
const nowOffered = await env.page
.locator("#wallet-list .btn-remove-address")
.count();
assert(
nowOffered === 2,
"expected the HD wallet's two rows to offer removal, got " + nowOffered,
);
});
// The gate itself: the control opens a confirmation, and leaving that
// confirmation by "Back" removes nothing.
test("leaving the removal confirmation removes nothing (#162)", async (env) => {
await env.page.locator("#wallet-list .btn-remove-address").nth(1).click();
await visible(env.page, "#view-delete-address-confirm");
const label = await env.page.locator("#delete-address-label").innerText();
assert(
label === "Address 2",
"the confirmation names the wrong address: " + JSON.stringify(label),
);
// The route back is written by the view, not by index.html, so an empty
// paragraph here means the user is confirming with no idea what it
// takes to undo. This wallet is an HD one, so it is told about its
// recovery phrase.
const recovery = await env.page
.locator("#delete-address-recovery")
.innerText();
assert(
recovery.includes("delete the whole wallet in Settings") &&
recovery.includes("recovery phrase"),
"the confirmation does not state the route back: " +
JSON.stringify(recovery),
);
// "Back" re-renders Home, so a count taken after it is a real
// measurement of the wallet rather than a stale screen.
await env.page.click("#btn-delete-address-back");
await visible(env.page, "#view-main");
const rows = await addressRowCount(env.page);
assert(rows === 3, "the address was removed without a confirmation");
});
test("confirming removes the address and returns Home (#162)", async (env) => {
await env.page.locator("#wallet-list .btn-remove-address").nth(1).click();
await visible(env.page, "#view-delete-address-confirm");
await env.page.click("#btn-delete-address-confirm");
await visible(env.page, "#view-main");
await waitForAddressRows(env.page, 2);
const offered = await env.page
.locator("#wallet-list .btn-remove-address")
.count();
assert(
offered === 0,
"the HD wallet still offers to remove its last address",
);
});
// ---------------------------------------------------------------- runner
async function main() {

296
tests/symbolSpoof.test.js Normal file
View File

@@ -0,0 +1,296 @@
// Tests for the known-symbol spoof rule (src/shared/symbolSpoof.js) and for
// its application on all three surfaces that show tokens: the transaction
// history, the Send token selector, and the balance list.
//
// Issue #235: the three surfaces disagreed about what a `null` entry in
// KNOWN_SYMBOLS means. The history and the selector read it as "no contract
// may bear this symbol" and filtered a fake `ETH` ERC-20; the balance list
// read it as "no comparison is possible" and listed the fake token next to
// the user's real ETH, which is where a user forms their belief about what
// they own. The rule now lives in one module, so a fourth surface cannot
// reintroduce a fourth reading, and these tests assert the same attack on
// each surface.
//
// Nothing here touches the network: global.fetch is a throwing stub and the
// only fetch path in the modules under test (debugFetch, from
// src/shared/log) is mocked at the module boundary.
// The RPC provider is replaced so that refreshBalances can be driven end to
// end: the native balance it reports must survive a balance list in which
// every ERC-20 row is a fake ETH. Everything else in ethers is the real
// module, including the formatters the assertions depend on.
jest.mock("ethers", () => {
const actual = jest.requireActual("ethers");
class StubProvider {
async getBalance() {
return 1234500000000000000n;
}
async lookupAddress() {
return null;
}
}
return {
...actual,
JsonRpcProvider: StubProvider,
Network: { from: () => ({}) },
};
});
jest.mock("../src/shared/log", () => ({
log: {
debugf: () => {},
infof: () => {},
warnf: () => {},
errorf: () => {},
},
debugFetch: jest.fn(),
setRuntimeDebug: () => {},
isDebug: () => false,
}));
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
global.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
const { isSpoofedSymbol } = require("../src/shared/symbolSpoof");
const { KNOWN_SYMBOLS } = require("../src/shared/tokenList");
const { filterTransactions } = require("../src/shared/transactions");
const {
fetchTokenBalances,
refreshBalances,
} = require("../src/shared/balances");
const { renderSendTokenSelect } = require("../src/popup/views/send");
const { state } = require("../src/shared/state");
const { debugFetch } = require("../src/shared/log");
// The fake "Ethereum" token with symbol "ETH" from the attack documented in
// README.md, given a holder count high enough to clear every other filter so
// that only the known-symbol rule can catch it.
const FAKE_ETH_CONTRACT = "0xd05339f9ea5ab9d9f03b9d57f671d2abd1f55c82";
const HOLDER = "0x66133e8ea0f5d1d612d2502a968757d1048c214a";
const USDC_CONTRACT = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
const WETH_CONTRACT = "0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2";
const BLOCKSCOUT = "https://eth.blockscout.com/api/v2";
describe("the shared rule", () => {
test('"ETH" is still the null-mapped symbol these tests assume', () => {
expect(KNOWN_SYMBOLS.get("ETH")).toBeNull();
});
test("a contract bearing a null-mapped symbol is a spoof", () => {
expect(isSpoofedSymbol("ETH", FAKE_ETH_CONTRACT)).toBe(true);
});
test("even a genuine contract may not bear a null-mapped symbol", () => {
expect(isSpoofedSymbol("ETH", WETH_CONTRACT)).toBe(true);
});
test("the native asset carries no contract and is never a spoof", () => {
expect(isSpoofedSymbol("ETH", null)).toBe(false);
expect(isSpoofedSymbol("ETH", undefined)).toBe(false);
expect(isSpoofedSymbol("ETH", "")).toBe(false);
});
// The native exemption is "has no contract address", not "the symbol is
// ETH". A second null-mapped symbol added to the table later inherits
// both halves of the rule without any call site being revisited.
test("a newly null-mapped symbol behaves the same way", () => {
const added = !KNOWN_SYMBOLS.has("XTZTEST");
KNOWN_SYMBOLS.set("XTZTEST", null);
try {
expect(isSpoofedSymbol("XTZTEST", FAKE_ETH_CONTRACT)).toBe(true);
expect(isSpoofedSymbol("XTZTEST", null)).toBe(false);
} finally {
if (added) KNOWN_SYMBOLS.delete("XTZTEST");
}
});
test("a known symbol from its own contract is not a spoof", () => {
expect(isSpoofedSymbol("USDC", USDC_CONTRACT)).toBe(false);
expect(isSpoofedSymbol("usdc", USDC_CONTRACT.toUpperCase())).toBe(
false,
);
});
test("a known symbol from another contract is a spoof", () => {
expect(isSpoofedSymbol("USDC", FAKE_ETH_CONTRACT)).toBe(true);
});
test("a symbol that is not in the table is not judged here", () => {
expect(isSpoofedSymbol("SPAMTKN", FAKE_ETH_CONTRACT)).toBe(false);
});
});
describe("surface 1: the transaction history", () => {
function fakeEthTransfer() {
return {
hash: "0x" + "1".repeat(64),
symbol: "ETH",
contractAddress: FAKE_ETH_CONTRACT,
holders: 900000,
valueGwei: null,
isContractCall: false,
};
}
test("a fake ETH token transfer is filtered", () => {
const result = filterTransactions([fakeEthTransfer()], {
hideSpoofedSymbols: true,
hideFraudContracts: true,
hideLowHolderTokens: true,
hideDustTransactions: true,
dustThresholdGwei: 100000,
});
expect(result.transactions).toEqual([]);
});
test("a real native ETH transfer survives", () => {
const native = {
hash: "0x" + "2".repeat(64),
symbol: "ETH",
contractAddress: null,
holders: null,
valueGwei: 5000000,
isContractCall: false,
};
const result = filterTransactions([native], {
hideSpoofedSymbols: true,
hideFraudContracts: true,
hideLowHolderTokens: true,
hideDustTransactions: true,
dustThresholdGwei: 100000,
});
expect(result.transactions).toEqual([native]);
});
});
describe("surface 2: the Send token selector", () => {
let select;
function render(tokenBalances) {
select = { innerHTML: "", children: [] };
select.appendChild = (child) => select.children.push(child);
globalThis.document = {
getElementById: (id) => (id === "send-token" ? select : null),
createElement: () => ({ value: "", textContent: "" }),
};
renderSendTokenSelect({
address: "0x" + "a".repeat(40),
tokenBalances,
});
}
beforeEach(() => {
state.fraudContracts = [];
state.hideLowHolderTokens = true;
});
test("a fake ETH token is not selectable", () => {
render([
{
address: FAKE_ETH_CONTRACT,
symbol: "ETH",
decimals: 18,
balance: "0.005",
holders: 900000,
},
]);
expect(select.children).toEqual([]);
});
test("native ETH remains the always-present option", () => {
render([]);
expect(select.innerHTML).toBe('<option value="ETH">ETH</option>');
});
});
describe("surface 3: the balance list", () => {
function respondWith(items) {
debugFetch.mockImplementation(async () => ({
ok: true,
status: 200,
statusText: "OK",
json: async () => items,
}));
}
function fakeEthItem(overrides = {}) {
return {
value: "5000000000000000",
token: {
type: "ERC-20",
address_hash: FAKE_ETH_CONTRACT,
symbol: "ETH",
name: "Ethereum",
decimals: "18",
holders_count: "900000",
...overrides,
},
};
}
beforeEach(() => {
debugFetch.mockReset();
});
// The bug in issue #235: this token cleared the balance list's own
// 1,000-holder floor and was listed as a holding named ETH.
test("a fake ETH token clearing the holder floor is filtered", async () => {
respondWith([fakeEthItem()]);
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
});
test("tracking the fake token manually does not admit it either", async () => {
respondWith([fakeEthItem({ holders_count: "0" })]);
const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, [
{ address: FAKE_ETH_CONTRACT },
]);
expect(balances).toEqual([]);
});
test("a genuine token keeps its place in the list", async () => {
respondWith([
fakeEthItem({
address_hash: USDC_CONTRACT,
symbol: "USDC",
name: "USD Coin",
decimals: "6",
}),
]);
const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, []);
expect(balances).toHaveLength(1);
expect(balances[0].symbol).toBe("USDC");
});
// The trap in this change: the user's real ETH balance is not an ERC-20
// and is fetched over RPC in refreshBalances, so it never passes through
// this loop at all. An explorer row that is not an ERC-20 is dropped
// before the symbol rule is consulted.
test("a non-ERC-20 row claiming ETH never reaches the symbol rule", async () => {
respondWith([fakeEthItem({ type: "ERC-721" })]);
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
});
// The money test: the user holds real ETH and has been airdropped a fake
// ETH ERC-20. The fake is gone from the list of tokens; the real balance
// is exactly what the node reported.
test("the real native ETH balance survives a fake ETH airdrop", async () => {
respondWith([fakeEthItem()]);
const addr = { address: HOLDER };
await refreshBalances(
[{ addresses: [addr] }],
"https://rpc.example.invalid",
BLOCKSCOUT,
[],
);
expect(addr.balance).toBe("1.2345");
expect(addr.tokenBalances).toEqual([]);
});
test("no test in this file performed a network request", () => {
expect(global.fetch).not.toHaveBeenCalled();
});
});

View File

@@ -1,4 +1,6 @@
const {
canRemoveAddress,
removeAddressFromState,
removeWalletFromState,
broadcastActiveChanged,
} = require("../src/shared/walletDelete");
@@ -6,6 +8,7 @@ const {
// Fixed addresses — never used for anything but these tests.
const A0 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const A1 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
const A2 = "0x514910771AF9Ca656af840dff83E8264EcF986CA";
const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599";
const C0 = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
@@ -111,6 +114,219 @@ describe("removeWalletFromState", () => {
});
});
// An HD wallet with three addresses next to a single-address key wallet.
// `nextIndex` is the wallet's derivation high-water mark, three addresses in.
function makeAddressState(overrides = {}) {
return {
hasWallet: true,
wallets: [
{ ...wallet("A", [A0, A1, A2]), type: "hd", nextIndex: 3 },
{ ...wallet("B", [B0]), type: "key" },
],
selectedWallet: 0,
selectedAddress: 0,
activeAddress: A0,
allowedSites: { [A0]: ["a.example"], [A1]: ["b.example"] },
deniedSites: { [A1]: ["d.example"], [B0]: ["e.example"] },
...overrides,
};
}
describe("canRemoveAddress", () => {
test("an HD wallet with more than one address may remove one", () => {
expect(canRemoveAddress({ type: "hd", addresses: [{}, {}] })).toBe(
true,
);
});
test("an xprv wallet with more than one address may too", () => {
expect(canRemoveAddress({ type: "xprv", addresses: [{}, {}] })).toBe(
true,
);
});
// The last address is what delete-wallet is for.
test("a wallet holding a single address may not", () => {
expect(canRemoveAddress({ type: "hd", addresses: [{}] })).toBe(false);
});
// A key wallet holds one bare private key and cannot derive more, so it
// has no "+" button and gets no remove control either.
test("a key wallet may not, whatever its address count", () => {
expect(canRemoveAddress({ type: "key", addresses: [{}] })).toBe(false);
expect(canRemoveAddress({ type: "key", addresses: [{}, {}] })).toBe(
false,
);
});
test("a missing or typeless wallet may not", () => {
expect(canRemoveAddress(undefined)).toBe(false);
expect(canRemoveAddress({})).toBe(false);
});
});
describe("removeAddressFromState", () => {
test("removing a non-selected address leaves the selection where it is", () => {
const state = makeAddressState({
selectedAddress: 2,
activeAddress: A2,
});
const { removed, activeAddressChanged } = removeAddressFromState(
state,
0,
0,
);
expect(removed).toBe(true);
// A2 moved from index 2 to index 1 by the splice.
expect(state.wallets[0].addresses.map((a) => a.address)).toEqual([
A1,
A2,
]);
expect(state.selectedWallet).toBe(0);
expect(state.selectedAddress).toBe(1);
expect(state.activeAddress).toBe(A2);
expect(activeAddressChanged).toBe(false);
// The wallet list itself is untouched.
expect(state.wallets).toHaveLength(2);
expect(state.hasWallet).toBe(true);
});
test("removing an address after the selection does not shift it", () => {
const state = makeAddressState({
selectedAddress: 0,
activeAddress: A0,
});
const { removed, activeAddressChanged } = removeAddressFromState(
state,
0,
2,
);
expect(removed).toBe(true);
expect(state.selectedAddress).toBe(0);
expect(state.activeAddress).toBe(A0);
expect(activeAddressChanged).toBe(false);
});
test("a selection in another wallet is untouched", () => {
const state = makeAddressState({
selectedWallet: 1,
selectedAddress: 0,
activeAddress: B0,
});
const { removed, activeAddressChanged } = removeAddressFromState(
state,
0,
1,
);
expect(removed).toBe(true);
expect(state.selectedWallet).toBe(1);
expect(state.selectedAddress).toBe(0);
expect(state.activeAddress).toBe(B0);
expect(activeAddressChanged).toBe(false);
});
test("removing the selected address falls back to the wallet's first address", () => {
const state = makeAddressState({
selectedAddress: 1,
activeAddress: A1,
});
const { removed, activeAddressChanged } = removeAddressFromState(
state,
0,
1,
);
expect(removed).toBe(true);
expect(state.wallets[0].addresses.map((a) => a.address)).toEqual([
A0,
A2,
]);
expect(state.selectedWallet).toBe(0);
expect(state.selectedAddress).toBe(0);
expect(state.activeAddress).toBe(A0);
expect(activeAddressChanged).toBe(true);
});
// The active address can be persisted in a different case than the
// wallet's copy of it, so the comparison must not be literal.
test("the active address is matched case-insensitively", () => {
const state = makeAddressState({
selectedAddress: 1,
activeAddress: A1.toLowerCase(),
});
const { activeAddressChanged } = removeAddressFromState(state, 0, 1);
expect(state.activeAddress).toBe(A0);
expect(activeAddressChanged).toBe(true);
});
test("site permissions are dropped for the removed address only", () => {
const state = makeAddressState();
removeAddressFromState(state, 0, 1);
expect(state.allowedSites).toEqual({ [A0]: ["a.example"] });
expect(state.deniedSites).toEqual({ [B0]: ["e.example"] });
});
// The derivation counter is a high-water mark, never rewound: "+" derives
// a fresh index rather than re-deriving the address just removed.
test("the wallet's derivation counter is not rewound", () => {
const state = makeAddressState();
removeAddressFromState(state, 0, 1);
expect(state.wallets[0].nextIndex).toBe(3);
});
test("the last address of a wallet is refused, and nothing changes", () => {
const state = makeAddressState({
selectedWallet: 1,
selectedAddress: 0,
activeAddress: B0,
});
const { removed, activeAddressChanged } = removeAddressFromState(
state,
1,
0,
);
expect(removed).toBe(false);
expect(activeAddressChanged).toBe(false);
expect(state.wallets[1].addresses.map((a) => a.address)).toEqual([B0]);
expect(state.activeAddress).toBe(B0);
expect(state.hasWallet).toBe(true);
});
// The same refusal reached the other way: an HD wallet worn down to one
// address is no more removable than a key wallet.
test("an HD wallet down to its last address is refused too", () => {
const state = makeAddressState();
expect(removeAddressFromState(state, 0, 2).removed).toBe(true);
expect(removeAddressFromState(state, 0, 1).removed).toBe(true);
expect(removeAddressFromState(state, 0, 0).removed).toBe(false);
expect(state.wallets[0].addresses.map((a) => a.address)).toEqual([A0]);
});
test("an out-of-range address index is refused", () => {
const state = makeAddressState();
expect(removeAddressFromState(state, 0, 7).removed).toBe(false);
expect(removeAddressFromState(state, 7, 0).removed).toBe(false);
expect(state.wallets[0].addresses).toHaveLength(3);
});
});
describe("broadcastActiveChanged", () => {
afterEach(() => {
delete global.chrome;