Compare commits

...

6 Commits

Author SHA1 Message Date
e94afc4c5e harden: verify all approval fields and make failed signing retryable (closes #174)
All checks were successful
check / check (push) Successful in 35s
verifySignedTx compared only from, to, value and data, so a signed
transaction could differ from the approval in chain id, nonce, gas limit
or any fee field and still be broadcast. Worse, it named the fields it
checked and so admitted every field it did not name: a type 4 artifact
carrying an EIP-7702 authorization passed verification, paying the
approved amount to the approved recipient and, in the same transaction,
permanently installing another contract's code at the signer's own
account.

The check is now an allowlist in both directions. The transaction type
must be 0, 1 or 2 — the only types this wallet signs — so no later
EIP-2718 type can bring a field along; authorizationList, blobs, blob
commitments and blob gas fees are refused by name; and the access list
is compared with the approval. Every consequential field is compared and
any mismatch refuses outright: the chain id against the selected network
(and against the approval when the page fixed one), plus nonce, gas
limit, gasPrice, maxFeePerGas and maxPriorityFeePerGas wherever the
approval carries a value, together with the fee mechanism the approval
implies. Fields the approval does not carry are populated locally by the
popup and have no approved value to compare against, so they are held to
absolute ceilings instead. Verification then closes by rebuilding the
transaction from exactly those checked fields and comparing the unsigned
bytes, so an artifact carrying anything this module does not account for
is refused without having to be named first.

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

A failed signing attempt also left a button that could not succeed: the
background deleted the approval before it broadcast, so a retry found
nothing to sign. The approval is now retired once the request has an
outcome, and the background tells the popup which stage failed. A popup
that could not sign is retryable; a mismatch spends the approval; a
failed broadcast is terminal, because the node may have accepted the
transaction and still failed to answer and the popup's retry re-signs at
a freshly fetched nonce rather than re-broadcasting the same bytes,
which would send the approved transfer twice.

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

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

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

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

The background's approval wiring had no tests, which is where these
defects lived. It has them now, driven through the real message listener
from eth_sendTransaction to broadcast, with windows.onRemoved captured
rather than stubbed away: each retirement path is asserted to leave a
mid-broadcast attempt alone and to still reject an approval no attempt
holds.
2026-08-12 09:18:32 +00: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
afe6ddaea0 fix: WaitTx timeout no longer overwrites a rendered success screen (closes #155)
All checks were successful
check / check (push) Successful in 30s
2026-08-12 10:58:36 +02:00
23712b53cb fix: wipe the exported private key from the DOM on any view leave (closes #221)
All checks were successful
check / check (push) Successful in 29s
2026-08-12 10:54:37 +02:00
32 changed files with 5242 additions and 306 deletions

154
README.md
View File

@@ -88,13 +88,21 @@ provide:
- `script/lint` — run the linter - `script/lint` — run the linter
- `script/fmt` — format all files (writes) - `script/fmt` — format all files (writes)
- `script/fmt-check` — check formatting (read-only) - `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 - `script/verify-build` — assert the compiled `DEBUG` state of the bundles in
`dist/`: every bundle containing `src/shared/constants.js` must have `DEBUG` `dist/`: every bundle containing `src/shared/constants.js` must have `DEBUG`
off, or on when `AUTISTMASK_DEBUG=1`. Run automatically at the end of 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 `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 cannot determine a bundle's state. Not part of `make check`, which does not
depend on build artifacts existing. 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/docker` — build the Docker image tagged via `script/projectname`
- `script/cibuild` — CI entrypoint: plain `docker build .` - `script/cibuild` — CI entrypoint: plain `docker build .`
- `script/precommit` — run by the git pre-commit hook; runs `script/check` - `script/precommit` — run by the git pre-commit hook; runs `script/check`
@@ -130,8 +138,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 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 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 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 land on it. It also covers address removal: which wallets offer the control at
from fixtures in `tests/e2e/network.js`, so the run is deterministic and fully 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 offline; unrecognised outbound requests are reported as failures rather than
silently allowed. silently allowed.
@@ -221,6 +232,7 @@ src/
prices.js — ETH/USD and token/USD via CoinDesk API prices.js — ETH/USD and token/USD via CoinDesk API
scamlist.js — known fraud contract addresses scamlist.js — known fraud contract addresses
state.js — persisted state (extension storage) 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) tokenList.js — top ERC-20 tokens by market cap (hardcoded)
transactions.js — tx history fetching + anti-poisoning filters transactions.js — tx history fetching + anti-poisoning filters
uniswap.js — Uniswap Universal Router calldata decoder uniswap.js — Uniswap Universal Router calldata decoder
@@ -450,11 +462,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 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 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 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 list from any other contract address is always dropped, and so is any token
unconditional — the "Hide tokens with fewer than 1,000 holders" setting governs claiming a symbol that belongs to the native asset and therefore has no
the transaction history and the send-screen token selector, not this list. legitimate contract at all (`"ETH"`). That filter is unconditional — the "Hide
Tracked tokens with a zero balance are listed as well while "Show tracked tokens tokens with fewer than 1,000 holders" setting governs the transaction history
with zero balance" is on. 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 #### Navigation
@@ -491,6 +504,14 @@ ExportPrivKey and ShowRecoveryPhrase — are deliberately absent from that list,
so the popup can never reopen onto one of them with no password prompt in front so the popup can never reopen onto one of them with no password prompt in front
of it. of it.
Every screen that holds secret material in the page registers a cleanup with
`onViewLeave()` (`src/popup/views/helpers.js`), which `showView()` runs on every
exit from that screen rather than only on its "Back" button, so nothing secret
survives in a hidden view once the user has navigated away by any route. That
covers the revealed private key and recovery phrase, the recovery phrase,
private key or extended private key entered on AddWallet, and the password typed
on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
#### Welcome (`welcome`) #### Welcome (`welcome`)
- **When**: No wallets exist yet (`state.hasWallet` is false). This is the root - **When**: No wallets exist yet (`state.hasWallet` is false). This is the root
@@ -513,8 +534,9 @@ of it.
- Wallet list: each wallet shows its name (tap to rename inline) and a "+" - 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 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 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 `[info]` button, an `[x]` button (only on HD and xprv wallets holding more
each token shown for that address 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 - "Recent Transactions": up to 25 transactions merged across every address
of every wallet, deduplicated by hash and filtered of every wallet, deduplicated by hash and filtered
- "Add additional wallet..." link at bottom - "Add additional wallet..." link at bottom
@@ -524,6 +546,7 @@ of it.
- Tap wallet name → inline rename field (no screen change) - Tap wallet name → inline rename field (no screen change)
- "+" on wallet → derives the next address inline (no screen change) - "+" on wallet → derives the next address inline (no screen change)
- `[info]` on address → **AddressDetail** - `[info]` on address → **AddressDetail**
- `[x]` on address → **DeleteAddress**
- "Send" → **Send** (refuses with a flash message on a zero balance) - "Send" → **Send** (refuses with a flash message on a zero balance)
- "Receive" → **Receive** (shows active address QR) - "Receive" → **Receive** (shows active address QR)
- Tap home tx row → **TransactionDetail** - Tap home tx row → **TransactionDetail**
@@ -601,10 +624,15 @@ of it.
- "Reveal" (correct password) → decrypts the wallet secret, derives this - "Reveal" (correct password) → decrypts the wallet secret, derives this
address's key, hides the password input and shows the key (no screen address's key, hides the password input and shows the key (no screen
change) change)
- "Reveal" (wrong password) → "Wrong password." on the error line, nothing - "Reveal" (wrong password) → full-sentence error on the error line, nothing
revealed revealed (no screen change)
- "Back" → clears the key and password from the DOM, then → previous screen - "Back" → previous screen (AddressDetail)
(AddressDetail) - **Secret handling**: nothing is decrypted, no key is derived, and nothing is
written into the page until the password is accepted; the key is never stored
in state, and it is wiped from the page whenever the screen is left by any
route, including the Settings gear. A decrypt still running when the screen is
left is discarded rather than written. The screen is not restorable, so
reopening the popup lands on Home rather than back on the key.
#### AddressToken (`address-token`) #### AddressToken (`address-token`)
@@ -693,10 +721,23 @@ of it.
- To: color dot + full address + etherscan link - To: color dot + full address + etherscan link
- Transaction hash: full hash (tap to copy) + etherscan link - Transaction hash: full hash (tap to copy) + etherscan link
- Count-up timer: "Waiting for confirmation... Ns" - Count-up timer: "Waiting for confirmation... Ns"
- **Behavior**: Polls `getTransactionReceipt` every 10 seconds. - **Behavior**: Polls `getTransactionReceipt` every 10 seconds. The wait is
persisted: closing and reopening the popup resumes the poll, with the elapsed
counter and the timeout deadline still measured from the original broadcast. A
lookup that fails is retried on the next tick rather than counted as a missing
receipt, because a failed lookup says nothing about the transaction; but six
failures in a row (60 seconds at the poll cadence) end the wait, so an RPC
that never answers cannot leave it running indefinitely. Any lookup that
answers resets that count.
- **Transitions**: - **Transitions**:
- Receipt found → **SuccessTx** - Receipt found → **SuccessTx**
- 60 seconds without confirmation → **ErrorTx** (timeout message) - A lookup that answers "no receipt" 60 seconds or more after broadcast →
**ErrorTx** (timeout message)
- Six consecutive failed lookups → **ErrorTx**, with a message naming the
unreachable network and pointing at the RPC URL in Settings. This is a
different fact from the timeout — the chain was never asked — and says so
- Exactly one outcome: a receipt found on the tick that crosses the deadline
wins, and no outcome can be rendered over another
#### SuccessTx (`success-tx`) #### SuccessTx (`success-tx`)
@@ -884,6 +925,55 @@ of it.
nothing deleted nothing deleted
- "Back" → previous screen (Settings) - "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`) #### SettingsAddToken (`settings-addtoken`)
- **When**: User tapped "+ Add token" in Settings. Tokens added here are tracked - **When**: User tapped "+ Add token" in Settings. Tokens added here are tracked
@@ -1246,14 +1336,15 @@ indexes it as a real token transfer.
that is the only thing that populates it. In the transaction history the check 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; 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 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 learned from them. The send-screen token selector and the balance list apply
unconditionally, because it decides which tokens the user can act on rather the same check unconditionally, because they decide which tokens the user can
than what the history displays. The balance list applies it unconditionally act on and what the user believes they own rather than what the history
too, but not identically: it exempts symbols that `KNOWN_SYMBOLS` maps to displays. All three surfaces read the rule from `src/shared/symbolSpoof.js`,
`null`, and `"ETH"` is the only one. So the fake "Ethereum" token above is so they cannot answer the question differently. A symbol the list maps to no
filtered from the transaction history and from the send selector, but a contract at all — `"ETH"`, the native asset, is the only one — may be borne by
fake-`ETH` ERC-20 that clears the balance list's own 1,000-holder floor — or no contract, so every ERC-20 claiming it is a spoof on all three. The user's
that the user tracked manually — is still shown in the balance list. 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 - **Low-holder token filtering**: Token transfers from ERC-20 contracts with
fewer than 1,000 holders are hidden from transaction history by default. fewer than 1,000 holders are hidden from transaction history by default.
@@ -1289,13 +1380,12 @@ indexes it as a real token transfer.
a sharp tool — users who understand the risks can configure the wallet to show a sharp tool — users who understand the risks can configure the wallet to show
everything unfiltered, unix-style. All four settings govern the transaction everything unfiltered, unix-style. All four settings govern the transaction
history; what else each one reaches varies. The known-symbol check also runs 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 unconditionally on the send-screen token selector and on the balance list, in
except for symbols mapped to `null` (`"ETH"` alone), which the balance list both cases identically to the history. The fraud contract blocklist is applied
does not filter. The fraud contract blocklist is applied unconditionally on unconditionally on that selector and is not consulted by the balance list at
that selector and is not consulted by the balance list at all. The low-holder all. The low-holder setting also gates the send selector, while the balance
setting also gates the send selector, while the balance list's own list's own 1,000-holder floor is unconditional (see Data Model). The dust
1,000-holder floor is unconditional (see Data Model). The dust threshold threshold applies to the transaction history alone.
applies to the transaction history alone.
#### Phishing Domain Protection #### Phishing Domain Protection
@@ -1367,7 +1457,7 @@ Currently supported:
### Wallet Management ### Wallet Management
- [x] Delete wallet (with confirmation) - [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) - [x] Show wallet's recovery phrase (requires password)
### Transactions ### Transactions

41
TODO.md
View File

@@ -44,6 +44,47 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 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
end the wait, reported as an unreachable network rather than as a timeout),
and the wait now resumes after a popup close
([#155](https://git.eeqj.de/sneak/AutistMask/issues/155)).
- 2026-08-12: The private key export screen now wipes the key from the page
whenever it is left by any route, and a decrypt still in flight when the
screen is left is discarded instead of written; the same `onViewLeave()`
cleanup was extended to every other screen holding secret material in the DOM
(AddWallet, ConfirmTx, DeleteWallet, ApproveTx, ApproveSign)
([#221](https://git.eeqj.de/sneak/AutistMask/issues/221)).
- 2026-08-12: An xprv wallet already in storage that was imported from a - 2026-08-12: An xprv wallet already in storage that was imported from a
non-master key is detected from the depth of its stored `xpub`, explained in non-master key is detected from the depth of its stored `xpub`, explained in
the wallet list, and blocked from signing, sending and private-key export the wallet list, and blocked from signing, sending and private-key export

View File

@@ -1,12 +1,13 @@
#!/bin/sh #!/bin/sh
# script/check: run all checks (test, lint, fmt-check). Our own # script/check: run all checks (test, test-verify-build, lint, fmt-check).
# extension to scripts-to-rule-them-all. Must not modify any files. # Our own extension to scripts-to-rule-them-all. Must not modify any files.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
main() { main() {
"$SCRIPT_DIR/test" "$SCRIPT_DIR/test"
"$SCRIPT_DIR/test-verify-build"
"$SCRIPT_DIR/lint" "$SCRIPT_DIR/lint"
"$SCRIPT_DIR/fmt-check" "$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"); } = require("../shared/state");
const { refreshBalances, getProvider } = require("../shared/balances"); const { refreshBalances, getProvider } = require("../shared/balances");
const { debugFetch, log } = require("../shared/log"); 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 { const {
isPhishingDomain, isPhishingDomain,
refreshPhishingListOnSchedule, 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. // Open approval in a separate popup window.
// This is the primary mechanism for tx/sign approvals (triggered programmatically, // This is the primary mechanism for tx/sign approvals (triggered programmatically,
// not from a user gesture) and the fallback for site-connection approvals. // not from a user gesture) and the fallback for site-connection approvals.
@@ -215,8 +273,7 @@ runtime.onConnect.addListener((port) => {
// Keep pending — user can reopen the toolbar popup // Keep pending — user can reopen the toolbar popup
return; return;
} }
approval.resolve({ approved: false, remember: false }); settleApproval(id, { approved: false, remember: false });
delete pendingApprovals[id];
} }
resetPopupUrl(); resetPopupUrl();
}); });
@@ -547,15 +604,21 @@ async function broadcastAccountsChanged() {
for (const key of Object.keys(connectedSites)) { for (const key of Object.keys(connectedSites)) {
delete connectedSites[key]; 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)) { for (const [id, approval] of Object.entries(pendingApprovals)) {
if (approval.type === "tx" || approval.type === "sign") { const rejection =
approval.resolve({ approval.type === "tx" || approval.type === "sign"
error: { code: 4001, message: "User rejected the request." }, ? {
}); error: {
} else { code: 4001,
approval.resolve({ approved: false, remember: false }); message: "User rejected the request.",
} },
}
: { approved: false, remember: false };
if (!settleApproval(id, rejection)) continue;
if (approval.windowId) { if (approval.windowId) {
windowsApi.remove(approval.windowId, () => { windowsApi.remove(approval.windowId, () => {
if (runtime.lastError) { if (runtime.lastError) {
@@ -563,7 +626,6 @@ async function broadcastAccountsChanged() {
} }
}); });
} }
delete pendingApprovals[id];
} }
resetPopupUrl(); resetPopupUrl();
const s = await getState(); const s = await getState();
@@ -679,23 +741,26 @@ if (runtime.onStartup) {
} }
startBackgroundJobs(); 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) { if (windowsApi && windowsApi.onRemoved) {
windowsApi.onRemoved.addListener((windowId) => { windowsApi.onRemoved.addListener((windowId) => {
for (const [id, approval] of Object.entries(pendingApprovals)) { for (const [id, approval] of Object.entries(pendingApprovals)) {
if (approval.windowId === windowId) { if (approval.windowId !== windowId) continue;
if (approval.type === "tx" || approval.type === "sign") { const rejection =
approval.resolve({ approval.type === "tx" || approval.type === "sign"
error: { ? {
code: 4001, error: {
message: "User rejected the request.", code: 4001,
}, message: "User rejected the request.",
}); },
} else { }
approval.resolve({ approved: false, remember: false }); : { approved: false, remember: false };
} settleApproval(id, rejection);
delete pendingApprovals[id];
}
} }
}); });
} }
@@ -761,14 +826,10 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
} }
if (msg.type === "AUTISTMASK_APPROVAL_RESPONSE") { if (msg.type === "AUTISTMASK_APPROVAL_RESPONSE") {
const approval = pendingApprovals[msg.id]; settleApproval(msg.id, {
if (approval) { approved: msg.approved,
approval.resolve({ remember: msg.remember,
approved: msg.approved, });
remember: msg.remember,
});
delete pendingApprovals[msg.id];
}
resetPopupUrl(); resetPopupUrl();
return false; return false;
} }
@@ -776,21 +837,50 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
if (msg.type === "AUTISTMASK_TX_RESPONSE") { if (msg.type === "AUTISTMASK_TX_RESPONSE") {
const approval = pendingApprovals[msg.id]; const approval = pendingApprovals[msg.id];
if (!approval) return false; if (!approval) return false;
delete pendingApprovals[msg.id];
resetPopupUrl();
// 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) { if (!msg.approved) {
approval.resolve({ if (
error: { code: 4001, message: "User rejected the request." }, !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; return true;
} }
// The popup signs; it reports back here when it could not. Fail the // The popup signs; it reports back here when it could not. Keep the
// request the same way this handler used to when it did the signing. // approval so the user can correct the problem and try again with the
// transaction they already saw.
if (msg.error) { if (msg.error) {
approval.resolve({ error: { message: msg.error } }); const outcome = describeTxFailure(TX_STAGE_SIGN, msg.error);
sendResponse({ error: 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; return false;
} }
@@ -800,22 +890,63 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
const activeAddress = await getActiveAddress(); const activeAddress = await getActiveAddress();
// The popup holds the secret, but the background stays the // The popup holds the secret, but the background stays the
// authority on what is broadcast: the raw transaction must be // authority on what is broadcast: the raw transaction must be
// the approved one, signed by the approved address. // the approved one, signed by the approved address, on the
// network that is selected.
verifySignedTx( verifySignedTx(
msg.rawSignedTx, msg.rawSignedTx,
approval.txParams, approval.txParams,
activeAddress, activeAddress,
currentNetwork().chainId,
); );
} catch (e) {
// A signed transaction that is not the approved one is not
// retried against that approval; it is refused outright.
// Anything else that failed before the check ran is the
// user's to retry.
const 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 provider = getProvider(state.rpcUrl);
const tx = await provider.broadcastTransaction(msg.rawSignedTx); const tx = await provider.broadcastTransaction(msg.rawSignedTx);
approval.resolve({ txHash: tx.hash }); settleApproval(
msg.id,
{ txHash: tx.hash },
{ holdsClaim: true },
);
sendResponse({ txHash: tx.hash }); sendResponse({ txHash: tx.hash });
} catch (e) { } catch (e) {
const errMsg = e.shortMessage || e.message; // Terminal, never retried: the node may have accepted the
approval.resolve({ // transaction and still failed to answer, and the popup's
error: { message: errMsg }, // 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; return true;
@@ -824,21 +955,43 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
if (msg.type === "AUTISTMASK_SIGN_RESPONSE") { if (msg.type === "AUTISTMASK_SIGN_RESPONSE") {
const approval = pendingApprovals[msg.id]; const approval = pendingApprovals[msg.id];
if (!approval) return false; if (!approval) return false;
delete pendingApprovals[msg.id];
resetPopupUrl();
// Same as the transaction path: a reject cannot retire an approval an
// attempt already holds.
if (!msg.approved) { if (!msg.approved) {
approval.resolve({ if (
error: { code: 4001, message: "User rejected the request." }, !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; return true;
} }
// The popup signs; it reports back here when it could not. Fail the // The popup signs; it reports back here when it could not. Keep the
// request the same way this handler used to when it did the signing. // approval so the user can correct the problem and try again with the
// message they already saw.
if (msg.error) { if (msg.error) {
approval.resolve({ error: { message: msg.error } }); sendResponse({ error: msg.error, retryable: true });
sendResponse({ error: msg.error }); return false;
}
// 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; return false;
} }
@@ -851,14 +1004,21 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
// address. // address.
const signature = msg.signature; const signature = msg.signature;
verifySignature(approval.signParams, signature, activeAddress); verifySignature(approval.signParams, signature, activeAddress);
approval.resolve({ signature }); settleApproval(msg.id, { signature }, { holdsClaim: true });
sendResponse({ signature }); sendResponse({ signature });
} catch (e) { } catch (e) {
const errMsg = e.shortMessage || e.message; const errMsg = e.shortMessage || e.message;
approval.resolve({ const retryable = failureIsRetryable(e);
error: { message: errMsg }, if (!retryable) {
}); settleApproval(
sendResponse({ error: errMsg }); msg.id,
{ error: { message: errMsg } },
{ holdsClaim: true },
);
} else {
releaseApproval(approval);
}
sendResponse({ error: errMsg, retryable });
} }
})(); })();
return true; return true;

View File

@@ -1142,6 +1142,62 @@
</button> </button>
</div> </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 ============ --> <!-- ============ SHOW RECOVERY PHRASE ============ -->
<div id="view-show-phrase" class="view hidden"> <div id="view-show-phrase" class="view hidden">
<button <button

View File

@@ -33,6 +33,7 @@ const receive = require("./views/receive");
const addToken = require("./views/addToken"); const addToken = require("./views/addToken");
const settings = require("./views/settings"); const settings = require("./views/settings");
const settingsAddToken = require("./views/settingsAddToken"); const settingsAddToken = require("./views/settingsAddToken");
const deleteAddress = require("./views/deleteAddress");
const approval = require("./views/approval"); const approval = require("./views/approval");
function renderWalletList() { function renderWalletList() {
@@ -101,6 +102,10 @@ const ctx = {
pushCurrentView(); pushCurrentView();
settingsAddToken.show(); settingsAddToken.show();
}, },
showDeleteAddress: (walletIdx, addrIdx) => {
pushCurrentView();
deleteAddress.show(walletIdx, addrIdx);
},
}; };
function needsAddress(view) { function needsAddress(view) {
@@ -165,6 +170,12 @@ function restoreView() {
fallbackView(); fallbackView();
} }
break; break;
case "wait-tx":
// Resumes the receipt poll from the persisted broadcast time.
if (!txStatus.restoreWait()) {
fallbackView();
}
break;
case "success-tx": case "success-tx":
if (state.viewData && state.viewData.hash) { if (state.viewData && state.viewData.hash) {
txStatus.renderSuccess(); txStatus.renderSuccess();
@@ -250,6 +261,7 @@ async function init() {
addToken.init(ctx); addToken.init(ctx);
settings.init(ctx); settings.init(ctx);
settingsAddToken.init(ctx); settingsAddToken.init(ctx);
deleteAddress.init(ctx);
if (!state.hasWallet) { if (!state.hasWallet) {
showView("welcome"); showView("welcome");

View File

@@ -22,6 +22,7 @@ const RESTORABLE_VIEWS = new Set([
"settings-addtoken", "settings-addtoken",
"confirm-tx", "confirm-tx",
"transaction", "transaction",
"wait-tx",
"success-tx", "success-tx",
"error-tx", "error-tx",
]); ]);

View File

@@ -1,4 +1,11 @@
const { $, showView, showFlash, goBack, clearViewStack } = require("./helpers"); const {
$,
showView,
showFlash,
goBack,
clearViewStack,
onViewLeave,
} = require("./helpers");
const { const {
generateMnemonic, generateMnemonic,
hdWalletFromMnemonic, hdWalletFromMnemonic,
@@ -66,13 +73,23 @@ function switchMode(mode) {
$("add-wallet-password-hint").textContent = PASSWORD_HINTS[mode]; $("add-wallet-password-hint").textContent = PASSWORD_HINTS[mode];
} }
function show() { // Wipe the secret material this screen holds in the DOM: a generated or
// pasted recovery phrase, an imported private key or extended private key,
// and the password that would encrypt them. Registered as the view-leave
// handler as well as run on entry, so none of it survives in the hidden
// view after the user navigates away by any route, including the Settings
// gear and the import itself.
function clear() {
$("wallet-mnemonic").value = ""; $("wallet-mnemonic").value = "";
$("import-private-key").value = ""; $("import-private-key").value = "";
$("import-xprv-key").value = ""; $("import-xprv-key").value = "";
$("add-wallet-password").value = ""; $("add-wallet-password").value = "";
$("add-wallet-password-confirm").value = ""; $("add-wallet-password-confirm").value = "";
$("add-wallet-phrase-warning").style.visibility = "hidden"; $("add-wallet-phrase-warning").style.visibility = "hidden";
}
function show() {
clear();
switchMode("mnemonic"); switchMode("mnemonic");
showView("add-wallet"); showView("add-wallet");
} }
@@ -288,6 +305,8 @@ async function importXprvKey(ctx) {
} }
function init(ctx) { function init(ctx) {
onViewLeave("add-wallet", clear);
// Tab click handlers // Tab click handlers
$("tab-mnemonic").addEventListener("click", () => switchMode("mnemonic")); $("tab-mnemonic").addEventListener("click", () => switchMode("mnemonic"));
$("tab-privkey").addEventListener("click", () => switchMode("privkey")); $("tab-privkey").addEventListener("click", () => switchMode("privkey"));

View File

@@ -2,7 +2,6 @@ const {
$, $,
showView, showView,
showFlash, showFlash,
flashCopyFeedback,
balanceLinesForAddress, balanceLinesForAddress,
addressDotHtml, addressDotHtml,
addressTitle, addressTitle,
@@ -27,8 +26,7 @@ const {
} = require("./send"); } = require("./send");
const { log } = require("../../shared/log"); const { log } = require("../../shared/log");
const makeBlockie = require("ethereum-blockies-base64"); const makeBlockie = require("ethereum-blockies-base64");
const { decryptWithPassword } = require("../../shared/vault"); const exportPrivkey = require("./exportPrivkey");
const { getSignerForAddress } = require("../../shared/wallet");
const { walletDefect } = require("../../shared/walletDefects"); const { walletDefect } = require("../../shared/walletDefects");
// The defect of the wallet the selected address belongs to, or null. Both the // The defect of the wallet the selected address belongs to, or null. Both the
@@ -321,81 +319,12 @@ function init(_ctx) {
showFlash(defect.shortMessage); showFlash(defect.shortMessage);
return; return;
} }
pushCurrentView(); // No pushCurrentView() here: exportPrivkey.show() can return
const wallet = state.wallets[state.selectedWallet]; // without navigating, so it does its own push.
const addr = wallet.addresses[state.selectedAddress]; exportPrivkey.show(state.selectedWallet, state.selectedAddress);
const blockieEl = $("export-privkey-jazzicon");
blockieEl.innerHTML = "";
const bImg = document.createElement("img");
bImg.src = makeBlockie(addr.address);
bImg.width = 48;
bImg.height = 48;
bImg.style.imageRendering = "pixelated";
bImg.style.borderRadius = "50%";
blockieEl.appendChild(bImg);
$("export-privkey-title").textContent =
wallet.name + " \u2014 Address " + (state.selectedAddress + 1);
const exportAddrContainer = $("export-privkey-dot").parentElement;
exportAddrContainer.innerHTML = renderAddressHtml(addr.address);
attachCopyHandlers(exportAddrContainer);
$("export-privkey-password").value = "";
$("export-privkey-flash").textContent = "";
$("export-privkey-flash").style.visibility = "hidden";
$("export-privkey-password-section").classList.remove("hidden");
$("export-privkey-result").classList.add("hidden");
$("export-privkey-value").textContent = "";
showView("export-privkey");
}); });
$("btn-export-privkey-confirm").addEventListener("click", async () => { exportPrivkey.init();
const password = $("export-privkey-password").value;
if (!password) {
$("export-privkey-flash").textContent = "Password is required.";
$("export-privkey-flash").style.visibility = "visible";
return;
}
const btn = $("btn-export-privkey-confirm");
btn.disabled = true;
btn.classList.add("text-muted");
const wallet = state.wallets[state.selectedWallet];
try {
const secret = await decryptWithPassword(
wallet.encryptedSecret,
password,
);
const signer = getSignerForAddress(
wallet,
state.selectedAddress,
secret,
);
const privateKey = signer.privateKey;
$("export-privkey-password-section").classList.add("hidden");
$("export-privkey-value").textContent = privateKey;
$("export-privkey-result").classList.remove("hidden");
$("export-privkey-flash").style.visibility = "hidden";
} catch {
$("export-privkey-flash").textContent = "Wrong password.";
$("export-privkey-flash").style.visibility = "visible";
} finally {
btn.disabled = false;
btn.classList.remove("text-muted");
}
});
$("export-privkey-value").addEventListener("click", () => {
const key = $("export-privkey-value").textContent;
if (key) {
navigator.clipboard.writeText(key);
showFlash("Copied!");
flashCopyFeedback($("export-privkey-value"));
}
});
$("btn-export-privkey-back").addEventListener("click", () => {
$("export-privkey-value").textContent = "";
$("export-privkey-password").value = "";
goBack();
});
} }
module.exports = { init, show }; module.exports = { init, show };

View File

@@ -7,6 +7,7 @@ const {
hideError, hideError,
renderAddressHtml, renderAddressHtml,
attachCopyHandlers, attachCopyHandlers,
onViewLeave,
} = require("./helpers"); } = require("./helpers");
const { state, saveState, currentNetwork } = require("../../shared/state"); const { state, saveState, currentNetwork } = require("../../shared/state");
const { const {
@@ -23,6 +24,7 @@ const { decryptWithPassword } = require("../../shared/vault");
const { getSignerForAddress } = require("../../shared/wallet"); const { getSignerForAddress } = require("../../shared/wallet");
const { walletDefect } = require("../../shared/walletDefects"); const { walletDefect } = require("../../shared/walletDefects");
const { getProvider } = require("../../shared/balances"); const { getProvider } = require("../../shared/balances");
const { describeSigningFailure } = require("../../shared/approvalVerify");
const txStatus = require("./txStatus"); const txStatus = require("./txStatus");
const uniswap = require("../../shared/uniswap"); const uniswap = require("../../shared/uniswap");
const runtime = const runtime =
@@ -461,7 +463,24 @@ function findActiveWallet() {
return null; return null;
} }
// Drop the password from the DOM when either approval screen is left. The
// approval window navigates on after a signature — approve-tx goes to the
// wait screen — and the password must not sit in the hidden view for the
// life of that window.
function clearTxPassword() {
$("approve-tx-password").value = "";
hideError("approve-tx-error");
}
function clearSignPassword() {
$("approve-sign-password").value = "";
hideError("approve-sign-error");
}
function init(ctx) { function init(ctx) {
onViewLeave("approve-tx", clearTxPassword);
onViewLeave("approve-sign", clearSignPassword);
$("approve-remember").addEventListener("change", async () => { $("approve-remember").addEventListener("change", async () => {
state.rememberSiteChoice = $("approve-remember").checked; state.rememberSiteChoice = $("approve-remember").checked;
await saveState(); await saveState();
@@ -571,10 +590,20 @@ function init(ctx) {
runtime.sendMessage(payload, (response) => { runtime.sendMessage(payload, (response) => {
if (response && response.txHash) { if (response && response.txHash) {
txStatus.showWait(pendingTxDetails, response.txHash); txStatus.showWait(pendingTxDetails, response.txHash);
return;
}
// A retryable failure leaves the approval pending in the
// background, so stay on this screen with a live button rather
// than sending the user to a dead end.
const outcome = describeSigningFailure(
response,
"The transaction could not be sent.",
);
if (outcome.retryable) {
showError("approve-tx-error", outcome.message);
setTxButtonBusy(false);
} else { } else {
const msg = txStatus.showError(pendingTxDetails, null, outcome.message);
(response && response.error) || "Transaction failed.";
txStatus.showError(pendingTxDetails, null, msg);
} }
}); });
}); });
@@ -677,11 +706,18 @@ function init(ctx) {
runtime.sendMessage(payload, (response) => { runtime.sendMessage(payload, (response) => {
if (response && response.signature) { if (response && response.signature) {
window.close(); window.close();
} else { return;
const msg = (response && response.error) || "Signing failed.";
showError("approve-sign-error", msg);
setSignButtonBusy(false);
} }
// The button comes back only when the approval is still pending in
// the background; otherwise it stays disabled and the message says
// why, because a control that cannot succeed must not look like it
// can.
const outcome = describeSigningFailure(
response,
"The message could not be signed.",
);
showError("approve-sign-error", outcome.message);
if (outcome.retryable) setSignButtonBusy(false);
}); });
}); });

View File

@@ -21,6 +21,7 @@ const {
renderAddressHtml, renderAddressHtml,
attachCopyHandlers, attachCopyHandlers,
goBack, goBack,
onViewLeave,
} = require("./helpers"); } = require("./helpers");
const { state, currentNetwork } = require("../../shared/state"); const { state, currentNetwork } = require("../../shared/state");
const { getSignerForAddress } = require("../../shared/wallet"); const { getSignerForAddress } = require("../../shared/wallet");
@@ -390,7 +391,17 @@ async function checkRecipientHistory(txInfo) {
} }
} }
// Drop the password from the DOM. Registered as the view-leave handler so
// it does not sit in the hidden view once the screen navigates on — to the
// wait screen after a send, or anywhere else the user goes.
function clearPassword() {
$("confirm-tx-password").value = "";
hideError("confirm-tx-password-error");
}
function init(ctx) { function init(ctx) {
onViewLeave("confirm-tx", clearPassword);
$("btn-confirm-send").addEventListener("click", async () => { $("btn-confirm-send").addEventListener("click", async () => {
const password = $("confirm-tx-password").value; const password = $("confirm-tx-password").value;
if (!password) { if (!password) {

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

@@ -1,4 +1,11 @@
const { $, showView, showFlash, goBack, clearViewStack } = require("./helpers"); const {
$,
showView,
showFlash,
goBack,
clearViewStack,
onViewLeave,
} = require("./helpers");
const { state, saveState } = require("../../shared/state"); const { state, saveState } = require("../../shared/state");
const { decryptWithPassword } = require("../../shared/vault"); const { decryptWithPassword } = require("../../shared/vault");
const { const {
@@ -9,22 +16,34 @@ const {
let deleteWalletIndex = null; let deleteWalletIndex = null;
let ctx = null; let ctx = null;
// Drop the password from the DOM and the wallet selection from the
// closure. Registered as the view-leave handler as well as run on entry,
// so the typed password does not sit in the hidden view after the user
// navigates away by any route, including the Settings gear.
function clear() {
deleteWalletIndex = null;
$("delete-wallet-password").value = "";
$("delete-wallet-flash").textContent = "";
$("delete-wallet-flash").style.visibility = "hidden";
}
function show(walletIdx) { function show(walletIdx) {
clear();
deleteWalletIndex = walletIdx; deleteWalletIndex = walletIdx;
const wallet = state.wallets[walletIdx]; const wallet = state.wallets[walletIdx];
$("delete-wallet-name").textContent = $("delete-wallet-name").textContent =
wallet.name || "Wallet " + (walletIdx + 1); wallet.name || "Wallet " + (walletIdx + 1);
$("delete-wallet-password").value = "";
$("delete-wallet-flash").textContent = "";
$("delete-wallet-flash").style.visibility = "hidden";
showView("delete-wallet-confirm"); showView("delete-wallet-confirm");
} }
function init(_ctx) { function init(_ctx) {
ctx = _ctx; ctx = _ctx;
onViewLeave("delete-wallet-confirm", clear);
// No wipe here: goBack() routes through showView(), which runs the
// leave hook.
$("btn-delete-wallet-back").addEventListener("click", () => { $("btn-delete-wallet-back").addEventListener("click", () => {
deleteWalletIndex = null;
goBack(); goBack();
}); });

View File

@@ -0,0 +1,174 @@
// Private key export for a single address.
//
// The key controls the address outright — anyone holding it can move every
// token in it, from any device, forever — so this screen is handled under
// the same rules as the recovery phrase screen (./showPhrase.js):
//
// 1. Nothing is decrypted, no key is derived, and nothing is written into
// the DOM until decryptWithPassword has accepted the password.
// 2. Leaving the screen by any path wipes it, via the onViewLeave hook,
// and a decrypt still in flight when that happens is discarded
// instead of written (revealGeneration).
// 3. The key never reaches the logger. This module deliberately does not
// import src/shared/log.js.
//
// The key is also never assigned to `state`, so it cannot be persisted to
// extension storage, and "export-privkey" is excluded from RESTORABLE_VIEWS
// so the popup can never reopen onto it.
const {
$,
showView,
showFlash,
flashCopyFeedback,
goBack,
onViewLeave,
pushCurrentView,
renderAddressHtml,
attachCopyHandlers,
} = require("./helpers");
const { state } = require("../../shared/state");
const { decryptWithPassword } = require("../../shared/vault");
const { getSignerForAddress } = require("../../shared/wallet");
const makeBlockie = require("ethereum-blockies-base64");
const VIEW = "export-privkey";
let walletIndex = null;
let addressIndex = null;
// Bumped by every clear(), which is what leaving the screen runs. reveal()
// captures it before awaiting the decrypt and refuses to touch the DOM if
// it has moved: a decrypt still in flight when the screen is left would
// otherwise write the key *after* the wipe, with nothing scheduled to wipe
// it again, leaving it in the hidden view for the life of the popup.
let revealGeneration = 0;
// True only if the reveal that captured `generation` is still the live one:
// the screen has not been left, cleared, or re-entered for another address
// since it started.
function isCurrentReveal(generation) {
return (
generation === revealGeneration &&
walletIndex !== null &&
addressIndex !== null &&
state.currentView === VIEW
);
}
function fail(message) {
$("export-privkey-flash").textContent = message;
$("export-privkey-flash").style.visibility = "visible";
}
// Wipe every trace of the key and drop the address selection. Safe to call
// when nothing was ever revealed, and safe to call twice.
function clear() {
walletIndex = null;
addressIndex = null;
revealGeneration += 1;
$("export-privkey-value").textContent = "";
$("export-privkey-password").value = "";
$("export-privkey-result").classList.add("hidden");
$("export-privkey-password-section").classList.remove("hidden");
$("export-privkey-flash").textContent = "";
$("export-privkey-flash").style.visibility = "hidden";
}
function show(walletIdx, addrIdx) {
const wallet = state.wallets[walletIdx];
const addr = wallet && wallet.addresses[addrIdx];
if (!addr) {
showFlash("That address is no longer available.");
return;
}
clear();
walletIndex = walletIdx;
addressIndex = addrIdx;
const blockieEl = $("export-privkey-jazzicon");
blockieEl.innerHTML = "";
const img = document.createElement("img");
img.src = makeBlockie(addr.address);
img.width = 48;
img.height = 48;
img.style.imageRendering = "pixelated";
img.style.borderRadius = "50%";
blockieEl.appendChild(img);
$("export-privkey-title").textContent =
wallet.name + " — Address " + (addrIdx + 1);
const addrContainer = $("export-privkey-dot").parentElement;
addrContainer.innerHTML = renderAddressHtml(addr.address);
attachCopyHandlers(addrContainer);
// Pushed here rather than by the caller: this function can return
// without navigating, and a push that happened anyway would leave an
// entry on the stack that no screen transition matches.
pushCurrentView();
showView(VIEW);
}
async function reveal() {
const password = $("export-privkey-password").value;
if (!password) {
fail("Password is required.");
return;
}
if (walletIndex === null) {
fail("No address is selected.");
return;
}
const wallet = state.wallets[walletIndex];
const btn = $("btn-export-privkey-confirm");
btn.disabled = true;
btn.classList.add("text-muted");
const generation = revealGeneration;
try {
const secret = await decryptWithPassword(
wallet.encryptedSecret,
password,
);
// The only suspension point in this view, and the gate on the only
// place a secret is written: if the screen was left while the
// decrypt ran, the wipe has already happened, so the key is not
// even derived, let alone written.
if (!isCurrentReveal(generation)) return;
const signer = getSignerForAddress(wallet, addressIndex, secret);
$("export-privkey-password").value = "";
$("export-privkey-password-section").classList.add("hidden");
$("export-privkey-value").textContent = signer.privateKey;
$("export-privkey-result").classList.remove("hidden");
$("export-privkey-flash").textContent = "";
$("export-privkey-flash").style.visibility = "hidden";
} catch {
if (!isCurrentReveal(generation)) return;
fail("That password is not correct. Please try again.");
} finally {
btn.disabled = false;
btn.classList.remove("text-muted");
}
}
function init() {
onViewLeave(VIEW, clear);
// No wipe here: goBack() routes through showView(), which runs the
// leave hook. A per-button wipe would only cover this one path.
$("btn-export-privkey-back").addEventListener("click", () => {
goBack();
});
$("btn-export-privkey-confirm").addEventListener("click", reveal);
$("export-privkey-value").addEventListener("click", () => {
const key = $("export-privkey-value").textContent;
if (!key) return;
navigator.clipboard.writeText(key);
showFlash("Copied!");
flashCopyFeedback($("export-privkey-value"));
});
}
module.exports = { init, show };

View File

@@ -25,6 +25,7 @@ const VIEWS = [
"add-token", "add-token",
"settings", "settings",
"delete-wallet-confirm", "delete-wallet-confirm",
"delete-address-confirm",
"settings-addtoken", "settings-addtoken",
"transaction", "transaction",
"approve-site", "approve-site",
@@ -217,6 +218,20 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
return html; 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 "…". // Truncate the middle of a string, replacing removed characters with "…".
// Safety: refuses to truncate more than 10 characters, which is the maximum // Safety: refuses to truncate more than 10 characters, which is the maximum
// that still prevents address spoofing attacks (see Display Consistency in // that still prevents address spoofing attacks (see Display Consistency in
@@ -463,6 +478,7 @@ module.exports = {
flashCopyFeedback, flashCopyFeedback,
balanceLine, balanceLine,
balanceLinesForAddress, balanceLinesForAddress,
addressHoldsFunds,
addressColor, addressColor,
addressDotHtml, addressDotHtml,
escapeHtml, escapeHtml,

View File

@@ -21,6 +21,7 @@ const {
resetSendValidation, resetSendValidation,
} = require("./send"); } = require("./send");
const { deriveAddressFromXpub } = require("../../shared/wallet"); const { deriveAddressFromXpub } = require("../../shared/wallet");
const { canRemoveAddress } = require("../../shared/walletDelete");
const { const {
walletDefect, walletDefect,
walletDefectHtml, 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}">`; 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 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>`; 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 dot = addressDotHtml(addr.address);
const titleBold = isActive ? "font-bold" : ""; const titleBold = isActive ? "font-bold" : "";
html += `<div class="text-xs ${titleBold}">Address ${ai + 1}</div>`; 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 += `<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 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>`; html += `</div>`;
const addrUsd = formatUsd(getAddressValueUsd(addr)); const addrUsd = formatUsd(getAddressValueUsd(addr));
html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrUsd || "&nbsp;"}</div>`; 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) => { container.querySelectorAll(".btn-add-address").forEach((btn) => {
btn.addEventListener("click", async (e) => { btn.addEventListener("click", async (e) => {
e.stopPropagation(); e.stopPropagation();

View File

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

View File

@@ -16,11 +16,36 @@ const { state, saveState, currentNetwork } = require("../../shared/state");
const { getProvider } = require("../../shared/balances"); const { getProvider } = require("../../shared/balances");
const { log } = require("../../shared/log"); const { log } = require("../../shared/log");
// Receipt poll cadence and the deadline after which the wait is reported as
// a timeout. Both are documented in the WaitTx section of README.md.
const POLL_INTERVAL_MS = 10000;
const TIMEOUT_MS = 60000;
// How many receipt lookups may fail in a row before the wait is ended and
// the failure reported. A lookup that throws says nothing about the
// transaction, so one must not end the wait — but an RPC that never answers
// (a mistyped URL in settings is the ordinary case) must not leave the wait
// running forever either, least of all a persisted one that every popup
// open would resume. Six is 60 seconds at the poll cadence: the same
// patience the confirmation deadline gets. Any lookup that answers, with a
// receipt or with null, resets the count.
const MAX_CONSECUTIVE_LOOKUP_FAILURES = 6;
let ctx; let ctx;
let elapsedTimer = null; let elapsedTimer = null;
let pollTimer = null; let pollTimer = null;
function clearTimers() { // Identifies the wait currently on screen. Bumped by endWait(), so a timer
// callback or an in-flight receipt lookup that outlives its wait can tell
// that it is stale and leave the current view alone. Without it, a receipt
// resolving after the wait has ended renders over whatever view replaced it.
let waitId = 0;
// End the wait on screen: stop its timers and invalidate its pending async
// work. Called on receipt, on timeout, when a new wait starts, and when the
// user navigates away.
function endWait() {
waitId++;
if (elapsedTimer) { if (elapsedTimer) {
clearInterval(elapsedTimer); clearInterval(elapsedTimer);
elapsedTimer = null; elapsedTimer = null;
@@ -47,8 +72,13 @@ function blockNumberHtml(blockNumber) {
return copyableHtml(num) + etherscanLinkHtml(link); return copyableHtml(num) + etherscanLinkHtml(link);
} }
function showWait(txInfo, txHash) { // Render the wait view and start polling for the receipt. broadcastTime is
clearTimers(); // when the transaction was broadcast, which is what the elapsed counter and
// the timeout deadline are both measured from; pollNow runs one lookup
// immediately instead of waiting a full poll interval.
function startWait(txInfo, txHash, broadcastTime, pollNow) {
endWait();
const id = waitId;
const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?"; const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?";
$("wait-tx-summary").textContent = txInfo.amount + " " + symbol; $("wait-tx-summary").textContent = txInfo.amount + " " + symbol;
@@ -56,41 +86,130 @@ function showWait(txInfo, txHash) {
$("wait-tx-hash").innerHTML = txHashHtml(txHash); $("wait-tx-hash").innerHTML = txHashHtml(txHash);
attachCopyHandlers("view-wait-tx"); attachCopyHandlers("view-wait-tx");
const broadcastTime = Date.now(); // Persisted so closing and reopening the popup resumes this wait
$("wait-tx-status").textContent = "Waiting for confirmation... 0s"; // instead of silently abandoning it.
state.viewData = {
pendingWait: {
txInfo: txInfo,
hash: txHash,
broadcastTime: broadcastTime,
},
};
elapsedTimer = setInterval(() => { function renderElapsed() {
const elapsed = Math.floor((Date.now() - broadcastTime) / 1000); const elapsed = Math.floor((Date.now() - broadcastTime) / 1000);
$("wait-tx-status").textContent = $("wait-tx-status").textContent =
"Waiting for confirmation... " + elapsed + "s"; "Waiting for confirmation... " + elapsed + "s";
}
renderElapsed();
elapsedTimer = setInterval(() => {
if (id !== waitId) return;
renderElapsed();
}, 1000); }, 1000);
const provider = getProvider(state.rpcUrl); const provider = getProvider(state.rpcUrl);
pollTimer = setInterval(async () => { let consecutiveFailures = 0;
async function poll() {
if (id !== waitId) return;
let receipt = null;
let answered = true;
try { try {
const receipt = await provider.getTransactionReceipt(txHash); receipt = await provider.getTransactionReceipt(txHash);
if (receipt) {
showSuccess(txInfo, txHash, receipt.blockNumber);
}
} catch (e) { } catch (e) {
// A thrown lookup means "no answer this tick", not "no
// receipt": the RPC failed, the chain said nothing. Declaring
// the timeout off it would report a confirmed transaction as
// failed — which matters most on a resumed wait, where the
// first poll is already past the deadline.
answered = false;
log.errorf("poll receipt failed:", e.message); log.errorf("poll receipt failed:", e.message);
} }
// The lookup is async: the wait may have ended while it was in
const elapsed = Math.floor((Date.now() - broadcastTime) / 1000); // flight, in which case this result must not touch the view.
if (elapsed >= 60) { if (id !== waitId) return;
// Exactly one outcome per wait. A receipt wins even on the tick
// that crosses the deadline, because the transaction did confirm.
if (receipt) {
showSuccess(txInfo, txHash, receipt.blockNumber);
return;
}
if (!answered) {
consecutiveFailures++;
// The failure is the user's news, and it is a different fact
// from "the transaction did not confirm" — the chain was never
// asked. Ending the wait here is what keeps it bounded and
// gives the user a Done button to leave by.
if (consecutiveFailures >= MAX_CONSECUTIVE_LOOKUP_FAILURES) {
showError(
txInfo,
txHash,
"The network could not be reached to check this transaction — " +
MAX_CONSECUTIVE_LOOKUP_FAILURES +
" lookups failed in a row. Check the RPC URL in Settings. The transaction may still have confirmed — check Etherscan.",
);
}
// Otherwise keep polling: the next tick may answer.
return;
}
consecutiveFailures = 0;
if (Date.now() - broadcastTime >= TIMEOUT_MS) {
showError( showError(
txInfo, txInfo,
txHash, txHash,
"Transaction was not confirmed within 60 seconds. It may still confirm later \u2014 check Etherscan.", "Transaction was not confirmed within 60 seconds. It may still confirm later \u2014 check Etherscan.",
); );
} }
}, 10000); }
pollTimer = setInterval(poll, POLL_INTERVAL_MS);
showView("wait-tx"); showView("wait-tx");
if (pollNow) poll();
}
function showWait(txInfo, txHash) {
startWait(txInfo, txHash, Date.now(), false);
}
// Resume a wait persisted by a previous popup session. The deadline still
// runs from the original broadcast, so a wait that has already outlived it
// resolves on the immediate first poll rather than restarting the clock.
// Returns false when there is nothing resumable to resume. Every field
// startWait() goes on to use is validated, not just the presence of the
// containers: txInfo.to reaches addressTitle(), which calls
// address.toLowerCase(), and txInfo.amount is rendered into the summary, so
// an object merely missing one of them throws a TypeError out of
// restoreView() — which init() does not guard, skipping the rest of popup
// init and leaving wait-tx on screen with no back control. A non-numeric
// broadcastTime leaves an unexitable wait counting "NaNs". txInfo.token and
// txInfo.tokenSymbol are deliberately unchecked: they are compared and
// coalesced rather than dereferenced, and tokenSymbol is null for ETH.
function restoreWait() {
const d = state.viewData;
if (!d || !d.pendingWait) return false;
const w = d.pendingWait;
if (!w.hash) return false;
// typeof [] is "object", so an array passes an object check.
const info = w.txInfo;
if (!info || typeof info !== "object" || Array.isArray(info)) return false;
// A string is the whole requirement: the empty string is what a
// contract-deployment approval persists (approval.js writes `to: toAddr
// || ""`), and both fields render harmlessly when empty, so refusing it
// would abandon a wait the live path itself created.
if (typeof info.to !== "string") return false;
if (typeof info.amount !== "string") return false;
if (typeof w.broadcastTime !== "number" || !isFinite(w.broadcastTime)) {
return false;
}
startWait(w.txInfo, w.hash, w.broadcastTime, true);
return true;
} }
function showSuccess(txInfo, txHash, blockNumber) { function showSuccess(txInfo, txHash, blockNumber) {
clearTimers(); endWait();
const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?"; const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?";
state.viewData = { state.viewData = {
@@ -182,7 +301,7 @@ function renderSuccess() {
} }
function showError(txInfo, txHash, message) { function showError(txInfo, txHash, message) {
clearTimers(); endWait();
const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?"; const symbol = txInfo.token === "ETH" ? "ETH" : txInfo.tokenSymbol || "?";
state.viewData = { state.viewData = {
@@ -218,6 +337,9 @@ function isApprovalPopup() {
} }
function navigateBack() { function navigateBack() {
// Nothing should still be polling by now, but leaving a view is the
// point at which its timers must be gone.
endWait();
if (isApprovalPopup()) { if (isApprovalPopup()) {
window.close(); window.close();
return; return;
@@ -242,4 +364,12 @@ function init(_ctx) {
$("btn-error-tx-done").addEventListener("click", navigateBack); $("btn-error-tx-done").addEventListener("click", navigateBack);
} }
module.exports = { init, showWait, showError, renderSuccess, renderError }; module.exports = {
init,
showWait,
restoreWait,
endWait,
showError,
renderSuccess,
renderError,
};

View File

@@ -7,17 +7,144 @@
// the signer from the artifact and checks it against the approval it is // the signer from the artifact and checks it against the approval it is
// holding before acting on it. All recovery is delegated to ethers. // holding before acting on it. All recovery is delegated to ethers.
// //
// 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 // Every failure message is a full sentence, because these strings are shown to
// the user and returned to the dApp. // the user and returned to the dApp.
const { const {
Transaction, Transaction,
accessListify,
getAddress, getAddress,
getBytes, getBytes,
verifyMessage, verifyMessage,
verifyTypedData, verifyTypedData,
} = require("ethers"); } = 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 // Case-insensitive address comparison that tolerates absent values on either
// side. Two absent addresses compare equal (contract creation has no `to`). // side. Two absent addresses compare equal (contract creation has no `to`).
function sameAddress(a, b) { function sameAddress(a, b) {
@@ -31,11 +158,64 @@ function sameAddress(a, b) {
} }
} }
// Whether the approval fixed a value for a field at all.
function present(v) {
return v !== null && v !== undefined && v !== "";
}
// Whether a field carries anything at all. An empty array is nothing: ethers
// reports an absent access list on a type 2 transaction as `[]`.
function carriesValue(v) {
if (!present(v)) return false;
if (Array.isArray(v)) return v.length > 0;
return true;
}
// Normalize a quantity that must be present, refusing anything that is not a
// number: an approval carrying junk in a fee field cannot be compared, and an
// uncomparable field is a refusal rather than a pass.
function normalizeQuantity(v, label) {
try {
return BigInt(v);
} catch {
throw refuse(
"The approved " +
label +
" is not a number, so it cannot be" +
" compared with the signed transaction.",
);
}
}
// Normalize a transaction value (hex string, decimal string, number or // Normalize a transaction value (hex string, decimal string, number or
// bigint) to a bigint. An absent value is zero, matching ethers. // bigint) to a bigint. An absent value is zero, matching ethers. The value is
// page-controlled, so it goes through the same refusal as every other
// quantity rather than throwing a raw BigInt conversion error.
function normalizeValue(v) { function normalizeValue(v) {
if (v === null || v === undefined || v === "") return 0n; if (!present(v)) return 0n;
return BigInt(v); 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". // Normalize call data to a lowercase hex string. Absent data is "0x".
@@ -44,44 +224,216 @@ function normalizeData(v) {
return String(v).toLowerCase(); return String(v).toLowerCase();
} }
// Quantity fields the requesting page may fix in the approval. Each is
// compared exactly when the approval carries it, and left to the ceilings
// above when it does not.
const APPROVED_QUANTITIES = [
{
key: "nonce",
label: "nonce",
message: "The signed transaction does not carry the approved nonce.",
},
{
key: "gasLimit",
label: "gas limit",
message:
"The signed transaction does not carry the approved gas limit.",
},
{
key: "gasPrice",
label: "gas price",
message:
"The signed transaction does not carry the approved gas price.",
},
{
key: "maxFeePerGas",
label: "maximum fee per gas",
message:
"The signed transaction does not carry the approved maximum fee per gas.",
},
{
key: "maxPriorityFeePerGas",
label: "maximum priority fee per gas",
message:
"The signed transaction does not carry the approved maximum priority fee per gas.",
},
];
// 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, // Assert that a raw signed transaction is the transaction the user approved,
// signed by the address the approval was raised for. Returns the parsed // signed by the address the approval was raised for, on the network that is
// ethers Transaction on success, throws otherwise. // selected. Returns the parsed ethers Transaction on success, throws
function verifySignedTx(rawSignedTx, txParams, expectedFrom) { // otherwise.
function verifySignedTx(rawSignedTx, txParams, expectedFrom, selectedChainId) {
if (typeof rawSignedTx !== "string" || !rawSignedTx.startsWith("0x")) { if (typeof rawSignedTx !== "string" || !rawSignedTx.startsWith("0x")) {
throw new Error("The signed transaction is missing or malformed."); throw refuse("The signed transaction is missing or malformed.");
} }
let parsed; let parsed;
try { try {
parsed = Transaction.from(rawSignedTx); parsed = Transaction.from(rawSignedTx);
} catch { } catch {
throw new Error("The signed transaction could not be decoded."); throw refuse("The signed transaction could not be decoded.");
} }
if (!parsed.from) { if (!parsed.from) {
throw new Error("The signed transaction carries no valid signature."); throw refuse("The signed transaction carries no valid signature.");
} }
if (!sameAddress(parsed.from, expectedFrom)) { if (!sameAddress(parsed.from, expectedFrom)) {
throw new Error( throw refuse(
"The signed transaction was signed by a different address than the one that was approved.", "The signed transaction was signed by a different address than the one that was approved.",
); );
} }
// 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)) { if (!sameAddress(parsed.to, txParams.to)) {
throw new Error( throw refuse(
"The signed transaction does not go to the approved recipient.", "The signed transaction does not go to the approved recipient.",
); );
} }
if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) { if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) {
throw new Error( throw refuse(
"The signed transaction does not carry the approved value.", "The signed transaction does not carry the approved value.",
); );
} }
if (normalizeData(parsed.data) !== normalizeData(txParams.data)) { if (normalizeData(parsed.data) !== normalizeData(txParams.data)) {
throw new Error( throw refuse(
"The signed transaction does not carry the approved call data.", "The signed transaction does not carry the approved call data.",
); );
} }
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; return parsed;
} }
@@ -91,7 +443,7 @@ function verifySignedTx(rawSignedTx, txParams, expectedFrom) {
// address on success, throws otherwise. // address on success, throws otherwise.
function verifySignature(signParams, signature, expectedFrom) { function verifySignature(signParams, signature, expectedFrom) {
if (typeof signature !== "string" || !signature.startsWith("0x")) { if (typeof signature !== "string" || !signature.startsWith("0x")) {
throw new Error("The signature is missing or malformed."); throw refuse("The signature is missing or malformed.");
} }
let recovered; let recovered;
@@ -109,11 +461,11 @@ function verifySignature(signParams, signature, expectedFrom) {
recovered = verifyTypedData(domain, types, message, signature); recovered = verifyTypedData(domain, types, message, signature);
} }
} catch { } catch {
throw new Error("The signature could not be verified."); throw refuse("The signature could not be verified.");
} }
if (!sameAddress(recovered, expectedFrom)) { if (!sameAddress(recovered, expectedFrom)) {
throw new Error( throw refuse(
"The signature was produced by a different address than the one that was approved.", "The signature was produced by a different address than the one that was approved.",
); );
} }
@@ -121,4 +473,98 @@ function verifySignature(signParams, signature, expectedFrom) {
return recovered; 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 { ERC20_ABI } = require("./constants");
const { log, debugFetch } = require("./log"); const { log, debugFetch } = require("./log");
const { deriveAddressFromXpub } = require("./wallet"); 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 { LOW_HOLDER_THRESHOLD, parseHoldersCount } = require("./holders");
const { isSpoofedSymbol } = require("./symbolSpoof");
// Use a static network to skip auto-detection (which can fail and cause // Use a static network to skip auto-detection (which can fail and cause
// "could not coalesce error" on some RPC endpoints like Cloudflare). // "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 // Skip spam tokens the user never asked to see
if (!isKnown && !isTracked && !hasEnoughHolders) continue; if (!isKnown && !isTracked && !hasEnoughHolders) continue;
// Skip tokens spoofing a known symbol from a different address // Skip tokens spoofing a known symbol from a different address.
const sym = (item.token.symbol || "").toUpperCase(); // Every row here is an ERC-20 the explorer reported, so it has a
const legitAddr = KNOWN_SYMBOLS.get(sym); // contract address; the native ETH balance is fetched over RPC in
if ( // refreshBalances and never passes through this loop.
legitAddr !== undefined && if (isSpoofedSymbol(item.token.symbol, tokenAddr)) continue;
legitAddr !== null &&
tokenAddr !== legitAddr
)
continue;
balances.push({ balances.push({
address: item.token.address_hash, 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 { formatEther, formatUnits } = require("ethers");
const { log, debugFetch } = require("./log"); 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 { parseHoldersCount, isLowHolderCount } = require("./holders");
const { isSpoofedSymbol } = require("./symbolSpoof");
// Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum // Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum
// over the address, not part of its identity. Every address comparison in // 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; 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, // Pure filter function. Takes raw transactions and filter settings,
// returns { transactions, newFraudContracts }. // returns { transactions, newFraudContracts }.
function filterTransactions(txs, filters = {}) { function filterTransactions(txs, filters = {}) {
@@ -283,7 +272,7 @@ function filterTransactions(txs, filters = {}) {
const contract = normalizeAddress(tx.contractAddress); const contract = normalizeAddress(tx.contractAddress);
// Filter spoofed known symbols and record the fraud contract // 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)) { if (contract && !fraudSet.has(contract)) {
fraudSet.add(contract); fraudSet.add(contract);
newFraud.push(contract); newFraud.push(contract);

View File

@@ -1,5 +1,22 @@
// Wallet deletion state transition, kept out of the view so the selection // Wallet and address deletion state transitions, kept out of the views so the
// and broadcast rules are testable without a DOM. // 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. // Remove wallet `walletIdx` from `state` and repair the derived state.
// //
@@ -18,19 +35,13 @@ function removeWalletFromState(state, walletIdx) {
const wallet = state.wallets[walletIdx]; const wallet = state.wallets[walletIdx];
const addresses = (wallet.addresses || []).map((a) => a.address); const addresses = (wallet.addresses || []).map((a) => a.address);
const previousActive = state.activeAddress; const previousActive = state.activeAddress;
const activeWasDeleted = const activeWasDeleted = addresses.some((a) =>
previousActive !== null && sameAddress(a, previousActive),
previousActive !== undefined && );
addresses.some(
(a) => a.toLowerCase() === String(previousActive).toLowerCase(),
);
state.wallets.splice(walletIdx, 1); state.wallets.splice(walletIdx, 1);
for (const addr of addresses) { dropSitePermissions(state, addresses);
delete state.allowedSites[addr];
delete state.deniedSites[addr];
}
state.hasWallet = state.wallets.length > 0; state.hasWallet = state.wallets.length > 0;
@@ -58,6 +69,77 @@ function removeWalletFromState(state, walletIdx) {
return { activeAddressChanged: state.activeAddress !== previousActive }; 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 // Tell the background the active address changed, so it re-emits
// accountsChanged to connected sites. Same call shape as the address // accountsChanged to connected sites. Same call shape as the address
// switch in the home view. // switch in the home view.
@@ -67,4 +149,9 @@ function broadcastActiveChanged() {
runtime.sendMessage({ type: "AUTISTMASK_ACTIVE_CHANGED" }); 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 { const {
verifySignedTx, verifySignedTx,
verifySignature, verifySignature,
assertNoForbiddenFields,
assertNothingUnchecked,
assertCanonicalBytes,
sameAddress, 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"); } = require("../src/shared/approvalVerify");
const { getSignerForAddress } = require("../src/shared/wallet"); const { getSignerForAddress } = require("../src/shared/wallet");
@@ -18,6 +38,10 @@ const other = new Wallet(OTHER_KEY);
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const OTHER_RECIPIENT = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; const OTHER_RECIPIENT = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
// The chain id of the selected network, as networks.js carries it.
const SELECTED = "0x1";
const SEPOLIA = "0xaa36a7";
// Approved parameters as a dApp would supply them over eth_sendTransaction. // Approved parameters as a dApp would supply them over eth_sendTransaction.
const TX_PARAMS = { const TX_PARAMS = {
from: signer.address, from: signer.address,
@@ -27,25 +51,38 @@ const TX_PARAMS = {
gas: "0x5208", gas: "0x5208",
}; };
// The values populateTransaction() fills in when the dApp fixed none of them.
const POPULATED = {
chainId: 1,
nonce: 7,
gasLimit: 100000n,
maxFeePerGas: 2000000000n,
maxPriorityFeePerGas: 1000000000n,
type: 2,
};
// Build a signable transaction from approved params. The popup does the same // Build a signable transaction from approved params. The popup does the same
// thing through populateTransaction(); here the fields are fixed so the test // thing through populateTransaction(); here the fields are fixed so the test
// needs no provider. // needs no provider. `overrides` stands in for what a tampered or misbuilt
function txFor(params) { // popup would put on the wire.
function txFor(params, overrides) {
return { return {
chainId: 1, ...POPULATED,
nonce: 7,
gasLimit: 100000n,
maxFeePerGas: 2000000000n,
maxPriorityFeePerGas: 1000000000n,
type: 2,
to: params.to, to: params.to,
value: params.value === undefined ? 0n : BigInt(params.value), value: params.value === undefined ? 0n : BigInt(params.value),
data: params.data || "0x", data: params.data || "0x",
...(overrides || {}),
}; };
} }
async function signedFor(params, withWallet) { async function signedFor(params, withWallet, overrides) {
return (withWallet || signer).signTransaction(txFor(params)); return (withWallet || signer).signTransaction(txFor(params, overrides));
}
// Sign the approved transaction with one field changed from what was
// populated, which is the shape of every tamper case below.
async function signedWith(overrides) {
return signedFor(TX_PARAMS, signer, overrides);
} }
describe("sameAddress", () => { describe("sameAddress", () => {
@@ -71,7 +108,7 @@ describe("sameAddress", () => {
describe("verifySignedTx", () => { describe("verifySignedTx", () => {
test("accepts the approved transaction signed by the approved address", async () => { test("accepts the approved transaction signed by the approved address", async () => {
const raw = await signedFor(TX_PARAMS); const raw = await signedFor(TX_PARAMS);
const parsed = verifySignedTx(raw, TX_PARAMS, signer.address); const parsed = verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
expect(parsed.from).toBe(signer.address); expect(parsed.from).toBe(signer.address);
expect(parsed.hash).toBe(Transaction.from(raw).hash); expect(parsed.hash).toBe(Transaction.from(raw).hash);
}); });
@@ -79,14 +116,16 @@ describe("verifySignedTx", () => {
test("accepts a contract creation with no recipient", async () => { test("accepts a contract creation with no recipient", async () => {
const params = { to: undefined, value: "0x0", data: "0x600160005500" }; const params = { to: undefined, value: "0x0", data: "0x600160005500" };
const raw = await signedFor(params); const raw = await signedFor(params);
expect(() => verifySignedTx(raw, params, signer.address)).not.toThrow(); expect(() =>
verifySignedTx(raw, params, signer.address, SELECTED),
).not.toThrow();
}); });
test("accepts an absent value as zero", async () => { test("accepts an absent value as zero", async () => {
const approved = { to: RECIPIENT, data: "0x" }; const approved = { to: RECIPIENT, data: "0x" };
const raw = await signedFor(approved); const raw = await signedFor(approved);
expect(() => expect(() =>
verifySignedTx(raw, approved, signer.address), verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow(); ).not.toThrow();
}); });
@@ -94,7 +133,7 @@ describe("verifySignedTx", () => {
const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" }; const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" };
const raw = await signedFor(approved); const raw = await signedFor(approved);
expect(() => expect(() =>
verifySignedTx(raw, approved, signer.address), verifySignedTx(raw, approved, signer.address, SELECTED),
).not.toThrow(); ).not.toThrow();
}); });
@@ -103,9 +142,9 @@ describe("verifySignedTx", () => {
...TX_PARAMS, ...TX_PARAMS,
to: OTHER_RECIPIENT, to: OTHER_RECIPIENT,
}); });
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( expect(() =>
/approved recipient/, verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
); ).toThrow(/approved recipient/);
}); });
test("rejects an inflated value", async () => { test("rejects an inflated value", async () => {
@@ -113,48 +152,48 @@ describe("verifySignedTx", () => {
...TX_PARAMS, ...TX_PARAMS,
value: "0x4563918244f40000", value: "0x4563918244f40000",
}); });
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( expect(() =>
/approved value/, verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
); ).toThrow(/approved value/);
}); });
test("rejects substituted call data", async () => { test("rejects substituted call data", async () => {
const raw = await signedFor({ ...TX_PARAMS, data: "0xc0ffee" }); const raw = await signedFor({ ...TX_PARAMS, data: "0xc0ffee" });
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( expect(() =>
/approved call data/, verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
); ).toThrow(/approved call data/);
}); });
test("rejects a transaction signed by a different address", async () => { test("rejects a transaction signed by a different address", async () => {
const raw = await signedFor(TX_PARAMS, other); const raw = await signedFor(TX_PARAMS, other);
expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow( expect(() =>
/different address/, verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
); ).toThrow(/different address/);
}); });
test("rejects an unsigned transaction", () => { test("rejects an unsigned transaction", () => {
const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized; const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized;
expect(() => expect(() =>
verifySignedTx(unsigned, TX_PARAMS, signer.address), verifySignedTx(unsigned, TX_PARAMS, signer.address, SELECTED),
).toThrow(/no valid signature/); ).toThrow(/no valid signature/);
}); });
test("rejects a missing or malformed payload", () => { test("rejects a missing or malformed payload", () => {
expect(() => expect(() =>
verifySignedTx(undefined, TX_PARAMS, signer.address), verifySignedTx(undefined, TX_PARAMS, signer.address, SELECTED),
).toThrow(/missing or malformed/); ).toThrow(/missing or malformed/);
expect(() => verifySignedTx("nope", TX_PARAMS, signer.address)).toThrow(
/missing or malformed/,
);
expect(() => expect(() =>
verifySignedTx("0xc0ffee", TX_PARAMS, signer.address), verifySignedTx("nope", TX_PARAMS, signer.address, SELECTED),
).toThrow(/missing or malformed/);
expect(() =>
verifySignedTx("0xc0ffee", TX_PARAMS, signer.address, SELECTED),
).toThrow(/could not be decoded/); ).toThrow(/could not be decoded/);
}); });
test("every rejection message is a full sentence", async () => { test("every rejection message is a full sentence", async () => {
const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT }); const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
try { try {
verifySignedTx(raw, TX_PARAMS, signer.address); verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
throw new Error("expected a rejection"); throw new Error("expected a rejection");
} catch (e) { } catch (e) {
expect(e.message).toMatch(/^[A-Z].*\.$/); expect(e.message).toMatch(/^[A-Z].*\.$/);
@@ -162,6 +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({ const TYPED_DATA = JSON.stringify({
domain: { domain: {
name: "AutistMask Test", 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 // End-to-end over the messaging boundary, without a browser: run the exact
// sequence the approval popup runs, then hand the artifact to the exact check // sequence the approval popup runs, then hand the artifact to the exact check
// the background runs before it broadcasts or resolves. Only what the popup // the background runs before it broadcasts or resolves. Only what the popup
@@ -314,7 +1094,12 @@ describe("popup signing sequence to background verification", () => {
test("a populated, signed transaction is accepted and broadcastable", async () => { test("a populated, signed transaction is accepted and broadcastable", async () => {
const rawSignedTx = await popupSignsTx(TX_PARAMS); const rawSignedTx = await popupSignsTx(TX_PARAMS);
const parsed = verifySignedTx(rawSignedTx, TX_PARAMS, signer.address); const parsed = verifySignedTx(
rawSignedTx,
TX_PARAMS,
signer.address,
SELECTED,
);
expect(parsed.nonce).toBe(7); expect(parsed.nonce).toBe(7);
expect(parsed.chainId).toBe(1n); expect(parsed.chainId).toBe(1n);
expect(parsed.gasLimit).toBe(21000n); expect(parsed.gasLimit).toBe(21000n);
@@ -349,7 +1134,14 @@ describe("popup signing sequence to background verification", () => {
to: OTHER_RECIPIENT, to: OTHER_RECIPIENT,
}); });
expect(() => expect(() =>
verifySignedTx(rawSignedTx, TX_PARAMS, signer.address), verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SELECTED),
).toThrow(/approved recipient/); ).toThrow(/approved recipient/);
}); });
test("the background rejects a transaction populated on another network", async () => {
const rawSignedTx = await popupSignsTx(TX_PARAMS);
expect(() =>
verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SEPOLIA),
).toThrow(/different network than the one that is selected/);
});
}); });

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

@@ -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"); 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 // ---------------------------------------------------------------- runner
async function main() { async function main() {

331
tests/exportPrivkey.test.js Normal file
View File

@@ -0,0 +1,331 @@
// Tests for the private key export screen (issue #221).
//
// The screen holds the one secret that owns an address outright, so what is
// pinned here is disposal: the key is wiped from the DOM whenever the screen
// is left by any route, and a decrypt still in flight when the screen is
// left never writes at all. That last case is the one a per-button wipe and
// a naive leave hook both miss — the write lands after the wipe, with
// nothing scheduled to wipe it again.
//
// The view is driven against a minimal DOM stub rather than a real browser:
// the module is deliberately shaped like src/popup/views/showPhrase.js, with
// no dependency that needs a document beyond the nodes it reads and writes.
const mockPrivateKey = "0x" + "ab".repeat(32);
jest.mock("ethereum-blockies-base64", () => () => "data:image/png;base64,x");
jest.mock("../src/shared/vault", () => ({
decryptWithPassword: jest.fn(),
}));
jest.mock("../src/shared/wallet", () => ({
getSignerForAddress: jest.fn(() => ({ privateKey: mockPrivateKey })),
}));
const { RESTORABLE_VIEWS } = require("../src/popup/restorableViews");
const VIEW = "export-privkey";
const PASSWORD = "correct horse battery";
// ------------------------------------------------------------ DOM stub
function makeElement(id, withParent) {
const classes = new Set();
const el = {
id,
textContent: "",
value: "",
innerHTML: "",
disabled: false,
style: {},
dataset: {},
listeners: {},
classList: {
add: (...names) => names.forEach((n) => classes.add(n)),
remove: (...names) => names.forEach((n) => classes.delete(n)),
contains: (n) => classes.has(n),
toggle: (n, force) => {
const on = force === undefined ? !classes.has(n) : force;
if (on) classes.add(n);
else classes.delete(n);
return on;
},
},
addEventListener: (name, fn) => {
el.listeners[name] = el.listeners[name] || [];
el.listeners[name].push(fn);
},
appendChild: () => {},
remove: () => {},
querySelectorAll: () => [],
};
el.parentElement = withParent ? makeElement(id + "-parent", false) : null;
return el;
}
function makeDocument() {
const els = new Map();
return {
getElementById(id) {
// The debug banner is created on demand by helpers.js; absent
// is the state a non-debug, non-testnet popup is in.
if (id === "debug-banner") return null;
if (!els.has(id)) els.set(id, makeElement(id, true));
return els.get(id);
},
createElement: () => makeElement("created", false),
addEventListener: () => {},
body: { prepend: () => {} },
};
}
// ------------------------------------------------------------ harness
function load() {
jest.resetModules();
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
globalThis.document = makeDocument();
const helpers = require("../src/popup/views/helpers");
const { state } = require("../src/shared/state");
const vault = require("../src/shared/vault");
const wallet = require("../src/shared/wallet");
const exportPrivkey = require("../src/popup/views/exportPrivkey");
state.wallets = [
{
name: "Wallet 1",
type: "key",
encryptedSecret: "ciphertext",
addresses: [
{
address: "0x" + "11".repeat(20),
balance: "0.0000",
tokenBalances: [],
},
{
address: "0x" + "22".repeat(20),
balance: "0.0000",
tokenBalances: [],
},
],
},
];
state.viewStack = [];
state.currentView = "address";
exportPrivkey.init();
return { helpers, state, vault, wallet, exportPrivkey };
}
function click(id) {
const el = globalThis.document.getElementById(id);
return Promise.all((el.listeners.click || []).map((fn) => fn()));
}
function node(id) {
return globalThis.document.getElementById(id);
}
// Start a reveal and hand back both the promise it returns and the resolver
// for the decrypt it is waiting on, so a test can navigate away mid-flight.
function startReveal(vault) {
let resolveDecrypt;
let rejectDecrypt;
vault.decryptWithPassword.mockImplementation(
() =>
new Promise((resolve, reject) => {
resolveDecrypt = resolve;
rejectDecrypt = reject;
}),
);
node("export-privkey-password").value = PASSWORD;
const pending = click("btn-export-privkey-confirm");
return {
pending,
resolve: (v) => resolveDecrypt(v),
reject: (e) => rejectDecrypt(e),
};
}
// ------------------------------------------------------------ tests
describe("a decrypt still running when the screen is left", () => {
// The load-bearing case. Without the liveness guard in reveal(), the
// write lands after the leave hook has already wiped, and the key sits
// in the hidden view for the life of the popup.
test("never writes the key into the DOM", async () => {
const { helpers, vault, wallet, exportPrivkey } = load();
exportPrivkey.show(0, 0);
const reveal = startReveal(vault);
// The settings gear, mid-decrypt.
helpers.showView("settings");
reveal.resolve("wallet secret");
await reveal.pending;
expect(node("export-privkey-value").textContent).toBe("");
// Nothing was even derived: the guard sits in front of the
// derivation, not just in front of the write.
expect(wallet.getSignerForAddress).not.toHaveBeenCalled();
});
// The generation counter, not merely the current-view check: by the time
// the stale decrypt resolves the user is back on the screen, so a guard
// that only asked "is this view showing?" would let the write through.
test("never writes it after the screen is re-entered", async () => {
const { helpers, vault, exportPrivkey } = load();
exportPrivkey.show(0, 0);
const stale = startReveal(vault);
helpers.showView("settings");
exportPrivkey.show(0, 1);
expect(node("export-privkey-value").textContent).toBe("");
stale.resolve("wallet secret");
await stale.pending;
expect(node("export-privkey-value").textContent).toBe("");
expect(node("export-privkey-result").classList.contains("hidden")).toBe(
true,
);
});
// Same hole on the failure path: a wrong-password error written after
// the wipe would restore the flash line on a screen the user has left.
test("never writes the failure message either", async () => {
const { helpers, vault, exportPrivkey } = load();
exportPrivkey.show(0, 0);
const reveal = startReveal(vault);
helpers.showView("settings");
reveal.reject(new Error("decryption failed"));
await reveal.pending;
expect(node("export-privkey-flash").textContent).toBe("");
expect(node("export-privkey-flash").style.visibility).toBe("hidden");
});
});
describe("a reveal that is not interrupted", () => {
// Guards the guard: a liveness check that rejected every write would
// pass every test above and ship a screen that reveals nothing.
test("puts the key on screen", async () => {
const { vault, exportPrivkey } = load();
exportPrivkey.show(0, 0);
const reveal = startReveal(vault);
reveal.resolve("wallet secret");
await reveal.pending;
expect(node("export-privkey-value").textContent).toBe(mockPrivateKey);
expect(node("export-privkey-result").classList.contains("hidden")).toBe(
false,
);
// The password is dropped as soon as it has been spent.
expect(node("export-privkey-password").value).toBe("");
});
test("writes nothing before the password is accepted", async () => {
const { vault, exportPrivkey } = load();
exportPrivkey.show(0, 0);
const reveal = startReveal(vault);
expect(node("export-privkey-value").textContent).toBe("");
reveal.resolve("wallet secret");
await reveal.pending;
});
test("reveals nothing when the password is wrong", async () => {
const { vault, exportPrivkey } = load();
exportPrivkey.show(0, 0);
const reveal = startReveal(vault);
reveal.reject(new Error("decryption failed"));
await reveal.pending;
expect(node("export-privkey-value").textContent).toBe("");
expect(node("export-privkey-flash").textContent).toBe(
"That password is not correct. Please try again.",
);
});
});
describe("leaving the screen after the key is on it", () => {
async function revealed() {
const loaded = load();
loaded.exportPrivkey.show(0, 0);
const reveal = startReveal(loaded.vault);
reveal.resolve("wallet secret");
await reveal.pending;
expect(node("export-privkey-value").textContent).toBe(mockPrivateKey);
return loaded;
}
test("the Back button clears the key", async () => {
await revealed();
await click("btn-export-privkey-back");
expect(node("export-privkey-value").textContent).toBe("");
expect(node("export-privkey-password").value).toBe("");
});
test("the settings gear clears the key", async () => {
const { helpers } = await revealed();
helpers.showView("settings");
expect(node("export-privkey-value").textContent).toBe("");
expect(node("export-privkey-password").value).toBe("");
// And the screen is back to its password prompt, not to a result
// panel that would flash an empty well on the next visit.
expect(node("export-privkey-result").classList.contains("hidden")).toBe(
true,
);
expect(
node("export-privkey-password-section").classList.contains(
"hidden",
),
).toBe(false);
});
// Any other navigation: the same hook covers routes that do not exist
// yet, which is the point of registering it on the view rather than on
// the controls that leave it.
test("any other navigation clears the key", async () => {
const { helpers } = await revealed();
helpers.showView("main");
expect(node("export-privkey-value").textContent).toBe("");
});
});
describe("views the popup may reopen onto", () => {
// Restoring onto this screen would put a private key on display with no
// password prompt in front of it, on a popup reopened by accident.
test("the private key export screen is not restorable", () => {
expect(RESTORABLE_VIEWS.has(VIEW)).toBe(false);
});
test("it is still a registered view", () => {
const { helpers } = load();
expect(helpers.VIEWS).toContain(VIEW);
});
});
describe("the key cannot reach the logger", () => {
const fs = require("fs");
const path = require("path");
const source = fs.readFileSync(
path.join(__dirname, "..", "src", "popup", "views", "exportPrivkey.js"),
"utf8",
);
test("the view does not import src/shared/log.js", () => {
expect(source).not.toMatch(/require\(["'][^"']*shared\/log["']\)/);
});
test("the view calls no logger method", () => {
expect(source).not.toMatch(/\blog\.(debugf|infof|warnf|errorf)\b/);
});
});

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();
});
});

482
tests/txStatus.test.js Normal file
View File

@@ -0,0 +1,482 @@
// Lifecycle tests for the post-broadcast transaction status views
// (src/popup/views/txStatus.js).
//
// The bug these pin down: the receipt poll rendered both outcomes on the tick
// that crossed the 60-second deadline, so a confirmed transaction was replaced
// by "not confirmed within 60 seconds" — the user is told their transaction
// failed when it succeeded. The same shape applies to any callback that
// outlives its wait: a receipt lookup still in flight when the view is left
// must not render over whatever replaced it.
//
// Fake timers make the race deterministic: the receipt promise is already
// resolved when the deadline tick runs, so in the unfixed code showSuccess()
// is always followed by showError() on that tick.
//
// No network: getProvider is mocked at the module boundary and there is no
// jsdom in this repo, so the handful of DOM calls these views make are served
// by the stub below.
jest.mock("../src/shared/log", () => ({
log: {
debugf: () => {},
infof: () => {},
warnf: () => {},
errorf: () => {},
},
debugFetch: jest.fn(),
setRuntimeDebug: () => {},
isDebug: () => false,
}));
const mockReceiptLookup = jest.fn();
jest.mock("../src/shared/balances", () => ({
getProvider: () => ({ getTransactionReceipt: mockReceiptLookup }),
refreshBalances: jest.fn(),
}));
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
// ---------------------------------------------------------------------------
// Minimal DOM. Every element is created on demand and remembered by id, so a
// test can read back what a view wrote into it.
// ---------------------------------------------------------------------------
const elements = new Map();
function makeElement(id) {
const classes = new Set(["view", "hidden"]);
const el = {
id,
textContent: "",
innerHTML: "",
style: {},
classList: {
add: (c) => classes.add(c),
remove: (c) => classes.delete(c),
contains: (c) => classes.has(c),
toggle: (c, on) => (on ? classes.add(c) : classes.delete(c)),
},
addEventListener: () => {},
querySelectorAll: () => [],
remove: () => {},
prepend: () => {},
};
// Views reach for .parentElement to hide whole sections.
Object.defineProperty(el, "parentElement", {
get: () => getElement(id + "-parent"),
});
return el;
}
function getElement(id) {
if (!elements.has(id)) elements.set(id, makeElement(id));
return elements.get(id);
}
global.document = {
getElementById: (id) => getElement(id),
// escapeHtml() builds a detached div; textContent in, escaped HTML out.
createElement: () => {
const el = { innerHTML: "" };
Object.defineProperty(el, "textContent", {
set(v) {
el.innerHTML = String(v)
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;");
},
});
return el;
},
body: { prepend: () => {} },
addEventListener: () => {},
};
global.window = { location: { search: "" } };
const stored = {};
global.chrome = {
storage: {
local: {
set: (obj) => {
Object.assign(stored, obj);
return Promise.resolve();
},
get: () => Promise.resolve(stored),
},
},
};
const txStatus = require("../src/popup/views/txStatus");
const { state } = require("../src/shared/state");
const { RESTORABLE_VIEWS } = require("../src/popup/restorableViews");
const TX_HASH =
"0x85215772ed26ea8b39c2b3b18779030487efbe0b5fd7e882592b2f62b837be84";
const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const TX_INFO = {
to: RECIPIENT,
amount: "0.0050",
token: "ETH",
tokenSymbol: null,
};
// True when a view element is not hidden.
function visible(view) {
return !getElement("view-" + view).classList.contains("hidden");
}
function waitStatusText() {
return getElement("wait-tx-status").textContent;
}
beforeEach(() => {
jest.useFakeTimers();
jest.setSystemTime(new Date("2026-08-11T12:00:00Z"));
elements.clear();
mockReceiptLookup.mockReset();
state.wallets = [];
state.viewData = {};
state.viewStack = [];
state.currentView = null;
txStatus.init({ doRefreshAndRender: jest.fn() });
});
afterEach(() => {
txStatus.endWait();
jest.useRealTimers();
});
describe("WaitTx receipt/timeout race", () => {
test("a receipt arriving on the deadline tick leaves the user on SuccessTx", async () => {
// No receipt for the first five polls; the sixth — the tick at
// t=60s, which is also the timeout deadline — returns one.
mockReceiptLookup
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(null)
.mockResolvedValueOnce(null)
.mockResolvedValue({ blockNumber: 21000000 });
txStatus.showWait(TX_INFO, TX_HASH);
expect(visible("wait-tx")).toBe(true);
await jest.advanceTimersByTimeAsync(60000);
expect(visible("success-tx")).toBe(true);
expect(visible("error-tx")).toBe(false);
expect(state.currentView).toBe("success-tx");
expect(state.viewData.blockNumber).toBe(21000000);
expect(state.viewData.message).toBeUndefined();
// And nothing is left running to undo it.
expect(jest.getTimerCount()).toBe(0);
await jest.advanceTimersByTimeAsync(300000);
expect(state.currentView).toBe("success-tx");
expect(mockReceiptLookup).toHaveBeenCalledTimes(6);
});
test("a genuine timeout still shows ErrorTx with the hash", async () => {
mockReceiptLookup.mockResolvedValue(null);
txStatus.showWait(TX_INFO, TX_HASH);
await jest.advanceTimersByTimeAsync(60000);
expect(visible("error-tx")).toBe(true);
expect(state.currentView).toBe("error-tx");
expect(state.viewData.message).toMatch(
/not confirmed within 60 seconds/,
);
expect(state.viewData.hash).toBe(TX_HASH);
// The hash section carries the hash and the etherscan link.
expect(getElement("error-tx-hash").innerHTML).toContain(TX_HASH);
expect(getElement("error-tx-hash").innerHTML).toContain(
"/tx/" + TX_HASH,
);
expect(jest.getTimerCount()).toBe(0);
});
test("a receipt still in flight when the view is left does not render over it", async () => {
let resolveReceipt;
mockReceiptLookup.mockReturnValue(
new Promise((r) => {
resolveReceipt = r;
}),
);
txStatus.showWait(TX_INFO, TX_HASH);
await jest.advanceTimersByTimeAsync(10000);
expect(mockReceiptLookup).toHaveBeenCalledTimes(1);
// User leaves the wait (popup navigation / teardown) while the
// lookup is outstanding, then the lookup finally answers.
txStatus.endWait();
state.currentView = "main";
resolveReceipt({ blockNumber: 21000000 });
await Promise.resolve();
await Promise.resolve();
expect(state.currentView).toBe("main");
expect(visible("success-tx")).toBe(false);
});
test("no timer survives the view being left", async () => {
mockReceiptLookup.mockResolvedValue(null);
txStatus.showWait(TX_INFO, TX_HASH);
expect(jest.getTimerCount()).toBeGreaterThan(0);
txStatus.endWait();
expect(jest.getTimerCount()).toBe(0);
await jest.advanceTimersByTimeAsync(120000);
expect(mockReceiptLookup).not.toHaveBeenCalled();
});
});
describe("WaitTx persistence across popup close", () => {
test("restoreWait resumes the poll with the deadline running from broadcast", async () => {
mockReceiptLookup.mockResolvedValue(null);
txStatus.showWait(TX_INFO, TX_HASH);
expect(state.viewData.pendingWait.hash).toBe(TX_HASH);
const persisted = JSON.parse(JSON.stringify(state.viewData));
// Popup closes: timers die with the page.
txStatus.endWait();
// 45 seconds pass with the popup shut, then it is reopened.
jest.advanceTimersByTime(45000);
state.viewData = persisted;
expect(txStatus.restoreWait()).toBe(true);
expect(visible("wait-tx")).toBe(true);
// Elapsed is counted from the broadcast, not from the reopen.
expect(waitStatusText()).toBe("Waiting for confirmation... 45s");
// The immediate poll on resume has already run.
await Promise.resolve();
expect(mockReceiptLookup).toHaveBeenCalledTimes(1);
// The deadline is 15 seconds away, not 60.
await jest.advanceTimersByTimeAsync(20000);
expect(state.currentView).toBe("error-tx");
});
test("a rejected lookup on the resume poll keeps waiting instead of reporting failure", async () => {
// A wait resumed after the deadline has already passed: the first
// poll is immediate and past 60s, so a thrown lookup must not be
// read as "no receipt". It means "no answer this tick" — keep
// polling, because the transaction may well have confirmed.
mockReceiptLookup.mockResolvedValue(null);
txStatus.showWait(TX_INFO, TX_HASH);
const persisted = JSON.parse(JSON.stringify(state.viewData));
txStatus.endWait();
// Ten minutes with the popup shut, then it is reopened and the
// first receipt lookup fails transiently.
jest.advanceTimersByTime(600000);
mockReceiptLookup.mockReset();
mockReceiptLookup
.mockRejectedValueOnce(new Error("rpc unavailable"))
.mockResolvedValue({ blockNumber: 21000000 });
state.viewData = persisted;
expect(txStatus.restoreWait()).toBe(true);
await jest.advanceTimersByTimeAsync(0);
// The wait is still alive: no timeout was declared off one error.
expect(visible("wait-tx")).toBe(true);
expect(visible("error-tx")).toBe(false);
expect(state.currentView).toBe("wait-tx");
expect(jest.getTimerCount()).toBeGreaterThan(0);
// And the next tick answers, so the confirmed transaction is
// reported as confirmed.
await jest.advanceTimersByTimeAsync(10000);
expect(state.currentView).toBe("success-tx");
expect(state.viewData.blockNumber).toBe(21000000);
});
test("a lookup returning null past the deadline still times out", async () => {
// The counterpart to the test above: the deadline must still fire
// when the lookup actually answers "no receipt".
mockReceiptLookup.mockResolvedValue(null);
txStatus.showWait(TX_INFO, TX_HASH);
const persisted = JSON.parse(JSON.stringify(state.viewData));
txStatus.endWait();
jest.advanceTimersByTime(600000);
state.viewData = persisted;
expect(txStatus.restoreWait()).toBe(true);
await jest.advanceTimersByTimeAsync(0);
expect(state.currentView).toBe("error-tx");
expect(state.viewData.message).toMatch(
/not confirmed within 60 seconds/,
);
});
test("restoreWait reports nothing to resume when no wait is persisted", () => {
state.viewData = {};
expect(txStatus.restoreWait()).toBe(false);
expect(jest.getTimerCount()).toBe(0);
});
test("restoreWait rejects a persisted wait missing its txInfo or broadcast time", () => {
for (const bad of [
{ hash: TX_HASH, broadcastTime: Date.now() },
{ hash: TX_HASH, txInfo: TX_INFO },
{ hash: TX_HASH, txInfo: TX_INFO, broadcastTime: "soon" },
{ hash: TX_HASH, txInfo: TX_INFO, broadcastTime: NaN },
{ hash: TX_HASH, txInfo: "nope", broadcastTime: Date.now() },
// An object that merely lacks a field startWait() dereferences
// is the shape that actually escaped: txInfo.to reaches
// addressTitle(), which calls address.toLowerCase(). typeof []
// is "object", so an array passes an object check.
{ hash: TX_HASH, txInfo: {}, broadcastTime: Date.now() },
{ hash: TX_HASH, txInfo: [], broadcastTime: Date.now() },
{ hash: TX_HASH, txInfo: { to: 42 }, broadcastTime: Date.now() },
// Otherwise complete but for a non-string `to`: only the `to`
// check rejects this one, and without it addressTitle() throws
// out of restoreView().
{
hash: TX_HASH,
txInfo: { to: 42, amount: "0.0050" },
broadcastTime: Date.now(),
},
// Otherwise complete but an array: only Array.isArray() rejects
// it, since typeof [] is "object" and the fields are present.
{
hash: TX_HASH,
txInfo: Object.assign([], { to: RECIPIENT, amount: "0.0050" }),
broadcastTime: Date.now(),
},
{
hash: TX_HASH,
txInfo: { to: RECIPIENT },
broadcastTime: Date.now(),
},
]) {
state.viewData = { pendingWait: bad };
expect(txStatus.restoreWait()).toBe(false);
expect(jest.getTimerCount()).toBe(0);
}
});
test("restoreWait resumes a wait whose recipient is the empty string", () => {
// The shape a contract-deployment approval persists: approval.js
// writes `to: toAddr || ""`, and showWait() renders it without
// complaint. Validation must not be stricter than the live path, or
// that wait is silently abandoned on every popup open.
mockReceiptLookup.mockResolvedValue(null);
state.viewData = {
pendingWait: {
hash: TX_HASH,
txInfo: { ...TX_INFO, to: "" },
broadcastTime: Date.now(),
},
};
expect(txStatus.restoreWait()).toBe(true);
expect(visible("wait-tx")).toBe(true);
});
});
describe("WaitTx against an RPC that never answers", () => {
test("a permanently failing lookup ends the wait instead of polling forever", async () => {
mockReceiptLookup.mockRejectedValue(new Error("rpc unavailable"));
txStatus.showWait(TX_INFO, TX_HASH);
// Six consecutive failures is 60 seconds at the 10s cadence — the
// same patience as the confirmation deadline.
await jest.advanceTimersByTimeAsync(60000);
expect(state.currentView).toBe("error-tx");
expect(visible("wait-tx")).toBe(false);
// The user is told what actually happened: the lookup failed. It is
// not the same fact as "the transaction did not confirm".
expect(state.viewData.message).toMatch(/could not be reached/i);
expect(state.viewData.message).not.toMatch(/not confirmed within/);
expect(state.viewData.hash).toBe(TX_HASH);
// Nothing is left running, and nothing is left to resume onto.
expect(jest.getTimerCount()).toBe(0);
expect(state.viewData.pendingWait).toBeUndefined();
const calls = mockReceiptLookup.mock.calls.length;
await jest.advanceTimersByTimeAsync(3600000);
expect(mockReceiptLookup).toHaveBeenCalledTimes(calls);
expect(state.currentView).toBe("error-tx");
});
test("an answered lookup clears the failure count, so the bound is on consecutive failures", async () => {
// The bound counts failures in a row, not failures in total: a
// flaky RPC that keeps answering in between must not accumulate its
// way to a false "network unreachable".
//
// Polls 1-5 (t=10s..50s) alternate reject / null, so three fail and
// the last answer resets the count at poll 4. From poll 6 on every
// lookup fails. Six in a row is then poll 10, at t=100s. A counter
// that never reset would have reached six at poll 8, t=80s, so the
// window between those two is what this test occupies.
mockReceiptLookup.mockImplementation(() => {
const n = mockReceiptLookup.mock.calls.length;
if (n <= 5 && n % 2 === 0) return Promise.resolve(null);
return Promise.reject(new Error("flaky"));
});
txStatus.showWait(TX_INFO, TX_HASH);
// t=90s: eight failures in total, five of them in a row. A
// cumulative counter has long since fired; a consecutive one has not.
await jest.advanceTimersByTimeAsync(90000);
expect(state.currentView).toBe("wait-tx");
expect(visible("wait-tx")).toBe(true);
expect(jest.getTimerCount()).toBeGreaterThan(0);
// t=100s: the sixth in a row.
await jest.advanceTimersByTimeAsync(10000);
expect(state.currentView).toBe("error-tx");
expect(state.viewData.message).toMatch(/could not be reached/i);
// No lookup ever answered "no receipt" past the deadline, so this
// is not the timeout and must not be reported as one.
expect(state.viewData.message).not.toMatch(/not confirmed within/);
expect(jest.getTimerCount()).toBe(0);
});
test("a resumed wait against a dead RPC also terminates", async () => {
// The reopen path is the one that made this unbounded: the wait is
// persisted, so without a bound every popup open resumes it forever.
mockReceiptLookup.mockResolvedValue(null);
txStatus.showWait(TX_INFO, TX_HASH);
const persisted = JSON.parse(JSON.stringify(state.viewData));
txStatus.endWait();
jest.advanceTimersByTime(3600000);
mockReceiptLookup.mockReset();
mockReceiptLookup.mockRejectedValue(new Error("rpc unavailable"));
state.viewData = persisted;
expect(txStatus.restoreWait()).toBe(true);
await jest.advanceTimersByTimeAsync(60000);
expect(state.currentView).toBe("error-tx");
expect(state.viewData.message).toMatch(/could not be reached/i);
expect(jest.getTimerCount()).toBe(0);
expect(state.viewData.pendingWait).toBeUndefined();
});
});
describe("wait-tx is a view the popup may reopen onto", () => {
// The resume feature is wired through RESTORABLE_VIEWS: restoreView()
// refuses any view not in the set, so dropping "wait-tx" from it kills
// the resume silently — the tests above call restoreWait() directly and
// would all still pass. This pins the membership. Mirrors the exclusion
// assertions in tests/showPhrase.test.js.
test("wait-tx is restorable", () => {
expect(RESTORABLE_VIEWS.has("wait-tx")).toBe(true);
});
});

View File

@@ -1,4 +1,6 @@
const { const {
canRemoveAddress,
removeAddressFromState,
removeWalletFromState, removeWalletFromState,
broadcastActiveChanged, broadcastActiveChanged,
} = require("../src/shared/walletDelete"); } = require("../src/shared/walletDelete");
@@ -6,6 +8,7 @@ const {
// Fixed addresses — never used for anything but these tests. // Fixed addresses — never used for anything but these tests.
const A0 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; const A0 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const A1 = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; const A1 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
const A2 = "0x514910771AF9Ca656af840dff83E8264EcF986CA";
const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599"; const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599";
const C0 = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; 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", () => { describe("broadcastActiveChanged", () => {
afterEach(() => { afterEach(() => {
delete global.chrome; delete global.chrome;