fix: a whitespace-padded symbol bypasses the spoof filter but still displays as the real one #260
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sneak/AutistMask#260
Reference in New Issue
Block a user
A token whose symbol is
" ETH "missesKNOWN_SYMBOLSon all three surfaces, soisSpoofedSymbol()reports it is not a spoof — while HTML collapses the whitespace and displays it asETH, right next to the user's real ETH.That is the exact confusion #235 just closed, reachable by adding one space. Found by the independent review of #257 and confirmed pre-existing — unchanged by that PR, which is why it was not blocked on it.
The attacker controls the symbol string entirely: it is whatever the ERC-20 contract returns.
Why it is now cheap
#257 moved the rule into a single module,
src/shared/symbolSpoof.js, and all three surfaces call it. Normalizing insideisSpoofedSymbol()fixes every surface at once — that was the point of consolidating it.Implementation requirements
src/shared/symbolSpoof.jsso all three call sites inherit it. Trim at minimum." ETH "; it does not catch a zero-width space, a non-breaking space, or Unicode confusables such as CyrillicЕ. Each of those renders indistinguishably fromETHto a user. Say which classes are covered and which are knowingly left open.KNOWN_SYMBOLSor the bundled token list has one.Two adjacent findings from the same review, worth handling here or ruling out:
item.token.type(anything but exactly"ERC-20") drops the row entirely atsrc/shared/balances.js:69, before any filtering. Fail-closed for spam, but it also silently hides a legitimate holding if an explorer ever varies the casing.hideSpoofedSymbolsoff, history still shows what the balance list now hides. Deliberate, from #176 — confirm it is still the intent now that the three surfaces otherwise agree.Definition of done
" ETH "is filtered from the balance list, history and the send selector.TODO.mdupdated in the same commit.make checkpasses.Implemented in #270.
isSpoofedSymbol()now normalizes before theKNOWN_SYMBOLSlookup — NFKC, every Unicode format character (\p{Cf}) removed wherever it sits, trim, uppercase — so all three surfaces inherit it with no call-site change.Boundary: covers ASCII and non-ASCII whitespace padding, zero-width and other invisible format characters, and compatibility variants (fullwidth). Knowingly open, each asserted as open by a test: confusables that are distinct letters (Cyrillic capital Ie, Greek capital Epsilon), bidi reordering, and interior whitespace (
E T Hrenders differently, so folding it would filter a token nobody could confuse).False positives: no entry in
KNOWN_SYMBOLS(506) and no bundled token symbol (512) contains whitespace or a non-ASCII character, so nothing legitimate is newly filtered; a test walks the whole table and asserts it.Adjacent findings: the token-type gate in
src/shared/balances.jsis now case-insensitive (erc-721andERC-20-EXTRAstill dropped, both directions tested). ThehideSpoofedSymbolsasymmetry is unchanged and confirmed deliberate — the switch is a history-only user escape hatch, per #176.Verified: the 10 new tests were written first and watched fail on unmodified
next(all three surfaces), then pass.make checkgreen — 595 tests, 25 suites,test-verify-build18 cases, prettier clean — and green in-container viascript/cibuildwithRUN make checkexecuting rather than cached. Mutation checks: deleting the no-contract native guard kills 21 tests, inverting the address comparison kills 26, dropping.trim()kills 6, dropping the\p{Cf}strip kills 2.