fix: judge the symbol a user sees, not the bytes a contract returns (closes #260)
All checks were successful
check / check (push) Successful in 35s
All checks were successful
check / check (push) Successful in 35s
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, then
every character that paints nothing removed wherever it sits, then trimmed,
then uppercased. All three surfaces inherit it unchanged.
The strip is "renders as nothing", spelled as \p{Cf} plus
\p{Default_Ignorable_Code_Point} plus U+007F. The classes are the spelling
and not the rule, which is why U+007F is named on its own: it is a control
rather than a default-ignorable character, so no class reaches it, yet it
measures 32.00px in the repo's pinned e2e Chromium at 16px sans-serif --
exactly a plain ETH -- so it paints nothing and would otherwise be a live
bypass. The remaining C0/C1 controls measure 48.00px, a visible box, and are
left alone; both directions are pinned by tests, and widening the strip to
\p{Cc} fails the visible-controls test.
Covered: ASCII and Unicode whitespace padding, zero-width and other
invisible 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, the visible
controls, 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:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user