KNOWN_SYMBOLS maps "ETH" to null. The transaction history and the Send
token selector read that as "no contract may bear this symbol" and filtered a
fake ETH ERC-20. The balance list's guard was legitAddr !== undefined && legitAddr !== null && tokenAddr !== legitAddr, so a null mapping failed the guard and the same token was listed as a holding named
ETH, next to the user's real ETH.
Verdict: oversight, not a deliberate native-token exemption. The commit that
added the balance-list check, 5af8a78, says in its own message: "Also rejects
tokens spoofing a known symbol from a different contract address (same check
used for transaction filtering)". The check it names had shipped the opposite
reading a day earlier in b5b4f75
(if (legit === null) return true; // "ETH" as ERC-20 is always fake). The
exemption is also uncommented, in a file that comments its holder-count
reasoning at length, and it protects nothing: the native ETH balance never
enters that loop.
The fix
The rule moves to src/shared/symbolSpoof.js and all three sites call it, the
way #230 extracted src/shared/holders.js. A fourth reading is no longer available to a future
call site.
isSpoofedSymbol(symbol, contractAddress)
No contract address means the native asset, which is never a spoof.
A known symbol mapped to null belongs to the native asset, so no contract may
bear it and every contract bearing it is a spoof.
Otherwise, a case-insensitive address comparison.
Correct by construction for a future null-mapped symbol: the native exemption is
"has no contract address", not "the symbol is ETH", so a second null entry
inherits both halves of the rule with no call-site change. A test adds one and
asserts it.
src/shared/transactions.js, src/popup/views/send.js and src/shared/balances.js lose their local copies; behaviour on the first two is
unchanged, which the existing suites confirm.
The real ETH balance is unaffected
It is not an ERC-20. refreshBalances reads it over RPC with provider.getBalance(); fetchTokenBalances iterates explorer rows and drops
anything that is not item.token.type === "ERC-20", so the rule never sees it. tests/symbolSpoof.test.js drives refreshBalances end to end with a stubbed
provider against a balance list whose only row is a fake ETH: the native balance
comes back exactly as the node reported it and the fake token is gone.
Tests
tests/symbolSpoof.test.js runs the same fake ETH token — the contract from
the attack documented in the README, given 900,000 holders so no other filter can
catch it — through all three surfaces, plus unit coverage of the rule itself.
Demonstrated failing first: with the module added but the three call sites
untouched, exactly the two balance-list cases failed while the history and
selector cases passed — the disagreement itself, reproduced:
● surface 3: the balance list › a fake ETH token clearing the holder floor is filtered
- Array []
+ Array [ Object { "symbol": "ETH", "balance": "0.005", "holders": 900000, ... } ]
Docs
The two README.md passages describing the balance list's null exemption now
state that all three surfaces apply the check identically and name src/shared/symbolSpoof.js as the single source; the Data Model paragraph on fetchTokenBalances() gains the null-mapped case; the source tree gains the new
module. TODO.md updated in the same commit.
Verification
make check green, run in the container via script/cibuild after the rebase
onto next at bd4bdca:
#11 [7/8] RUN make check
#11 11.44 Test Suites: 20 passed, 20 total
#11 11.44 Tests: 434 passed, 434 total
#11 14.79 All matched files use Prettier code style!
An uncached docker build --no-cache run before the rebase showed the same
layer executing rather than replaying cache.
Closes [#235](https://git.eeqj.de/sneak/AutistMask/issues/235).
## The disagreement
`KNOWN_SYMBOLS` maps `"ETH"` to `null`. The transaction history and the Send
token selector read that as "no contract may bear this symbol" and filtered a
fake `ETH` ERC-20. The balance list's guard was
`legitAddr !== undefined && legitAddr !== null && tokenAddr !== legitAddr`, so a
`null` mapping failed the guard and the same token was listed as a holding named
ETH, next to the user's real ETH.
**Verdict: oversight, not a deliberate native-token exemption.** The commit that
added the balance-list check, `5af8a78`, says in its own message: "Also rejects
tokens spoofing a known symbol from a different contract address (same check
used for transaction filtering)". The check it names had shipped the opposite
reading a day earlier in `b5b4f75`
(`if (legit === null) return true; // "ETH" as ERC-20 is always fake`). The
exemption is also uncommented, in a file that comments its holder-count
reasoning at length, and it protects nothing: the native ETH balance never
enters that loop.
## The fix
The rule moves to `src/shared/symbolSpoof.js` and all three sites call it, the
way [#230](https://git.eeqj.de/sneak/AutistMask/issues/230) extracted
`src/shared/holders.js`. A fourth reading is no longer available to a future
call site.
isSpoofedSymbol(symbol, contractAddress)
- No contract address means the native asset, which is never a spoof.
- A known symbol mapped to `null` belongs to the native asset, so no contract may
bear it and every contract bearing it is a spoof.
- Otherwise, a case-insensitive address comparison.
Correct by construction for a future null-mapped symbol: the native exemption is
"has no contract address", not "the symbol is ETH", so a second `null` entry
inherits both halves of the rule with no call-site change. A test adds one and
asserts it.
`src/shared/transactions.js`, `src/popup/views/send.js` and
`src/shared/balances.js` lose their local copies; behaviour on the first two is
unchanged, which the existing suites confirm.
## The real ETH balance is unaffected
It is not an ERC-20. `refreshBalances` reads it over RPC with
`provider.getBalance()`; `fetchTokenBalances` iterates explorer rows and drops
anything that is not `item.token.type === "ERC-20"`, so the rule never sees it.
`tests/symbolSpoof.test.js` drives `refreshBalances` end to end with a stubbed
provider against a balance list whose only row is a fake ETH: the native balance
comes back exactly as the node reported it and the fake token is gone.
## Tests
`tests/symbolSpoof.test.js` runs the same fake `ETH` token — the contract from
the attack documented in the README, given 900,000 holders so no other filter can
catch it — through all three surfaces, plus unit coverage of the rule itself.
Demonstrated failing first: with the module added but the three call sites
untouched, exactly the two balance-list cases failed while the history and
selector cases passed — the disagreement itself, reproduced:
● surface 3: the balance list › a fake ETH token clearing the holder floor is filtered
- Array []
+ Array [ Object { "symbol": "ETH", "balance": "0.005", "holders": 900000, ... } ]
## Docs
The two `README.md` passages describing the balance list's `null` exemption now
state that all three surfaces apply the check identically and name
`src/shared/symbolSpoof.js` as the single source; the Data Model paragraph on
`fetchTokenBalances()` gains the null-mapped case; the source tree gains the new
module. `TODO.md` updated in the same commit.
## Verification
`make check` green, run in the container via `script/cibuild` after the rebase
onto `next` at `bd4bdca`:
#11 [7/8] RUN make check
#11 11.44 Test Suites: 20 passed, 20 total
#11 11.44 Tests: 434 passed, 434 total
#11 14.79 All matched files use Prettier code style!
An uncached `docker build --no-cache` run before the rebase showed the same
layer executing rather than replaying cache.
KNOWN_SYMBOLS maps "ETH" to null, and the three surfaces that show tokens
disagreed about what that means. The transaction history and the Send
token selector read it as "no contract may bear this symbol" and filtered
a fake ETH ERC-20; the balance list's guard required a non-null mapping,
so the same token was listed as a holding named ETH next to the user's
real ETH. That is the surface where the user forms their belief about
what they own.
The rule now lives in src/shared/symbolSpoof.js and all three sites call
it, so a fourth reading is not available to a future call site. A symbol
mapped to null belongs to the native asset and may be borne by no
contract at all; the native exemption is "has no contract address", not
"the symbol is ETH", so a second null-mapped entry needs no call-site
change.
The user's real ETH balance is untouched: it is read over RPC in
refreshBalances and never enters fetchTokenBalances, whose loop only
considers explorer rows of type ERC-20.
tests/symbolSpoof.test.js drives the same fake ETH token through all
three surfaces plus refreshBalances, which reports the native balance
unchanged while the fake token is gone. Its two balance-list cases were
watched failing against the unmodified call sites first.
clawbot
self-assigned this 2026-08-12 10:47:46 +02:00
Independent review of 88a5c92 against #235: definition of done met, archaeology verified (b5b4f75 2026-02-26 precedes 5af8a78 2026-02-27 and contains the quoted legit === null line), all six new guards mutation-tested and each kills tests, no local copy of the rule remains, fast-forwardable onto origin/next at bd4bdca, one commit, correct author/committer, TODO.md one bullet at top with no landed entry lost, README passages match behaviour, no forbidden references.
Money check, done independently rather than taken on the author's word: with a balance list holding a fake ETH, real WETH (both checksummed and lowercase forms) and USDC, exactly the fake ETH disappears; addr.balance from provider.getBalance() is unchanged. A WETH-symbol token from a foreign contract is still caught. No input to isSpoofedSymbol was found that hides real ETH or a legitimate token.
Anomalies and disclosures:
The failing-first claim reproduces, but with three of this PR's own tests failing against the unmodified call sites, not two as the PR body states: a fake ETH token clearing the holder floor is filtered, tracking the fake token manually does not admit it either, and the real native ETH balance survives a fake ETH airdrop (the last on its tokenBalances assertion only; the native balance itself is correct in both directions). History and Send-selector cases pass alongside, which is the disagreement. Substance unaffected; the count in the PR body is wrong.
Pre-existing and out of scope, noted because it sits next to this rule: a symbol with surrounding whitespace (" ETH ") misses KNOWN_SYMBOLS on all three surfaces and is not a spoof by this rule, while HTML collapses the whitespace on display. Unchanged by this PR, in place before it; worth its own issue.
Tracker CI status ignored per #220. Checks were run here instead: script/cibuild executed make check in the container (layer DONE 17.2s, not CACHED) reporting 20 suites / 434 tests and Prettier clean, matching the PR body.
## Review: PASS
Independent review of `88a5c92` against [#235](https://git.eeqj.de/sneak/AutistMask/issues/235): definition of done met, archaeology verified (`b5b4f75` 2026-02-26 precedes `5af8a78` 2026-02-27 and contains the quoted `legit === null` line), all six new guards mutation-tested and each kills tests, no local copy of the rule remains, fast-forwardable onto `origin/next` at `bd4bdca`, one commit, correct author/committer, TODO.md one bullet at top with no landed entry lost, README passages match behaviour, no forbidden references.
Money check, done independently rather than taken on the author's word: with a balance list holding a fake `ETH`, real WETH (both checksummed and lowercase forms) and USDC, exactly the fake `ETH` disappears; `addr.balance` from `provider.getBalance()` is unchanged. A `WETH`-symbol token from a foreign contract is still caught. No input to `isSpoofedSymbol` was found that hides real ETH or a legitimate token.
Anomalies and disclosures:
- The failing-first claim reproduces, but with three of this PR's own tests failing against the unmodified call sites, not two as the PR body states: `a fake ETH token clearing the holder floor is filtered`, `tracking the fake token manually does not admit it either`, and `the real native ETH balance survives a fake ETH airdrop` (the last on its `tokenBalances` assertion only; the native balance itself is correct in both directions). History and Send-selector cases pass alongside, which is the disagreement. Substance unaffected; the count in the PR body is wrong.
- Pre-existing and out of scope, noted because it sits next to this rule: a symbol with surrounding whitespace (`" ETH "`) misses `KNOWN_SYMBOLS` on all three surfaces and is not a spoof by this rule, while HTML collapses the whitespace on display. Unchanged by this PR, in place before it; worth its own issue.
- Tracker CI status ignored per [#220](https://git.eeqj.de/sneak/AutistMask/issues/220). Checks were run here instead: `script/cibuild` executed `make check` in the container (layer `DONE 17.2s`, not `CACHED`) reporting 20 suites / 434 tests and Prettier clean, matching the PR body.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #235.
The disagreement
KNOWN_SYMBOLSmaps"ETH"tonull. The transaction history and the Sendtoken selector read that as "no contract may bear this symbol" and filtered a
fake
ETHERC-20. The balance list's guard waslegitAddr !== undefined && legitAddr !== null && tokenAddr !== legitAddr, so anullmapping failed the guard and the same token was listed as a holding namedETH, next to the user's real ETH.
Verdict: oversight, not a deliberate native-token exemption. The commit that
added the balance-list check,
5af8a78, says in its own message: "Also rejectstokens spoofing a known symbol from a different contract address (same check
used for transaction filtering)". The check it names had shipped the opposite
reading a day earlier in
b5b4f75(
if (legit === null) return true; // "ETH" as ERC-20 is always fake). Theexemption is also uncommented, in a file that comments its holder-count
reasoning at length, and it protects nothing: the native ETH balance never
enters that loop.
The fix
The rule moves to
src/shared/symbolSpoof.jsand all three sites call it, theway #230 extracted
src/shared/holders.js. A fourth reading is no longer available to a futurecall site.
nullbelongs to the native asset, so no contract maybear it and every contract bearing it is a spoof.
Correct by construction for a future null-mapped symbol: the native exemption is
"has no contract address", not "the symbol is ETH", so a second
nullentryinherits both halves of the rule with no call-site change. A test adds one and
asserts it.
src/shared/transactions.js,src/popup/views/send.jsandsrc/shared/balances.jslose their local copies; behaviour on the first two isunchanged, which the existing suites confirm.
The real ETH balance is unaffected
It is not an ERC-20.
refreshBalancesreads it over RPC withprovider.getBalance();fetchTokenBalancesiterates explorer rows and dropsanything that is not
item.token.type === "ERC-20", so the rule never sees it.tests/symbolSpoof.test.jsdrivesrefreshBalancesend to end with a stubbedprovider against a balance list whose only row is a fake ETH: the native balance
comes back exactly as the node reported it and the fake token is gone.
Tests
tests/symbolSpoof.test.jsruns the same fakeETHtoken — the contract fromthe attack documented in the README, given 900,000 holders so no other filter can
catch it — through all three surfaces, plus unit coverage of the rule itself.
Demonstrated failing first: with the module added but the three call sites
untouched, exactly the two balance-list cases failed while the history and
selector cases passed — the disagreement itself, reproduced:
Docs
The two
README.mdpassages describing the balance list'snullexemption nowstate that all three surfaces apply the check identically and name
src/shared/symbolSpoof.jsas the single source; the Data Model paragraph onfetchTokenBalances()gains the null-mapped case; the source tree gains the newmodule.
TODO.mdupdated in the same commit.Verification
make checkgreen, run in the container viascript/cibuildafter the rebaseonto
nextatbd4bdca:An uncached
docker build --no-cacherun before the rebase showed the samelayer executing rather than replaying cache.
Review: PASS
Independent review of
88a5c92against #235: definition of done met, archaeology verified (b5b4f752026-02-26 precedes5af8a782026-02-27 and contains the quotedlegit === nullline), all six new guards mutation-tested and each kills tests, no local copy of the rule remains, fast-forwardable ontoorigin/nextatbd4bdca, one commit, correct author/committer, TODO.md one bullet at top with no landed entry lost, README passages match behaviour, no forbidden references.Money check, done independently rather than taken on the author's word: with a balance list holding a fake
ETH, real WETH (both checksummed and lowercase forms) and USDC, exactly the fakeETHdisappears;addr.balancefromprovider.getBalance()is unchanged. AWETH-symbol token from a foreign contract is still caught. No input toisSpoofedSymbolwas found that hides real ETH or a legitimate token.Anomalies and disclosures:
a fake ETH token clearing the holder floor is filtered,tracking the fake token manually does not admit it either, andthe real native ETH balance survives a fake ETH airdrop(the last on itstokenBalancesassertion only; the native balance itself is correct in both directions). History and Send-selector cases pass alongside, which is the disagreement. Substance unaffected; the count in the PR body is wrong." ETH ") missesKNOWN_SYMBOLSon all three surfaces and is not a spoof by this rule, while HTML collapses the whitespace on display. Unchanged by this PR, in place before it; worth its own issue.script/cibuildexecutedmake checkin the container (layerDONE 17.2s, notCACHED) reporting 20 suites / 434 tests and Prettier clean, matching the PR body.88a5c92547tob75bd197b2clawbot referenced this pull request2026-08-12 11:49:19 +02:00