Compare commits

..

2 Commits

Author SHA1 Message Date
dbd83fee02 test: drive the EIP-1193 dApp approval round trips in the browser (closes #183)
All checks were successful
check / check (push) Successful in 30s
The seam between the content script, the inpage provider, the background worker
and the approval popup had no coverage at all. The unit suite covers each side
in isolation, so a wallet that signed the wrong payload, handed back a signature
from the wrong key, hung on a rejected prompt, or put the user's password back
on the extension messaging boundary would have passed everything.

Ten end-to-end tests now drive it. The route handler serves a local test page on
a reserved-TLD origin; window.ethereum arrives there from the shipped MAIN-world
content script, not from anything the fixture installs, and the page's own
EIP-6963 announcement is required to be that provider by object identity. The
page then speaks eth_requestAccounts, personal_sign, eth_signTypedData_v4 and
eth_sendTransaction through the real prompts, approved and rejected.

The assertions are the point:

  - every signature is recovered in the runner with verifyMessage,
    verifyTypedData and Transaction.from(), and compared against the active
    address read out of extension storage. The background verifies too; nothing
    here leans on its verdict.
  - the transaction is checked against the raw signed bytes captured at
    eth_sendRawTransaction, which the RPC stub now records and answers with the
    transaction's real hash. Signer, recipient, value, call data and chain id
    are all compared there, and the hash the page received is required to be the
    hash of those bytes.
  - a rejected prompt must reach the page as a rejection rather than hang or
    resolve, and must carry EIP-1193 code 4001 across the boundary. The code is
    asserted on the wire because that is where it survives: the inpage provider
    rebuilds the rejection as a bare Error, so the calling page catches a message
    and no code. Reported, not asserted either way.
  - the password must appear in no message the approval window sends to the
    background, observed directly by wrapping chrome.runtime.sendMessage before
    Approve is clicked, with the response message that would carry it required
    to be present so the check cannot pass on an empty record.

Every one of those was run against a deliberately broken variant and seen to
fail: a corrupted recovered signer, a payload carrying the password again, a
provider that resolves instead of rejecting, and approval screens displaying the
wrong message, value and call data.

Two honest limits. The RPC is stubbed throughout, so this does not discharge a
real dApp with real funds against a real network. And the site-connection prompt
goes through chrome.action.openPopup(), whose browser-action popup headless
Chromium will not expose as a page, so that one prompt is driven at the URL the
extension puts on the action instead — same page, same approval id, but a real
toolbar click is not observable from a headless harness. Both are stated in
README.md rather than presented as covered.

Test-only: nothing under src/ changes.
2026-08-12 11:18:24 +00:00
e4c3708b84 fix: fold invisible characters before the known-symbol spoof check (closes #260)
All checks were successful
check / check (push) Successful in 36s
A token calling itself " ETH " missed the known-symbol table entirely, so the
spoof check reported it was not a spoof -- while HTML collapsed the whitespace
and displayed it as ETH next to the user's real ETH. One space defeated the
filter.

The symbol is now folded before the lookup: NFKC, remove what paints nothing,
trim, uppercase. The rule is "remove what paints nothing"; the Unicode classes
are how that is spelled, which is why U+007F is named separately -- it is a
control, reached by no class, and measures identical to no character at all.

Every width in the module comment was measured in the pinned browser rather
than reasoned about, and the boundary is pinned from both sides: widening to
all control characters fails the visible-controls test, narrowing back fails
the invisible-characters test. Two default-ignorable code points do paint a
box and are folded anyway, which can only hide a token that does not resemble
the symbol it folds to -- the harmless direction, recorded rather than glossed.

Confusables that are distinct letters, bidi reordering and interior whitespace
are knowingly left open and asserted open by tests.
2026-08-12 13:06:54 +02:00
4 changed files with 350 additions and 2 deletions

14
TODO.md
View File

@@ -56,6 +56,20 @@ undefined identifiers, which is how
that gives [#157](https://git.eeqj.de/sneak/AutistMask/issues/157) a permanent
floor. This does not discharge a real dApp with real funds against mainnet
([#183](https://git.eeqj.de/sneak/AutistMask/issues/183)).
- 2026-08-12: The known-symbol spoof rule now judges the symbol a user actually
sees. `isSpoofedSymbol()` normalizes before the lookup — NFKC, then every
character that paints nothing removed (the format and default-ignorable
characters, plus U+007F), then trimmed — so `" ETH "`, a no-break space, a
zero-width space, a Hangul filler, a variation selector, a DELETE 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 and the visible C0/C1 controls — which measure 48.00px, a box,
in the pinned e2e Chromium where an invisible prefix measures 32.00px — 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: A containerized Firefox end-to-end harness
(`make test-e2e-firefox`) drives the real popup in a real Firefox with the MV2
build installed as a temporary add-on. Zero npm dependencies — a WebDriver

View File

@@ -66,7 +66,12 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
const balances = [];
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 bal = formatTokenBalance(item.value || "0", decimals);
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
// 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 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");
@@ -22,6 +27,59 @@ function normalizeAddress(addr) {
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 what paints nothing: \p{Cf} plus
// \p{Default_Ignorable_Code_Point} plus U+007F. That covers
// the format characters (zero-width space, joiner and
// non-joiner, word joiner, soft hyphen, byte-order mark, bidi
// marks and overrides), the variation selectors, the Hangul
// fillers, and DELETE. 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 rule is "strip what paints nothing". The Unicode classes are how
// that is spelled, not what it means, which is why U+007F is named on its
// own: it is a control rather than a default-ignorable character, so no
// class here reaches it, yet it paints nothing all the same. Measured in
// the repo's pinned e2e Chromium (16px sans-serif, plain `ETH` = 32.00px,
// so an invisible prefix leaves 32.00px):
//
// U+007F, U+3164, U+115F, U+FE0F, U+FE00 32.00px — invisible
// U+FFA0 40.00px — a box
// U+1160 48.00px — a box
// U+0001, U+0085, U+0090 48.00px — a box
//
// U+1160 and U+FFA0 are `Default_Ignorable_Code_Point` members that font
// fallback nonetheless draws, and they are stripped anyway: erring toward
// hiding a token that does not look like `ETH` is the harmless direction of
// the two. The other controls are left alone for the same reason read the
// other way — a symbol carrying a visible box does not reach the eye as
// `ETH`, so filtering it would hide a token the user could not have
// confused with the native asset.
//
// 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), bidi reordering, which needs the
// bidi algorithm rather than a character filter, and the visible controls.
//
// 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}\x7F]/gu, "")
.trim()
.toUpperCase();
}
// True when a token bearing `symbol` from contract `contractAddress` is
// impersonating a known symbol.
//
@@ -31,7 +89,7 @@ function normalizeAddress(addr) {
function isSpoofedSymbol(symbol, contractAddress) {
const contract = normalizeAddress(contractAddress);
if (!contract) return false;
const sym = (symbol || "").toUpperCase();
const sym = normalizeSymbol(symbol);
if (!KNOWN_SYMBOLS.has(sym)) return false;
const legit = KNOWN_SYMBOLS.get(sym);
if (legit === null) return true;

View File

@@ -124,6 +124,175 @@ 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);
});
// Nor is it confined to the Unicode classes. U+007F is a control (Cc)
// and is not default-ignorable, so neither class reaches it, but it
// measures 32.00px in the same browser — it paints nothing, so a
// symbol carrying it reaches the eye as `ETH`. It is named on its own
// in the strip for exactly that reason.
test("U+007F paints nothing and is stripped", () => {
expect(isSpoofedSymbol(DEL + "ETH", FAKE_ETH_CONTRACT)).toBe(true);
});
// The other side of the boundary, which is not the class boundary but
// the visibility one: the remaining 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. Widening
// the strip to \p{Cc} — the obvious over-correction once U+007F is in
// it — fails this test.
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,
);
});
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", () => {
function fakeEthTransfer() {
return {
@@ -147,6 +316,22 @@ describe("surface 1: the transaction history", () => {
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", () => {
const native = {
hash: "0x" + "2".repeat(64),
@@ -201,6 +386,36 @@ describe("surface 2: the Send token selector", () => {
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", () => {
render([]);
expect(select.innerHTML).toBe('<option value="ETH">ETH</option>');
@@ -251,6 +466,35 @@ describe("surface 3: the balance list", () => {
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 () => {
respondWith([
fakeEthItem({
@@ -274,6 +518,33 @@ describe("surface 3: the balance list", () => {
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
// ETH ERC-20. The fake is gone from the list of tokens; the real balance
// is exactly what the node reported.