diff --git a/README.md b/README.md index 2ed0dc3..6981b64 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,7 @@ src/ prices.js — ETH/USD and token/USD via CoinDesk API scamlist.js — known fraud contract addresses 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) transactions.js — tx history fetching + anti-poisoning filters uniswap.js — Uniswap Universal Router calldata decoder @@ -450,11 +451,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 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 -list from any other contract address is always dropped. That filter is -unconditional — the "Hide tokens with fewer than 1,000 holders" setting governs -the transaction history 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. +list from any other contract address is always dropped, and so is any token +claiming a symbol that belongs to the native asset and therefore has no +legitimate contract at all (`"ETH"`). That filter is unconditional — the "Hide +tokens with fewer than 1,000 holders" setting governs the transaction history +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 @@ -1246,14 +1248,15 @@ indexes it as a real token transfer. 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; 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 - unconditionally, because it decides which tokens the user can act on rather - than what the history displays. The balance list applies it unconditionally - too, but not identically: it exempts symbols that `KNOWN_SYMBOLS` maps to - `null`, and `"ETH"` is the only one. So the fake "Ethereum" token above is - filtered from the transaction history and from the send selector, but a - fake-`ETH` ERC-20 that clears the balance list's own 1,000-holder floor — or - that the user tracked manually — is still shown in the balance list. + learned from them. The send-screen token selector and the balance list apply + the same check unconditionally, because they decide which tokens the user can + act on and what the user believes they own rather than what the history + displays. All three surfaces read the rule from `src/shared/symbolSpoof.js`, + so they cannot answer the question differently. A symbol the list maps to no + contract at all — `"ETH"`, the native asset, is the only one — may be borne by + no contract, so every ERC-20 claiming it is a spoof on all three. The user's + 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 fewer than 1,000 holders are hidden from transaction history by default. @@ -1289,13 +1292,12 @@ indexes it as a real token transfer. a sharp tool — users who understand the risks can configure the wallet to show everything unfiltered, unix-style. All four settings govern the transaction 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 - except for symbols mapped to `null` (`"ETH"` alone), which the balance list - does not filter. The fraud contract blocklist is applied unconditionally on - that selector and is not consulted by the balance list at all. The low-holder - setting also gates the send selector, while the balance list's own - 1,000-holder floor is unconditional (see Data Model). The dust threshold - applies to the transaction history alone. + unconditionally on the send-screen token selector and on the balance list, in + both cases identically to the history. The fraud contract blocklist is applied + unconditionally on that selector and is not consulted by the balance list at + all. The low-holder setting also gates the send selector, while the balance + list's own 1,000-holder floor is unconditional (see Data Model). The dust + threshold applies to the transaction history alone. #### Phishing Domain Protection diff --git a/TODO.md b/TODO.md index eb6dd07..fd8fab0 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,15 @@ undefined identifiers, which is how # Completed Steps +- 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: 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 the wallet list, and blocked from signing, sending and private-key export diff --git a/src/popup/views/send.js b/src/popup/views/send.js index 6884764..85cadbf 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -12,8 +12,9 @@ const { const { state, currentAddress } = require("../../shared/state"); let ctx; const { getProvider } = require("../../shared/balances"); -const { KNOWN_SYMBOLS, resolveSymbol } = require("../../shared/tokenList"); +const { resolveSymbol } = require("../../shared/tokenList"); const { isLowHolderCount } = require("../../shared/holders"); +const { isSpoofedSymbol } = require("../../shared/symbolSpoof"); const { getAddress } = require("ethers"); 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) { const sel = $("send-token"); sel.innerHTML = ''; @@ -131,7 +124,7 @@ function renderSendTokenSelect(addr) { (state.fraudContracts || []).map((a) => a.toLowerCase()), ); for (const t of addr.tokenBalances || []) { - if (isSpoofedToken(t)) continue; + if (isSpoofedSymbol(t.symbol, t.address)) continue; if (fraudSet.has(t.address.toLowerCase())) continue; // An unknown holder count does not withhold a token the user holds: // only a count the explorer actually reported as below the threshold diff --git a/src/shared/balances.js b/src/shared/balances.js index 1ca5be5..a66bc5c 100644 --- a/src/shared/balances.js +++ b/src/shared/balances.js @@ -11,8 +11,9 @@ const { const { ERC20_ABI } = require("./constants"); const { log, debugFetch } = require("./log"); 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 { isSpoofedSymbol } = require("./symbolSpoof"); // Use a static network to skip auto-detection (which can fail and cause // "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 if (!isKnown && !isTracked && !hasEnoughHolders) continue; - // Skip tokens spoofing a known symbol from a different address - const sym = (item.token.symbol || "").toUpperCase(); - const legitAddr = KNOWN_SYMBOLS.get(sym); - if ( - legitAddr !== undefined && - legitAddr !== null && - tokenAddr !== legitAddr - ) - continue; + // Skip tokens spoofing a known symbol from a different address. + // Every row here is an ERC-20 the explorer reported, so it has a + // contract address; the native ETH balance is fetched over RPC in + // refreshBalances and never passes through this loop. + if (isSpoofedSymbol(item.token.symbol, tokenAddr)) continue; balances.push({ address: item.token.address_hash, diff --git a/src/shared/symbolSpoof.js b/src/shared/symbolSpoof.js new file mode 100644 index 0000000..b106528 --- /dev/null +++ b/src/shared/symbolSpoof.js @@ -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, +}; diff --git a/src/shared/transactions.js b/src/shared/transactions.js index b4c9638..0a87103 100644 --- a/src/shared/transactions.js +++ b/src/shared/transactions.js @@ -8,8 +8,9 @@ const { formatEther, formatUnits } = require("ethers"); 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 { isSpoofedSymbol } = require("./symbolSpoof"); // Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum // 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; } -// 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, // returns { transactions, newFraudContracts }. function filterTransactions(txs, filters = {}) { @@ -283,7 +272,7 @@ function filterTransactions(txs, filters = {}) { const contract = normalizeAddress(tx.contractAddress); // 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)) { fraudSet.add(contract); newFraud.push(contract); diff --git a/tests/symbolSpoof.test.js b/tests/symbolSpoof.test.js new file mode 100644 index 0000000..b0e612a --- /dev/null +++ b/tests/symbolSpoof.test.js @@ -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(''); + }); +}); + +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(); + }); +});