From c755a5e944e9c7a9cd7b4e862ca9d3735c34d968 Mon Sep 17 00:00:00 2001 From: clawbot Date: Wed, 12 Aug 2026 13:31:55 +0200 Subject: [PATCH] fix: a shared ticker no longer hides one of its two real tokens (closes #276) Seven bundled tokens were filtered as spoofs at their own address, so a user holding FRAX, TON, REUSD, EURE, MSUSD, MUSD or JPYC could not see or spend the one the wallet happened not to pick. The known-symbol table is derived from the bundled token list, first-wins in market-cap order, so a symbol that appears twice silently condemned its second contract. Both are real tokens from the same fetch and neither is stale -- three pairs are one issuer's old and new contract, four are unrelated issuers sharing a ticker. Picking a winner would have been guessing, and dropping the ambiguous symbols would have ended spoof filtering for those tickers entirely. The table now maps a symbol to the set of addresses that legitimately bear it. A contract outside the set is still a spoof, so the check is not weakened: a third contract bearing any of the seven shared tickers is refused, and that is tested. The filter decides what is fake, not what is worth holding, so a legacy contract stays in the set -- it still holds real balances. A test walks the whole bundled list asserting no token is filtered at its own address, which is the guard whose absence let this ship. --- TODO.md | 14 ++++ src/shared/symbolSpoof.js | 17 +++-- src/shared/tokenList.js | 25 ++++++- tests/symbolSpoof.test.js | 134 +++++++++++++++++++++++++++++++++++-- tests/transactions.test.js | 4 +- 5 files changed, 180 insertions(+), 14 deletions(-) diff --git a/TODO.md b/TODO.md index b9a5818..e318de9 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,20 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-12: `KNOWN_SYMBOLS` now maps a symbol to the set of contract addresses + that bear it, not to one of them. A ticker is not unique: seven of the 512 + bundled tokens — `FRAX`, `REUSD`, `TON`, `EURE`, `MSUSD`, `MUSD` and `JPYC` — + share a symbol with another bundled entry at a different real contract, and + the table, built from the list first-wins, kept only the earlier one. The + other seven were judged spoofs of their own symbol at their own address and + hidden from the balance list, the history and the send selector, so a holder + could not spend them. Both contracts of each pair come from the same CoinGecko + fetch of 2026-02-27, so neither was stale and neither was dropped. + `isSpoofedSymbol()` asks set membership instead of equality, which does not + loosen the rule — a contract outside the set is still a spoof — and a test now + walks `TOKENS` asserting no bundled token is filtered at its own address, + which is the walk the suite lacked + ([#276](https://git.eeqj.de/sneak/AutistMask/issues/276)). - 2026-08-12: The dApp approval round trips are driven end to end in the browser. A test page served by the harness speaks EIP-1193 to the real inpage provider through the real content script, background worker and approval popup diff --git a/src/shared/symbolSpoof.js b/src/shared/symbolSpoof.js index 0c97c4e..af908fe 100644 --- a/src/shared/symbolSpoof.js +++ b/src/shared/symbolSpoof.js @@ -8,12 +8,19 @@ // 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 +// KNOWN_SYMBOLS maps a symbol to the set of lowercased contract addresses +// 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. // +// The value is a set because a ticker is not unique: seven symbols in the +// bundled list belong to two real contracts each, and answering with one of +// them hid the other one's holders' money (issue #276). Membership, not +// equality, is therefore the question — but it is the same question, asked of +// a table that can now state the truth. Every address in a set is one the +// wallet ships as a real token; a contract outside the set is still a spoof. +// // 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, @@ -93,7 +100,7 @@ function isSpoofedSymbol(symbol, contractAddress) { if (!KNOWN_SYMBOLS.has(sym)) return false; const legit = KNOWN_SYMBOLS.get(sym); if (legit === null) return true; - return contract !== normalizeAddress(legit); + return !legit.has(contract); } module.exports = { diff --git a/src/shared/tokenList.js b/src/shared/tokenList.js index eff9dd6..a960105 100644 --- a/src/shared/tokenList.js +++ b/src/shared/tokenList.js @@ -3607,14 +3607,33 @@ for (const t of TOKENS) { TOKEN_BY_ADDRESS.set(t.address.toLowerCase(), t); } -// Build a map of symbol (uppercased) -> legitimate contract address (lowercased). -// Used for spoofed-symbol detection. "ETH" maps to null (native token). +// Build a map of symbol (uppercased) -> the set of contract addresses +// (lowercased) that legitimately bear it. Used for spoofed-symbol detection. +// "ETH" maps to null: the native asset has no contract, so no contract may +// bear its symbol. +// +// The value is a set and not a single address because tickers are not unique +// and the list above proves it: seven of these 512 tokens share a symbol with +// another entry — FRAX, REUSD, TON, EURE, MSUSD, MUSD and JPYC — at two +// different real contracts each, all of them from the same source fetch. A +// one-address-per-symbol table can only answer that by picking a winner, and +// the loser is then a token in our own bundled list that the spoof filter +// hides from the balance list, the history and the send selector at its own +// address, so the user cannot spend it (issue #276). Naming every address +// that bears the symbol is the only shape that says what is true; it does not +// loosen the rule, because a contract outside the set is still a spoof. const KNOWN_SYMBOLS = new Map(); KNOWN_SYMBOLS.set("ETH", null); for (const t of TOKENS) { const upper = t.symbol.toUpperCase(); if (!KNOWN_SYMBOLS.has(upper)) { - KNOWN_SYMBOLS.set(upper, t.address.toLowerCase()); + KNOWN_SYMBOLS.set(upper, new Set()); + } + const addresses = KNOWN_SYMBOLS.get(upper); + // A null entry is the native asset and stays null: an ERC-20 that reports + // the native symbol does not thereby become entitled to it. + if (addresses !== null) { + addresses.add(t.address.toLowerCase()); } } diff --git a/tests/symbolSpoof.test.js b/tests/symbolSpoof.test.js index 5e5ee1a..da45ccf 100644 --- a/tests/symbolSpoof.test.js +++ b/tests/symbolSpoof.test.js @@ -56,7 +56,7 @@ global.chrome = { }; const { isSpoofedSymbol } = require("../src/shared/symbolSpoof"); -const { KNOWN_SYMBOLS } = require("../src/shared/tokenList"); +const { TOKENS, KNOWN_SYMBOLS } = require("../src/shared/tokenList"); const { filterTransactions } = require("../src/shared/transactions"); const { fetchTokenBalances, @@ -284,11 +284,137 @@ describe("the shared rule: symbols that render as a known symbol", () => { // 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) { + for (const [symbol, addresses] of KNOWN_SYMBOLS) { expect(symbol).toBe(symbol.trim()); expect(symbol).toMatch(/^[!-~]+$/); - if (address === null) continue; - expect(isSpoofedSymbol(symbol, address)).toBe(false); + if (addresses === null) continue; + for (const address of addresses) { + expect(isSpoofedSymbol(symbol, address)).toBe(false); + } + } + }); +}); + +// Issue #276: the guard that was missing. The suite walked KNOWN_SYMBOLS, +// which is built from TOKENS, so it could only ever assert that the table +// agrees with itself. Seven symbols appear twice in the bundled list at two +// different real contracts, and the table kept whichever came first, so the +// other seven contracts — tokens in our own shipped list, at their own +// addresses — were judged spoofs and hidden from the balance list, the +// history and the send selector. That is the over-filtering direction: it +// hides a holding the user cannot then spend. +// +// This walk is over TOKENS, the data the wallet actually ships, so it fails +// whenever a bundled token would be filtered at its own address no matter +// which side of the table the mistake is on. +describe("the shipped token list", () => { + test("no bundled token is filtered at its own address", () => { + const filtered = TOKENS.filter((t) => + isSpoofedSymbol(t.symbol, t.address), + ).map((t) => t.symbol + " @ " + t.address); + expect(filtered).toEqual([]); + }); + + // The third failure mode the issue asks about: a symbol whose table entry + // names an address that is in neither the table nor the list would be a + // contract we vouch for and do not ship. There is none, and the table is + // built from the list, so this asserts the derivation has not acquired a + // hand-written entry. + test("every address the table vouches for is a bundled token", () => { + const bundled = new Set(TOKENS.map((t) => t.address.toLowerCase())); + for (const [symbol, addresses] of KNOWN_SYMBOLS) { + if (addresses === null) continue; + expect(addresses.size).toBeGreaterThan(0); + for (const address of addresses) { + expect(address).toBe(address.toLowerCase()); + expect(bundled.has(address)).toBe(true); + // And it is the token that actually reports that symbol. + const token = TOKENS.find( + (t) => t.address.toLowerCase() === address, + ); + expect(token.symbol.toUpperCase()).toBe(symbol); + } + } + }); + + // Both contracts behind a shared ticker must pass, from either side: a + // rule that admits only the one the table happens to visit first is the + // bug, not the fix. + test("both contracts behind a shared ticker are admitted", () => { + const bySymbol = new Map(); + for (const t of TOKENS) { + const upper = t.symbol.toUpperCase(); + if (!bySymbol.has(upper)) bySymbol.set(upper, []); + bySymbol.get(upper).push(t); + } + const shared = [...bySymbol].filter(([, list]) => list.length > 1); + // The shared tickers are a fact about the shipped data; if a future + // list has none, this test would silently assert nothing. + expect(shared.length).toBeGreaterThan(0); + for (const [, list] of shared) { + for (const t of list) { + expect(isSpoofedSymbol(t.symbol, t.address)).toBe(false); + } + } + }); + + // The seven from issue #276, named so that the reconciliation is a fact + // in the suite: each is two real contracts from the same source fetch, + // and the table now holds both rather than the one that came first. + test("the seven shared tickers each name both bundled contracts", () => { + const expected = { + TON: [ + "0x582d872a1b094fc48f5de31d3b73f2d9be47def1", // Toncoin + "0x2be5e8c109e2197d077d13a82daead6a9b3433c5", // Tokamak Network + ], + FRAX: [ + "0x853d955acef822db058eb8505911ed77f175b99e", // Legacy Frax Dollar + "0x3432b6a60d23ca0dfca7761b7ab56459d9c964d0", // Frax (prev. FXS) + ], + REUSD: [ + "0x5086bf358635b81d8c47c66d1c8b9e567db70c72", // Re Protocol reUSD + "0x57ab1e0003f623289cd798b1824be09a793e4bec", // Resupply USD + ], + EURE: [ + "0x39b8b6385416f4ca36a20319f70d28621895279d", // Monerium EUR emoney + "0x3231cb76718cdef2155fc47b5286d82e6eda273f", // Monerium EUR emoney [OLD] + ], + MSUSD: [ + "0x4ba01f22827018b4772cd326c7627fb4956a7c00", // Main Street USD + "0xab5eb14c09d416f0ac63661e57edb7aecdb9befa", // Metronome Synth USD + ], + MUSD: [ + "0xaca92e438df0b2401ff60da7e4337b687a2435da", // MetaMask USD + "0xdd468a1ddc392dcdbef6db6e34e89aa338f9f186", // Mezo USD + ], + JPYC: [ + "0x431d5dff03120afa4bdf332c61a6e1766ef37bdb", // JPY Coin + "0x2370f9d504c7a6e775bf6e14b3f12846b594cd53", // JPY Coin v1 + ], + }; + for (const [symbol, addresses] of Object.entries(expected)) { + expect([...KNOWN_SYMBOLS.get(symbol)].sort()).toEqual( + [...addresses].sort(), + ); + for (const address of addresses) { + expect(isSpoofedSymbol(symbol, address)).toBe(false); + } + } + }); + + // The other direction, on the same symbols: widening the table to hold + // every bundled address for a ticker must not turn it into a pass for + // any other contract. + test("a shared ticker from a third contract is still a spoof", () => { + const bySymbol = new Map(); + for (const t of TOKENS) { + const upper = t.symbol.toUpperCase(); + if (!bySymbol.has(upper)) bySymbol.set(upper, []); + bySymbol.get(upper).push(t); + } + for (const [symbol, list] of bySymbol) { + if (list.length < 2) continue; + expect(isSpoofedSymbol(symbol, FAKE_ETH_CONTRACT)).toBe(true); } }); }); diff --git a/tests/transactions.test.js b/tests/transactions.test.js index 962173b..cd66568 100644 --- a/tests/transactions.test.js +++ b/tests/transactions.test.js @@ -207,8 +207,8 @@ describe("token list assumptions the fixtures rely on", () => { }); test("USDC and WETH map to their genuine lowercased contracts", () => { - expect(KNOWN_SYMBOLS.get("USDC")).toBe(USDC_CONTRACT); - expect(KNOWN_SYMBOLS.get("WETH")).toBe(WETH_CONTRACT); + expect([...KNOWN_SYMBOLS.get("USDC")]).toEqual([USDC_CONTRACT]); + expect([...KNOWN_SYMBOLS.get("WETH")]).toEqual([WETH_CONTRACT]); }); test("the spam fixture symbol is not in the known token list", () => {