fix: fold invisible characters before the known-symbol spoof check (closes #260)
All checks were successful
check / check (push) Successful in 36s
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.
This commit was merged in pull request #270.
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