fix: judge the symbol a user sees, not the bytes a contract returns (closes #260)
All checks were successful
check / check (push) Successful in 32s

A token whose symbol is " ETH " missed KNOWN_SYMBOLS on all three surfaces
while HTML collapsed the padding and painted it as ETH next to the user's
real ETH. isSpoofedSymbol() now normalizes before the lookup: NFKC, every
Unicode format character removed wherever it sits, then trimmed, then
uppercased. All three surfaces inherit it unchanged.

Covered: ASCII and Unicode whitespace padding, zero-width and other
invisible format characters, and compatibility variants such as fullwidth
letters. Knowingly left open, and asserted as open in the suite: confusables
that are distinct letters (Cyrillic capital Ie), bidi reordering, and
interior whitespace, which renders differently and so is not the confusion.

No symbol in KNOWN_SYMBOLS or the bundled token list contains whitespace or
a non-ASCII character, so nothing legitimate is newly filtered; a test walks
the whole table and asserts it.

The balance list's token-type gate is now case-insensitive. It compared
exactly, so an explorer writing "erc-20" would silently drop a real holding
before any filter ran. Which types are admitted is unchanged.
This commit is contained in:
2026-08-12 10:12:28 +00:00
parent a08ba6a66d
commit ada643cb44
4 changed files with 339 additions and 2 deletions

12
TODO.md
View File

@@ -44,6 +44,18 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 2026-08-12: The known-symbol spoof rule now judges the symbol a user actually
sees. `isSpoofedSymbol()` normalizes before the lookup — NFKC, then every
format and default-ignorable character removed, then trimmed — so `" ETH "`, a
no-break space, a zero-width space, a Hangul filler, a variation selector and
a fullwidth `` are all caught on the balance list, the history and the
send selector at once. Confusables that are distinct letters (Cyrillic `Е`),
bidi reordering, the visible C0/C1 controls and U+007F stay knowingly open and
are asserted as open in the suite. No bundled symbol contains whitespace or a
non-ASCII character, so nothing legitimate is newly filtered; the balance
list's token-type gate also became case-insensitive, which no longer drops a
real holding if an explorer writes `erc-20`
([#260](https://git.eeqj.de/sneak/AutistMask/issues/260)).
- 2026-08-12: The restored navigation stack is filtered against - 2026-08-12: The restored navigation stack is filtered against
`RESTORABLE_VIEWS` on load, truncated at the first entry the popup would not `RESTORABLE_VIEWS` on load, truncated at the first entry the popup would not
render so that every surviving entry keeps the Back target it had. Back after render so that every surviving entry keeps the Back target it had. Back after

View File

@@ -66,7 +66,12 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
const balances = []; const balances = [];
for (const item of items) { for (const item of items) {
if (item.token?.type !== "ERC-20") continue; // Case-insensitive: the token type is an explorer's label, not a
// protocol value, and an exact comparison silently drops a real
// holding if one ever writes "erc-20". Which types are admitted
// is unchanged.
const type = String(item.token?.type || "").toUpperCase();
if (type !== "ERC-20") continue;
const decimals = parseInt(item.token.decimals || "18", 10); const decimals = parseInt(item.token.decimals || "18", 10);
const bal = formatTokenBalance(item.value || "0", decimals); const bal = formatTokenBalance(item.value || "0", decimals);
if (bal === "0.0") continue; if (bal === "0.0") continue;

View File

@@ -13,6 +13,11 @@
// which has no contract at all, so no contract may bear it and every one // 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 // 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. // written so that a second one needs no change here or at any call site.
//
// The symbol is attacker-controlled — it is whatever the ERC-20 contract
// returns — so the lookup is done on a normalized form (issue #260): the
// question is whether the symbol reaches the user's eye as a known one,
// since that is what the user acts on.
const { KNOWN_SYMBOLS } = require("./tokenList"); const { KNOWN_SYMBOLS } = require("./tokenList");
@@ -22,6 +27,52 @@ function normalizeAddress(addr) {
return (addr || "").toLowerCase(); return (addr || "").toLowerCase();
} }
// Fold a symbol onto what a user actually sees, and no further:
//
// NFKC collapses compatibility variants that render as the ASCII
// letters they imitate — fullwidth ETH, styled mathematical
// letters — and maps the non-ASCII spaces onto U+0020.
// strip drops \p{Cf} plus \p{Default_Ignorable_Code_Point}: the
// format characters (zero-width space, joiner and non-joiner,
// word joiner, soft hyphen, byte-order mark, bidi marks and
// overrides), the variation selectors, and the Hangul
// fillers. \p{Cf} alone is not the class of things that
// paint nothing — a Hangul filler is Lo and a variation
// selector is Mn, and both are as invisible as a zero-width
// space. Removed everywhere, not merely at the ends.
// trim removes surrounding whitespace, which HTML collapses:
// `" ETH "` is painted next to the user's real ETH as `ETH`.
// toUpperCase makes the comparison case-insensitive, as before.
//
// The class is Unicode's, so what it covers is a definition rather than a
// measurement; measured in the repo's pinned e2e Chromium (16px sans-serif,
// plain `ETH` = 32.00px), every stripped character paints nothing except
// U+1160 and U+FFA0, which font fallback draws as a box. Stripping those
// two hides a token that does not look like the symbol, which is the
// harmless direction of the two.
//
// Deliberately not folded, and asserted as open in tests/symbolSpoof.test.js:
// interior whitespace (`E T H` renders as `E T H`, so folding it would filter
// a token nobody could confuse with the native asset), confusables that are
// distinct letters rather than compatibility variants (Cyrillic capital Ie,
// U+0415; Greek capital Epsilon, U+0395), and bidi reordering, which needs
// the bidi algorithm rather than a character filter. The C0/C1 controls are
// left alone because they render as a visible box (48.00px) — except U+007F,
// which measures 32.00px, i.e. invisible and still not caught. That one is
// a live gap, flagged rather than closed here because it is a control
// character rather than a default-ignorable one and the class to strip is a
// decision of its own.
//
// This decides only how the question is asked. Nothing here changes what a
// surface displays; a token still shows the symbol it reports.
function normalizeSymbol(symbol) {
return String(symbol || "")
.normalize("NFKC")
.replace(/[\p{Cf}\p{Default_Ignorable_Code_Point}]/gu, "")
.trim()
.toUpperCase();
}
// True when a token bearing `symbol` from contract `contractAddress` is // True when a token bearing `symbol` from contract `contractAddress` is
// impersonating a known symbol. // impersonating a known symbol.
// //
@@ -31,7 +82,7 @@ function normalizeAddress(addr) {
function isSpoofedSymbol(symbol, contractAddress) { function isSpoofedSymbol(symbol, contractAddress) {
const contract = normalizeAddress(contractAddress); const contract = normalizeAddress(contractAddress);
if (!contract) return false; if (!contract) return false;
const sym = (symbol || "").toUpperCase(); const sym = normalizeSymbol(symbol);
if (!KNOWN_SYMBOLS.has(sym)) return false; if (!KNOWN_SYMBOLS.has(sym)) return false;
const legit = KNOWN_SYMBOLS.get(sym); const legit = KNOWN_SYMBOLS.get(sym);
if (legit === null) return true; if (legit === null) return true;

View File

@@ -124,6 +124,173 @@ describe("the shared rule", () => {
}); });
}); });
// Issue #260: the symbol is whatever the ERC-20 contract returns, and HTML
// collapses leading and trailing whitespace, so a token calling itself
// `" ETH "` reaches the user's eye as `ETH` while missing a raw
// KNOWN_SYMBOLS lookup. Normalizing inside the shared rule fixes all three
// surfaces at once, which is what consolidating the rule bought.
//
// Every character under test here is built from its code point rather than
// pasted in: most of them are invisible, and an invisible character in a
// test file is unreviewable.
const cp = (...codes) => String.fromCodePoint(...codes);
const NBSP = cp(0x00a0); // no-break space
const FIGURE_SPACE = cp(0x2007);
const IDEOGRAPHIC_SPACE = cp(0x3000);
const ZWSP = cp(0x200b); // zero-width space
const BOM = cp(0xfeff); // zero-width no-break space
const WORD_JOINER = cp(0x2060);
const SOFT_HYPHEN = cp(0x00ad);
const LRM = cp(0x200e); // left-to-right mark
const RLO = cp(0x202e); // right-to-left override
const HANGUL_FILLER = cp(0x3164);
const CHOSEONG_FILLER = cp(0x115f);
const VS16 = cp(0xfe0f); // variation selector-16
const VS1 = cp(0xfe00); // variation selector-1
const NEL = cp(0x0085); // next line, a C1 control
const DEL = cp(0x007f);
const FULLWIDTH_ETH = cp(0xff25, 0xff34, 0xff28);
const FULLWIDTH_USDC = cp(0xff55, 0xff53, 0xff44, 0xff43); // lowercase
const CYRILLIC_CAPITAL_IE = cp(0x0415);
describe("the shared rule: symbols that render as a known symbol", () => {
test("ASCII padding does not buy a pass", () => {
expect(isSpoofedSymbol(" ETH ", FAKE_ETH_CONTRACT)).toBe(true);
expect(isSpoofedSymbol("\tETH\n", FAKE_ETH_CONTRACT)).toBe(true);
expect(isSpoofedSymbol(" usdc ", FAKE_ETH_CONTRACT)).toBe(true);
});
test("non-breaking and other Unicode spaces do not either", () => {
expect(isSpoofedSymbol(NBSP + "ETH" + NBSP, FAKE_ETH_CONTRACT)).toBe(
true,
);
expect(
isSpoofedSymbol(
FIGURE_SPACE + "ETH" + IDEOGRAPHIC_SPACE,
FAKE_ETH_CONTRACT,
),
).toBe(true);
});
// These render as nothing at all, in any position, so they are removed
// wherever they sit rather than only at the ends.
test("zero-width characters are stripped wherever they sit", () => {
expect(isSpoofedSymbol("E" + ZWSP + "TH", FAKE_ETH_CONTRACT)).toBe(
true,
);
expect(isSpoofedSymbol(BOM + "ETH", FAKE_ETH_CONTRACT)).toBe(true);
expect(
isSpoofedSymbol("ET" + WORD_JOINER + "H", FAKE_ETH_CONTRACT),
).toBe(true);
expect(
isSpoofedSymbol("E" + SOFT_HYPHEN + "TH", FAKE_ETH_CONTRACT),
).toBe(true);
});
// An LRM is invisible and, in all-Latin text, moves nothing: dropping it
// leaves exactly the string the user saw.
test("an invisible bidi mark does not hide a known symbol", () => {
expect(isSpoofedSymbol(LRM + "ETH", FAKE_ETH_CONTRACT)).toBe(true);
});
// Invisibility is not confined to \p{Cf}. A Hangul filler is Lo and a
// variation selector is Mn, yet each of these four measures 32.00px in
// the repo's pinned e2e Chromium at 16px sans-serif — exactly the width
// of a plain `ETH` — so each reaches the user's eye as `ETH`. They are
// caught by \p{Default_Ignorable_Code_Point}, not by \p{Cf}.
test("invisible non-format characters are stripped too", () => {
expect(isSpoofedSymbol(HANGUL_FILLER + "ETH", FAKE_ETH_CONTRACT)).toBe(
true,
);
expect(
isSpoofedSymbol(CHOSEONG_FILLER + "ETH", FAKE_ETH_CONTRACT),
).toBe(true);
expect(isSpoofedSymbol("ETH" + VS16, FAKE_ETH_CONTRACT)).toBe(true);
expect(isSpoofedSymbol("E" + VS1 + "TH", FAKE_ETH_CONTRACT)).toBe(true);
});
// The strip stops at default-ignorable and must not creep past it: the
// C0 and C1 controls render as a visible 48.00px box in the same
// browser, so a symbol carrying one does not look like `ETH` and must
// not be judged a spoof.
test("visible control characters do not make a symbol a spoof", () => {
expect(isSpoofedSymbol(NEL + "ETH", FAKE_ETH_CONTRACT)).toBe(false);
expect(isSpoofedSymbol(cp(0x0001) + "ETH", FAKE_ETH_CONTRACT)).toBe(
false,
);
expect(isSpoofedSymbol(cp(0x0090) + "ETH", FAKE_ETH_CONTRACT)).toBe(
false,
);
});
// Known gap, recorded rather than endorsed: U+007F is a control (Cc),
// not default-ignorable, so the strip does not reach it — but unlike
// the rest of its class it measures 32.00px, i.e. it paints nothing.
// Closing it means picking a rule for the controls, which is a decision
// of its own; this assertion is here so the gap cannot be forgotten.
test("U+007F renders as nothing and is knowingly still not caught", () => {
expect(isSpoofedSymbol(DEL + "ETH", FAKE_ETH_CONTRACT)).toBe(false);
});
test("compatibility forms fold onto the symbol they imitate", () => {
expect(isSpoofedSymbol(FULLWIDTH_ETH, FAKE_ETH_CONTRACT)).toBe(true);
expect(isSpoofedSymbol(FULLWIDTH_USDC, FAKE_ETH_CONTRACT)).toBe(true);
});
// The two knowingly open classes, asserted here so that the boundary is
// a fact in the suite and not a claim in a PR body. A Cyrillic capital
// Ie is a distinct letter rather than a compatibility variant, so NFKC
// leaves it alone; and a right-to-left override reverses the rendering
// of what follows it, which dropping the control character does not
// undo. Closing either needs a confusables table or a bidi resolver,
// and both are a separate change from this one.
test("a Cyrillic homoglyph is knowingly still not caught", () => {
expect(
isSpoofedSymbol(CYRILLIC_CAPITAL_IE + "TH", FAKE_ETH_CONTRACT),
).toBe(false);
});
test("a bidi-reordered symbol is knowingly still not caught", () => {
expect(isSpoofedSymbol(RLO + "HTE", FAKE_ETH_CONTRACT)).toBe(false);
});
// Normalization does not reach the native-asset exemption, which turns
// on the absence of a contract address and never on the symbol.
test("a padded symbol with no contract is still not a spoof", () => {
expect(isSpoofedSymbol(" ETH ", null)).toBe(false);
expect(isSpoofedSymbol(NBSP + "ETH", "")).toBe(false);
});
test("a genuine contract still bears its own padded symbol", () => {
expect(isSpoofedSymbol(" USDC ", USDC_CONTRACT)).toBe(false);
expect(isSpoofedSymbol(ZWSP + "WETH", WETH_CONTRACT)).toBe(false);
});
// Normalization must not invent a match. Interior ASCII whitespace is
// left alone: `E T H` renders as `E T H`, not as `ETH`, so folding it
// would filter a token no user could confuse with the native asset.
test("a symbol that renders differently is not judged a spoof", () => {
expect(isSpoofedSymbol("E T H", FAKE_ETH_CONTRACT)).toBe(false);
expect(isSpoofedSymbol("ETH2", FAKE_ETH_CONTRACT)).toBe(false);
expect(isSpoofedSymbol("MY ETH", FAKE_ETH_CONTRACT)).toBe(false);
});
// The false-positive question, answered against the shipped data rather
// than by assertion: no bundled symbol carries whitespace or a
// non-ASCII character, so the normalization cannot newly filter one.
// The character class starts at `!` rather than at the space so that it
// asserts the claim it stands for — `[ -~]` would admit an interior
// space and let a whitespace-bearing entry through the guard.
test("no bundled symbol is touched by the normalization", () => {
for (const [symbol, address] of KNOWN_SYMBOLS) {
expect(symbol).toBe(symbol.trim());
expect(symbol).toMatch(/^[!-~]+$/);
if (address === null) continue;
expect(isSpoofedSymbol(symbol, address)).toBe(false);
}
});
});
describe("surface 1: the transaction history", () => { describe("surface 1: the transaction history", () => {
function fakeEthTransfer() { function fakeEthTransfer() {
return { return {
@@ -147,6 +314,22 @@ describe("surface 1: the transaction history", () => {
expect(result.transactions).toEqual([]); expect(result.transactions).toEqual([]);
}); });
// Issue #260 on this surface: the same transfer with a padded symbol.
test("a padded fake ETH token transfer is filtered too", () => {
const padded = { ...fakeEthTransfer(), symbol: " ETH " };
const result = filterTransactions([padded], {
hideSpoofedSymbols: true,
hideFraudContracts: true,
hideLowHolderTokens: true,
hideDustTransactions: true,
dustThresholdGwei: 100000,
});
expect(result.transactions).toEqual([]);
// The contract is learned as fraudulent, exactly as for the
// unpadded symbol: the padding must not cost the blocklist entry.
expect(result.newFraudContracts).toEqual([FAKE_ETH_CONTRACT]);
});
test("a real native ETH transfer survives", () => { test("a real native ETH transfer survives", () => {
const native = { const native = {
hash: "0x" + "2".repeat(64), hash: "0x" + "2".repeat(64),
@@ -201,6 +384,36 @@ describe("surface 2: the Send token selector", () => {
expect(select.children).toEqual([]); expect(select.children).toEqual([]);
}); });
// Issue #260 on this surface: the option text is rendered into HTML,
// which collapses the padding, so an unfiltered padded token would sit
// in the selector reading exactly `ETH`.
test("a padded fake ETH token is not selectable either", () => {
render([
{
address: FAKE_ETH_CONTRACT,
symbol: " ETH ",
decimals: 18,
balance: "0.005",
holders: 900000,
},
]);
expect(select.children).toEqual([]);
});
test("a genuine token with a padded symbol stays selectable", () => {
render([
{
address: USDC_CONTRACT,
symbol: " USDC ",
decimals: 6,
balance: "12.5",
holders: 900000,
},
]);
expect(select.children).toHaveLength(1);
expect(select.children[0].value).toBe(USDC_CONTRACT);
});
test("native ETH remains the always-present option", () => { test("native ETH remains the always-present option", () => {
render([]); render([]);
expect(select.innerHTML).toBe('<option value="ETH">ETH</option>'); expect(select.innerHTML).toBe('<option value="ETH">ETH</option>');
@@ -251,6 +464,35 @@ describe("surface 3: the balance list", () => {
expect(balances).toEqual([]); expect(balances).toEqual([]);
}); });
// Issue #260 on this surface: the balance list is where the user forms
// their belief about what they own, and it renders the symbol into HTML.
test("a padded fake ETH token is filtered too", async () => {
respondWith([fakeEthItem({ symbol: " ETH " })]);
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
});
test("a fake ETH token padded with a no-break space is filtered", async () => {
respondWith([fakeEthItem({ symbol: NBSP + "ETH" + NBSP })]);
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
});
// The false-positive direction on the surface that matters most: a real
// holding whose symbol happens to carry padding is still listed, and the
// list still shows the symbol the token actually reports.
test("a genuine token with a padded symbol is not newly filtered", 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 ");
});
test("a genuine token keeps its place in the list", async () => { test("a genuine token keeps its place in the list", async () => {
respondWith([ respondWith([
fakeEthItem({ fakeEthItem({
@@ -274,6 +516,33 @@ describe("surface 3: the balance list", () => {
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]); expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
}); });
// The adjacent finding from the same review as issue #260: the type gate
// compared exactly, so an explorer that ever varied the casing would
// silently drop a real holding before any filter ran. The comparison is
// now case-insensitive, which changes nothing about which types are
// admitted.
test("a differently-cased ERC-20 type still lists a real holding", async () => {
respondWith([
fakeEthItem({
type: "erc-20",
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");
});
test("case insensitivity does not admit another token type", async () => {
respondWith([fakeEthItem({ type: "erc-721" })]);
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
respondWith([fakeEthItem({ type: "ERC-20-EXTRA" })]);
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
});
// The money test: the user holds real ETH and has been airdropped a fake // 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 // ETH ERC-20. The fake is gone from the list of tokens; the real balance
// is exactly what the node reported. // is exactly what the node reported.