sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

fix: judge the symbol a user sees, not the bytes a contract returns (closes #260) #270

Merged
clawbot merged 1 commits from fix/issue-260-padded-symbol-spoof into next 2026-08-12 13:06:55 +02:00
Conversation 4 Commits 1 Files Changed 4 +350 -2
clawbot commented 2026-08-12 11:49:19 +02:00
Collaborator
Copy Link

Closes #260.

isSpoofedSymbol() compared the raw contract-returned symbol against
KNOWN_SYMBOLS, so " ETH " missed the table on all three surfaces while
HTML collapsed the padding and painted it as ETH next to the user's real
ETH. Normalization now happens inside src/shared/symbolSpoof.js, so the
history, the send selector and the balance list inherit it with no call-site
change.

The normalization, and why this far

String(symbol || "")
    .normalize("NFKC")
    .replace(/[\p{Cf}\p{Default_Ignorable_Code_Point}\x7F]/gu, "")
    .trim()
    .toUpperCase()

The question the rule asks is "does this reach the user's eye as a known
symbol", so the folding goes exactly as far as the rendering does and no
further.

The rule the strip implements is "remove what paints nothing." The
Unicode classes are how that is spelled, not what it means — which is why
U+007F DELETE is named on its own: it is Cc and not default-ignorable, so
no class in the strip reaches it, yet it measures the same as no character
at all. A property name is not the boundary; measured invisibility is.

Covered:

  • ASCII whitespace padding — " ETH ", tabs, newlines.
  • Non-ASCII spaces — U+00A0 no-break, U+2007 figure, U+3000 ideographic;
    NFKC maps them onto U+0020 and trim() removes them.
  • Characters that paint nothing, anywhere in the string: the format
    characters \p{Cf} (U+200B zero-width space, U+200C/U+200D joiners,
    U+2060 word joiner, U+00AD soft hyphen, U+FEFF BOM, U+200E/U+200F bidi
    marks), \p{Default_Ignorable_Code_Point}, which is where most of the
    rest of invisibility lives — \p{Cf} alone was too narrow, since a
    Hangul filler is Lo and a variation selector is Mn — and U+007F,
    which neither class covers.
  • Compatibility variants that NFKC folds onto ASCII: fullwidth ＥＴＨ,
    styled mathematical letters.

What "invisible" means here, measured rather than asserted

Rendering is not a definition, so it was measured — 16px sans-serif span in
the repo's pinned e2e Chromium (mcr.microsoft.com/playwright
v1.56.0-noble, the same digest script/test-e2e pins), where a plain ETH
is 32.00px, so an invisible prefix leaves 32.00px:

symbol width isSpoofedSymbol
ETH baseline 32.00px —
U+007F DELETE + ETH 32.00px true
U+3164 HANGUL FILLER + ETH 32.00px true
U+115F HANGUL CHOSEONG FILLER + ETH 32.00px true
ETH + U+FE0F VARIATION SELECTOR-16 32.00px true
E + U+FE00 VARIATION SELECTOR-1 + TH 32.00px true
U+200B ZERO WIDTH SPACE + ETH 32.00px true
U+034F COMBINING GRAPHEME JOINER + ETH 32.00px true
U+2065 (unassigned, ignorable) + ETH 32.00px true
U+0085 NEL + ETH 48.00px false
U+0001 + ETH 48.00px false
U+0090 + ETH 48.00px false
U+FFA0 HALFWIDTH HANGUL FILLER + ETH 40.00px true
U+1160 HANGUL JUNGSEONG FILLER + ETH 48.00px true

U+007F was the finding of the last review round, and it is closed here. The
review had it at 48.00px, i.e. a visible box and correctly excluded; the
measurement above says 32.00px, which makes it a live bypass, and the
measurement is what this change follows. Re-measured on this head, in the
same pinned container, with the visible controls measured alongside it in
the same run.

Two members of the stripped class do not render as nothing, and saying
otherwise would be the same kind of overclaim an earlier revision made:
U+1160 measures 48.00px and U+FFA0 40.00px, because font fallback draws a
box. Both are stripped anyway — they are Default_Ignorable_Code_Point, and
NFKC folds U+3164 and U+FFA0 onto U+1160 before the strip runs. The error
that introduces is hiding a token that does not look like ETH, which is
the harmless direction, and no bundled symbol is affected (below).

Knowingly left open, each asserted as open by a test so the boundary is a
fact in the suite rather than a claim here:

  • Confusables that are distinct letters, not compatibility variants —
    Cyrillic capital Ie (U+0415), Greek capital Epsilon (U+0395). NFKC does
    not touch them by design. Closing this needs a confusable-skeleton table
    (UTS #39), which is a separate change with its own false-positive
    question.
  • Bidi reordering — U+202E followed by HTE renders as ETH. Dropping
    the control character leaves HTE, which does not match; undoing the
    reordering needs the bidi algorithm, not a character filter.
  • Interior whitespace — E T H is deliberately not folded, because it
    renders as E T H. Folding it would filter a token that presents no
    confusion at all.
  • The remaining C0 and C1 controls — measured at 48.00px each (U+0085
    NEL, U+0001, U+0090), i.e. a visible box, so a symbol carrying one does
    not look like ETH and must not be judged a spoof. This is the reason
    U+007F is added by itself rather than by widening to \p{Cc}: the class
    is not the rule, and widening to it fails the suite.

Normalization decides only how the question is asked. Nothing here changes
what any surface displays; a token still shows the symbol it reports.

False positives

Bundled data. No entry in KNOWN_SYMBOLS (506 symbols) and no symbol in
the bundled token list (512 tokens) contains whitespace or any non-ASCII
character, so the normalization cannot newly filter a bundled token. Nearest
neighbours are hyphenated (MF-ONE) and numeric (0G, 69420), none of
which normalization touches. A test walks the whole table and asserts both
properties plus isSpoofedSymbol(symbol, itsOwnAddress) === false for every
entry, so a future list entry with a space fails the suite rather than
silently disappearing from users' balance lists.

Arbitrary explorer-supplied symbols, which is what the filter actually
runs against — stated explicitly rather than left as an unexamined case.
Sweeping every non-ASCII code point: 210 single code points normalize
onto a bundled symbol. U+2121 TELEPHONE SIGN folds to TEL, U+33CC SQUARE
IN folds to IN, U+24BB and U+FF26 both fold to F. This is intended, not
a defect: such a symbol renders as the thing it folds to, which is exactly
the property the rule is built on, and a contract that returns U+2121 while
not being the TEL contract is impersonating TEL on screen. The
consequence is that a token whose symbol is a single compatibility glyph
gets judged against the bundled symbol it looks like, which is the intended
behaviour of the whole change.

The two adjacent findings

  • Mis-cased item.token.type — handled. src/shared/balances.js
    compared !== "ERC-20" exactly, which fails closed for spam but also
    drops a real holding if an explorer ever writes erc-20. The comparison
    is now case-insensitive on an explorer-supplied label. Which types are
    admitted is unchanged: erc-721 and ERC-20-EXTRA are still dropped,
    and both directions are tested.
  • hideSpoofedSymbols off leaves history showing what the balance list
    hides     — ruled out, deliberate, unchanged. Confirmed still the intent
    from #176: the switch
    is a user-controlled escape hatch for the history only, and the balance
    list and send selector have no such switch by design (the balance list is
    where a user forms their belief about what they own). The three surfaces
    agree whenever the user has not explicitly asked the history to show
    everything.

Verification

Failing first, on unmodified next with only the tests applied — 10 tests
failed, on all three surfaces:

the shared rule ... > ASCII padding does not buy a pass
the shared rule ... > non-breaking and other Unicode spaces do not either
the shared rule ... > zero-width characters are stripped wherever they sit
the shared rule ... > an invisible bidi mark does not hide a known symbol
the shared rule ... > compatibility forms fold onto the symbol they imitate
surface 1: the transaction history > a padded fake ETH token transfer is filtered too
surface 2: the Send token selector > a padded fake ETH token is not selectable either
surface 3: the balance list > a padded fake ETH token is filtered too
surface 3: the balance list > a fake ETH token padded with a no-break space is filtered
surface 3: the balance list > a differently-cased ERC-20 type still lists a real holding

The U+007F test is the eleventh, added in this round; its failing-first
evidence is the \x7F mutation row below, which is exactly this branch with
that one character removed from the strip.

make check: green. 659 tests in 27 suites, test-verify-build 18 cases,
prettier clean. make test-e2e: green, 27/27 in the pinned Playwright
container.

Mutation checks — the strip's boundary is pinned from both sides, so neither
narrowing it nor widening it past what is invisible survives:

mutation tests killed
delete the no-contract native guard 21
invert the address comparison 26
drop .trim() 6
drop the strip entirely 3
narrow the strip back to \p{Cf} only 1
drop \x7F from the strip 1
widen the strip to \p{Cc} 1
drop .normalize("NFKC") 1
drop .toUpperCase() 3

The last two strip rows are the two sides of the boundary: dropping \x7F
kills the U+007F test, widening to \p{Cc} kills the visible-controls test.
Widening previously killed 2 because it also killed the assertion that
U+007F was not caught; that assertion is now inverted, so the count is 1
and the discrimination is unchanged.

The native-asset exemption is untouched: it still turns on "has no contract
address" and never on the symbol, the second-null-mapped-symbol test still
passes and still dies under the guard mutation, and the real native ETH
balance never enters the filtered loop.

Invisible characters in the tests are built with String.fromCodePoint from
named constants rather than pasted in, so the source stays reviewable ASCII.
The bundled-symbol guard was /^[ -~]+$/, which admitted an interior space
and was therefore weaker than the "no bundled symbol contains whitespace"
claim it stood for; it is now /^[!-~]+$/. The property held either way.

Closes [#260](https://git.eeqj.de/sneak/AutistMask/issues/260). `isSpoofedSymbol()` compared the raw contract-returned symbol against `KNOWN_SYMBOLS`, so `" ETH "` missed the table on all three surfaces while HTML collapsed the padding and painted it as `ETH` next to the user's real ETH. Normalization now happens inside `src/shared/symbolSpoof.js`, so the history, the send selector and the balance list inherit it with no call-site change. ## The normalization, and why this far ``` String(symbol || "") .normalize("NFKC") .replace(/[\p{Cf}\p{Default_Ignorable_Code_Point}\x7F]/gu, "") .trim() .toUpperCase() ``` The question the rule asks is "does this reach the user's eye as a known symbol", so the folding goes exactly as far as the rendering does and no further. **The rule the strip implements is "remove what paints nothing."** The Unicode classes are how that is spelled, not what it means — which is why U+007F DELETE is named on its own: it is `Cc` and not default-ignorable, so no class in the strip reaches it, yet it measures the same as no character at all. A property name is not the boundary; measured invisibility is. Covered: - ASCII whitespace padding — `" ETH "`, tabs, newlines. - Non-ASCII spaces — U+00A0 no-break, U+2007 figure, U+3000 ideographic; NFKC maps them onto U+0020 and `trim()` removes them. - Characters that paint nothing, anywhere in the string: the format characters `\p{Cf}` (U+200B zero-width space, U+200C/U+200D joiners, U+2060 word joiner, U+00AD soft hyphen, U+FEFF BOM, U+200E/U+200F bidi marks), `\p{Default_Ignorable_Code_Point}`, which is where most of the rest of invisibility lives — `\p{Cf}` alone was too narrow, since a Hangul filler is `Lo` and a variation selector is `Mn` — and U+007F, which neither class covers. - Compatibility variants that NFKC folds onto ASCII: fullwidth `ＥＴＨ`, styled mathematical letters. ### What "invisible" means here, measured rather than asserted Rendering is not a definition, so it was measured — 16px sans-serif span in the repo's pinned e2e Chromium (`mcr.microsoft.com/playwright` v1.56.0-noble, the same digest `script/test-e2e` pins), where a plain `ETH` is 32.00px, so an invisible prefix leaves 32.00px: | symbol | width | `isSpoofedSymbol` | | --- | --- | --- | | `ETH` baseline | 32.00px | — | | U+007F DELETE + `ETH` | 32.00px | `true` | | U+3164 HANGUL FILLER + `ETH` | 32.00px | `true` | | U+115F HANGUL CHOSEONG FILLER + `ETH` | 32.00px | `true` | | `ETH` + U+FE0F VARIATION SELECTOR-16 | 32.00px | `true` | | `E` + U+FE00 VARIATION SELECTOR-1 + `TH` | 32.00px | `true` | | U+200B ZERO WIDTH SPACE + `ETH` | 32.00px | `true` | | U+034F COMBINING GRAPHEME JOINER + `ETH` | 32.00px | `true` | | U+2065 (unassigned, ignorable) + `ETH` | 32.00px | `true` | | U+0085 NEL + `ETH` | 48.00px | `false` | | U+0001 + `ETH` | 48.00px | `false` | | U+0090 + `ETH` | 48.00px | `false` | | U+FFA0 HALFWIDTH HANGUL FILLER + `ETH` | 40.00px | `true` | | U+1160 HANGUL JUNGSEONG FILLER + `ETH` | 48.00px | `true` | U+007F was the finding of the last review round, and it is closed here. The review had it at 48.00px, i.e. a visible box and correctly excluded; the measurement above says 32.00px, which makes it a live bypass, and the measurement is what this change follows. Re-measured on this head, in the same pinned container, with the visible controls measured alongside it in the same run. Two members of the stripped class do **not** render as nothing, and saying otherwise would be the same kind of overclaim an earlier revision made: U+1160 measures 48.00px and U+FFA0 40.00px, because font fallback draws a box. Both are stripped anyway — they are `Default_Ignorable_Code_Point`, and NFKC folds U+3164 and U+FFA0 onto U+1160 before the strip runs. The error that introduces is hiding a token that does *not* look like `ETH`, which is the harmless direction, and no bundled symbol is affected (below). Knowingly left open, each asserted as open by a test so the boundary is a fact in the suite rather than a claim here: - **Confusables that are distinct letters**, not compatibility variants — Cyrillic capital Ie (U+0415), Greek capital Epsilon (U+0395). NFKC does not touch them by design. Closing this needs a confusable-skeleton table (UTS #39), which is a separate change with its own false-positive question. - **Bidi reordering** — U+202E followed by `HTE` renders as `ETH`. Dropping the control character leaves `HTE`, which does not match; undoing the reordering needs the bidi algorithm, not a character filter. - **Interior whitespace** — `E T H` is deliberately not folded, because it renders as `E T H`. Folding it would filter a token that presents no confusion at all. - **The remaining C0 and C1 controls** — measured at 48.00px each (U+0085 NEL, U+0001, U+0090), i.e. a visible box, so a symbol carrying one does not look like `ETH` and must not be judged a spoof. This is the reason U+007F is added by itself rather than by widening to `\p{Cc}`: the class is not the rule, and widening to it fails the suite. Normalization decides only how the question is asked. Nothing here changes what any surface displays; a token still shows the symbol it reports. ## False positives **Bundled data.** No entry in `KNOWN_SYMBOLS` (506 symbols) and no symbol in the bundled token list (512 tokens) contains whitespace or any non-ASCII character, so the normalization cannot newly filter a bundled token. Nearest neighbours are hyphenated (`MF-ONE`) and numeric (`0G`, `69420`), none of which normalization touches. A test walks the whole table and asserts both properties plus `isSpoofedSymbol(symbol, itsOwnAddress) === false` for every entry, so a future list entry with a space fails the suite rather than silently disappearing from users' balance lists. **Arbitrary explorer-supplied symbols**, which is what the filter actually runs against — stated explicitly rather than left as an unexamined case. Sweeping every non-ASCII code point: **210** single code points normalize onto a bundled symbol. U+2121 TELEPHONE SIGN folds to `TEL`, U+33CC SQUARE IN folds to `IN`, U+24BB and U+FF26 both fold to `F`. This is intended, not a defect: such a symbol *renders as* the thing it folds to, which is exactly the property the rule is built on, and a contract that returns U+2121 while not being the `TEL` contract is impersonating `TEL` on screen. The consequence is that a token whose symbol is a single compatibility glyph gets judged against the bundled symbol it looks like, which is the intended behaviour of the whole change. ## The two adjacent findings - **Mis-cased `item.token.type`** — handled. `src/shared/balances.js` compared `!== "ERC-20"` exactly, which fails closed for spam but also drops a real holding if an explorer ever writes `erc-20`. The comparison is now case-insensitive on an explorer-supplied label. Which types are admitted is unchanged: `erc-721` and `ERC-20-EXTRA` are still dropped, and both directions are tested. - **`hideSpoofedSymbols` off leaves history showing what the balance list hides** — ruled out, deliberate, unchanged. Confirmed still the intent from [#176](https://git.eeqj.de/sneak/AutistMask/issues/176): the switch is a user-controlled escape hatch for the history only, and the balance list and send selector have no such switch by design (the balance list is where a user forms their belief about what they own). The three surfaces agree whenever the user has not explicitly asked the history to show everything. ## Verification Failing first, on unmodified `next` with only the tests applied — 10 tests failed, on all three surfaces: ``` the shared rule ... > ASCII padding does not buy a pass the shared rule ... > non-breaking and other Unicode spaces do not either the shared rule ... > zero-width characters are stripped wherever they sit the shared rule ... > an invisible bidi mark does not hide a known symbol the shared rule ... > compatibility forms fold onto the symbol they imitate surface 1: the transaction history > a padded fake ETH token transfer is filtered too surface 2: the Send token selector > a padded fake ETH token is not selectable either surface 3: the balance list > a padded fake ETH token is filtered too surface 3: the balance list > a fake ETH token padded with a no-break space is filtered surface 3: the balance list > a differently-cased ERC-20 type still lists a real holding ``` The U+007F test is the eleventh, added in this round; its failing-first evidence is the `\x7F` mutation row below, which is exactly this branch with that one character removed from the strip. `make check`: green. 659 tests in 27 suites, `test-verify-build` 18 cases, prettier clean. `make test-e2e`: green, 27/27 in the pinned Playwright container. Mutation checks — the strip's boundary is pinned from both sides, so neither narrowing it nor widening it past what is invisible survives: | mutation | tests killed | | --- | --- | | delete the no-contract native guard | 21 | | invert the address comparison | 26 | | drop `.trim()` | 6 | | drop the strip entirely | 3 | | narrow the strip back to `\p{Cf}` only | 1 | | drop `\x7F` from the strip | 1 | | widen the strip to `\p{Cc}` | 1 | | drop `.normalize("NFKC")` | 1 | | drop `.toUpperCase()` | 3 | The last two strip rows are the two sides of the boundary: dropping `\x7F` kills the U+007F test, widening to `\p{Cc}` kills the visible-controls test. Widening previously killed 2 because it also killed the assertion that U+007F was *not* caught; that assertion is now inverted, so the count is 1 and the discrimination is unchanged. The native-asset exemption is untouched: it still turns on "has no contract address" and never on the symbol, the second-null-mapped-symbol test still passes and still dies under the guard mutation, and the real native ETH balance never enters the filtered loop. Invisible characters in the tests are built with `String.fromCodePoint` from named constants rather than pasted in, so the source stays reviewable ASCII. The bundled-symbol guard was `/^[ -~]+$/`, which admitted an interior space and was therefore weaker than the "no bundled symbol contains whitespace" claim it stood for; it is now `/^[!-~]+$/`. The property held either way.
clawbot added 1 commit 2026-08-12 11:49:19 +02:00
fix: judge the symbol a user sees, not the bytes a contract returns (closes #260)
All checks were successful
check / check (push) Successful in 40s
Details
13bf482327 
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, every
Unicode format character removed wherever it sits, then trimmed, then
uppercased. All three surfaces inherit it unchanged.

Covered: ASCII and Unicode whitespace padding, zero-width and other
invisible format 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, 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.
clawbot added the needs-review label 2026-08-12 11:49:30 +02:00
clawbot self-assigned this 2026-08-12 11:49:30 +02:00
clawbot commented 2026-08-12 12:04:39 +02:00
Author
Collaborator
Copy Link

FAIL — needs-rework (a rebase is also required).

1. src/shared/symbolSpoof.js:56 — the invisible-character strip is narrower than the boundary this PR claims, and four classes that render pixel-identically to ETH bypass the filter

\p{Cf} is used as a proxy for "renders as nothing", but invisibility is not confined to Cf. Measured in the pinned e2e Chromium (mcr.microsoft.com/playwright@sha256:35246d87...), each string rendered in a 16px sans-serif span, width compared against plain ETH (32.00px):

symbol rendered width isSpoofedSymbol(sym, fakeContract)
ETH (baseline) 32.00px —
U+3164 HANGUL FILLER + ETH 32.00px false
U+115F HANGUL CHOSEONG FILLER + ETH 32.00px false
ETH + U+FE0F VARIATION SELECTOR-16 32.00px false
E + U+FE00 VARIATION SELECTOR-1 + TH 32.00px false

Reproduction:

const { isSpoofedSymbol } = require("./src/shared/symbolSpoof");
const FAKE = "0x" + "9".repeat(40);
isSpoofedSymbol(String.fromCodePoint(0x3164) + "ETH", FAKE); // false
isSpoofedSymbol("ETH" + String.fromCodePoint(0xfe0f), FAKE); // false

The Hangul fillers are Lo and the variation selectors are Mn, so neither is caught, yet both are default-ignorable and paint nothing. This is exactly the issue #260 failure mode — a fake token sitting next to the user's real ETH reading ETH — reachable by swapping one character for another. U+3164 is the blank character in common use for exactly this purpose.

The claim is wrong as well as the coverage: the PR body and the comment at src/shared/symbolSpoof.js:35-39 justify the strip with "These render as nothing at all, anywhere in the string", and that justification does not hold for the set actually stripped. Either the set or the claim has to change.

Acceptable, and verified here: strip the Unicode default-ignorable set alongside Cf —

.replace(/[\p{Cf}\p{Default_Ignorable_Code_Point}]/gu, "")

All four cases above become true, and the full suite stays 595/595 green, so nothing legitimate is newly folded and the bundled-symbol walk still holds. The genuinely-open classes stay open and correctly so: U+0085 NEL, the C0/C1 controls and U+007F all render as a visible box (48px, wider than baseline), as do U+1160 and U+FFA0. Add each of the four as a test, in the same code-point style the file already uses.

2. Not fast-forwardable onto current origin/next

origin/next is at 18b47cd; head 13bf482 is parented on 5af89a1. git rebase origin/next conflicts in TODO.md — both sides add a bullet at the top of # Completed Steps. Rebase, keep both entries, re-push.

Verified and passing

DoD items 1-3, 5 and 6; failing-first reproduced exactly (10 tests, all three surfaces, on 5af89a1 with only the test file applied); mutation kills reproduce as claimed (native guard 21, inverted address comparison 26, .trim() 6, \p{Cf} 2) plus NFKC 1 and .toUpperCase() 3, no survivors; the #257 property (native exemption keyed on absence of a contract address, second-null-mapped-symbol test) intact; the balances.js type gate admits erc-20 and still drops erc-721/ERC-20-EXTRA, and case folding admits no other Blockscout type; make check 595 tests / 25 suites executed, test-verify-build 18 cases, script/cibuild with RUN make check executing uncached (18.7s, not CACHED), make test-e2e 27/27, make fmt clean; single commit, author and committer clawbot, one TODO.md bullet at the top of # Completed Steps with no landed entry lost, title ends (closes #260), no attribution trailers, no scope creep.

Disclosures

  • The false-positive analysis covers the bundled data — independently confirmed, 506 KNOWN_SYMBOLS and 512 TOKENS, zero whitespace and zero non-ASCII — but not the population the filter actually runs against, which is arbitrary explorer-supplied symbols. 209 single code points now NFKC-fold onto a bundled symbol (U+2121 TELEPHONE SIGN to TEL, U+33CC SQUARE IN to IN, U+33FF SQUARE GAL to GAL, U+24BB / U+FF26 / U+1D405 to F), so such a token at a non-matching address is now hidden. I judge that intended rather than a defect, since it renders as the symbol it folds to, but it is outside the stated analysis. I could not construct a legitimate token that is now wrongly hidden.
  • tests/symbolSpoof.test.js:238 asserts /^[ -~]+$/, which admits an interior space, so that guard is weaker than "no bundled symbol contains whitespace". The property itself holds — verified independently — and interior whitespace is not folded, so nothing is at risk today.
  • script/check runs prettier on the host; the containerized evidence above comes from script/cibuild.
**FAIL — `needs-rework`** (a rebase is also required). ## 1. `src/shared/symbolSpoof.js:56` — the invisible-character strip is narrower than the boundary this PR claims, and four classes that render pixel-identically to `ETH` bypass the filter `\p{Cf}` is used as a proxy for "renders as nothing", but invisibility is not confined to `Cf`. Measured in the pinned e2e Chromium (`mcr.microsoft.com/playwright@sha256:35246d87...`), each string rendered in a 16px sans-serif span, width compared against plain `ETH` (32.00px): | symbol | rendered width | `isSpoofedSymbol(sym, fakeContract)` | | --- | --- | --- | | `ETH` (baseline) | 32.00px | — | | U+3164 HANGUL FILLER + `ETH` | 32.00px | `false` | | U+115F HANGUL CHOSEONG FILLER + `ETH` | 32.00px | `false` | | `ETH` + U+FE0F VARIATION SELECTOR-16 | 32.00px | `false` | | `E` + U+FE00 VARIATION SELECTOR-1 + `TH` | 32.00px | `false` | Reproduction: ```js const { isSpoofedSymbol } = require("./src/shared/symbolSpoof"); const FAKE = "0x" + "9".repeat(40); isSpoofedSymbol(String.fromCodePoint(0x3164) + "ETH", FAKE); // false isSpoofedSymbol("ETH" + String.fromCodePoint(0xfe0f), FAKE); // false ``` The Hangul fillers are `Lo` and the variation selectors are `Mn`, so neither is caught, yet both are default-ignorable and paint nothing. This is exactly the issue [#260](https://git.eeqj.de/sneak/AutistMask/issues/260) failure mode — a fake token sitting next to the user's real ETH reading `ETH` — reachable by swapping one character for another. U+3164 is the blank character in common use for exactly this purpose. The claim is wrong as well as the coverage: the PR body and the comment at `src/shared/symbolSpoof.js:35-39` justify the strip with "These render as nothing at all, anywhere in the string", and that justification does not hold for the set actually stripped. Either the set or the claim has to change. Acceptable, and verified here: strip the Unicode default-ignorable set alongside `Cf` — ```js .replace(/[\p{Cf}\p{Default_Ignorable_Code_Point}]/gu, "") ``` All four cases above become `true`, and the full suite stays 595/595 green, so nothing legitimate is newly folded and the bundled-symbol walk still holds. The genuinely-open classes stay open and correctly so: U+0085 NEL, the C0/C1 controls and U+007F all render as a visible box (48px, wider than baseline), as do U+1160 and U+FFA0. Add each of the four as a test, in the same code-point style the file already uses. ## 2. Not fast-forwardable onto current `origin/next` `origin/next` is at `18b47cd`; head `13bf482` is parented on `5af89a1`. `git rebase origin/next` conflicts in `TODO.md` — both sides add a bullet at the top of `# Completed Steps`. Rebase, keep both entries, re-push. ## Verified and passing DoD items 1-3, 5 and 6; failing-first reproduced exactly (10 tests, all three surfaces, on `5af89a1` with only the test file applied); mutation kills reproduce as claimed (native guard 21, inverted address comparison 26, `.trim()` 6, `\p{Cf}` 2) plus NFKC 1 and `.toUpperCase()` 3, no survivors; the [#257](https://git.eeqj.de/sneak/AutistMask/pulls/257) property (native exemption keyed on absence of a contract address, second-null-mapped-symbol test) intact; the `balances.js` type gate admits `erc-20` and still drops `erc-721`/`ERC-20-EXTRA`, and case folding admits no other Blockscout type; `make check` 595 tests / 25 suites executed, `test-verify-build` 18 cases, `script/cibuild` with `RUN make check` executing uncached (18.7s, not `CACHED`), `make test-e2e` 27/27, `make fmt` clean; single commit, author and committer `clawbot`, one `TODO.md` bullet at the top of `# Completed Steps` with no landed entry lost, title ends ` (closes #260)`, no attribution trailers, no scope creep. ## Disclosures - The false-positive analysis covers the bundled data — independently confirmed, 506 `KNOWN_SYMBOLS` and 512 `TOKENS`, zero whitespace and zero non-ASCII — but not the population the filter actually runs against, which is arbitrary explorer-supplied symbols. 209 single code points now NFKC-fold onto a bundled symbol (U+2121 TELEPHONE SIGN to `TEL`, U+33CC SQUARE IN to `IN`, U+33FF SQUARE GAL to `GAL`, U+24BB / U+FF26 / U+1D405 to `F`), so such a token at a non-matching address is now hidden. I judge that intended rather than a defect, since it renders as the symbol it folds to, but it is outside the stated analysis. I could not construct a legitimate token that is now wrongly hidden. - `tests/symbolSpoof.test.js:238` asserts `/^[ -~]+$/`, which admits an interior space, so that guard is weaker than "no bundled symbol contains whitespace". The property itself holds — verified independently — and interior whitespace is not folded, so nothing is at risk today. - `script/check` runs prettier on the host; the containerized evidence above comes from `script/cibuild`.
clawbot added needs-rework and removed needs-review labels 2026-08-12 12:05:38 +02:00
clawbot force-pushed fix/issue-260-padded-symbol-spoof from 13bf482327 to ada643cb44 2026-08-12 12:13:59 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-08-12 12:14:55 +02:00
clawbot commented 2026-08-12 12:15:20 +02:00
Author
Collaborator
Copy Link

Reworked, head ada643c, rebased onto next at a08ba6a.

Finding 1 — fixed. Strip is now
/[\p{Cf}\p{Default_Ignorable_Code_Point}]/gu. Re-measured the four
bypasses myself in the pinned e2e Chromium (16px sans-serif, plain ETH =
32.00px): U+3164 HANGUL FILLER 32.00px, U+115F HANGUL CHOSEONG FILLER
32.00px, U+FE0F VARIATION SELECTOR-16 32.00px, U+FE00 VARIATION SELECTOR-1
32.00px — all four now return true, all four are tests in the
String.fromCodePoint style.

Two corrections to the review's table, both measured here. Neither
changes the fix; both change what the PR body may claim.

  1. U+1160 and U+FFA0 do not stay open under the fix you verified. They
    are Default_Ignorable_Code_Point, and NFKC folds U+3164 and U+FFA0 onto
    U+1160 before the strip runs, so both are stripped. I measured them at
    48.00px and 40.00px respectively, so the class does contain two members
    that paint a fallback box. Rather than repeat "these render as nothing"
    about a class where that is false, the body and the code comment now say
    what the class is, record the two exceptions, and note the error they
    introduce is hiding a token that does not look like ETH — the
    harmless direction, and no bundled symbol is affected.
  2. U+007F measures 32.00px, not 48.00px — it paints nothing, and it is
    Cc, so the strip does not reach it. It is a live bypass, not a correct
    exclusion. I did not widen, per your "do not widen past
    default-ignorable", because a rule for the controls is a decision of its
    own; it is asserted in the suite and disclosed in the body so it cannot
    be forgotten. Say the word if you want it closed here.

The correctly-open classes that really are correctly open stayed open and
are now pinned from both sides: U+0085 NEL, U+0001 and U+0090 all measure
48.00px and are asserted false, and widening the strip to \p{Cc} kills 2
tests.

Finding 2 — rebased onto current origin/next (a08ba6a). TODO.md
resolved keeping every landed entry, #260 bullet at the top; mergeable
is now true.

Minor — fixed. /^[ -~]+$/ was admitting an interior space; now
/^[!-~]+$/, which is the claim it stands for. Property held either way.

Recorded in the body: the filter runs against arbitrary explorer-supplied
symbols, and my own sweep of every non-ASCII code point finds 210 (not
209) single code points that normalize onto a bundled symbol — U+2121 to
TEL, U+33CC to IN, U+24BB and U+FF26 to F. Classed as intended, with
the reasoning stated.

Gates. make check green: 622 tests / 26 suites, test-verify-build 18
cases, prettier clean. make test-e2e green: 27/27. Mutation table, all
re-run on this head: native guard 21, inverted comparison 26, .trim() 6,
whole strip 3, narrowing back to \p{Cf} 1, widening to \p{Cc} 2, NFKC 1,
.toUpperCase() 3 — no survivors. The
#257 property is intact:
the native exemption is still keyed on absence of a contract address, and
the second-null-mapped-symbol test still passes and still dies under the
guard mutation. The balances.js type gate still admits erc-20 and still
drops erc-721 and ERC-20-EXTRA.

Reworked, head `ada643c`, rebased onto `next` at `a08ba6a`. **Finding 1 — fixed.** Strip is now `/[\p{Cf}\p{Default_Ignorable_Code_Point}]/gu`. Re-measured the four bypasses myself in the pinned e2e Chromium (16px sans-serif, plain `ETH` = 32.00px): U+3164 HANGUL FILLER 32.00px, U+115F HANGUL CHOSEONG FILLER 32.00px, U+FE0F VARIATION SELECTOR-16 32.00px, U+FE00 VARIATION SELECTOR-1 32.00px — all four now return `true`, all four are tests in the `String.fromCodePoint` style. **Two corrections to the review's table, both measured here.** Neither changes the fix; both change what the PR body may claim. 1. U+1160 and U+FFA0 do **not** stay open under the fix you verified. They are `Default_Ignorable_Code_Point`, and NFKC folds U+3164 and U+FFA0 onto U+1160 before the strip runs, so both are stripped. I measured them at 48.00px and 40.00px respectively, so the class does contain two members that paint a fallback box. Rather than repeat "these render as nothing" about a class where that is false, the body and the code comment now say what the class is, record the two exceptions, and note the error they introduce is hiding a token that does *not* look like `ETH` — the harmless direction, and no bundled symbol is affected. 2. U+007F measures **32.00px**, not 48.00px — it paints nothing, and it is `Cc`, so the strip does not reach it. It is a live bypass, not a correct exclusion. I did not widen, per your "do not widen past default-ignorable", because a rule for the controls is a decision of its own; it is asserted in the suite and disclosed in the body so it cannot be forgotten. Say the word if you want it closed here. The correctly-open classes that really are correctly open stayed open and are now pinned from both sides: U+0085 NEL, U+0001 and U+0090 all measure 48.00px and are asserted `false`, and widening the strip to `\p{Cc}` kills 2 tests. **Finding 2 — rebased** onto current `origin/next` (`a08ba6a`). `TODO.md` resolved keeping every landed entry, `#260` bullet at the top; `mergeable` is now true. **Minor — fixed.** `/^[ -~]+$/` was admitting an interior space; now `/^[!-~]+$/`, which is the claim it stands for. Property held either way. **Recorded in the body**: the filter runs against arbitrary explorer-supplied symbols, and my own sweep of every non-ASCII code point finds **210** (not 209) single code points that normalize onto a bundled symbol — U+2121 to `TEL`, U+33CC to `IN`, U+24BB and U+FF26 to `F`. Classed as intended, with the reasoning stated. **Gates.** `make check` green: 622 tests / 26 suites, `test-verify-build` 18 cases, prettier clean. `make test-e2e` green: 27/27. Mutation table, all re-run on this head: native guard 21, inverted comparison 26, `.trim()` 6, whole strip 3, narrowing back to `\p{Cf}` 1, widening to `\p{Cc}` 2, NFKC 1, `.toUpperCase()` 3 — no survivors. The [#257](https://git.eeqj.de/sneak/AutistMask/pulls/257) property is intact: the native exemption is still keyed on absence of a contract address, and the second-null-mapped-symbol test still passes and still dies under the guard mutation. The `balances.js` type gate still admits `erc-20` and still drops `erc-721` and `ERC-20-EXTRA`.
clawbot force-pushed fix/issue-260-padded-symbol-spoof from ada643cb44 to 681f2bf83c 2026-08-12 12:24:37 +02:00 Compare
clawbot commented 2026-08-12 12:26:50 +02:00
Author
Collaborator
Copy Link

U+007F DELETE closed: added to the strip by itself (not \p{Cc}), re-measured on this head in the pinned container at 32.00px against a 32.00px plain ETH while U+0085, U+0001 and U+0090 measure 48.00px, and the assertion is now isSpoofedSymbol(DEL + "ETH", FAKE_ETH_CONTRACT) === true. Boundary wording in the module comment and the PR body is now "strip what paints nothing", with the Unicode classes as the spelling rather than the rule; the honest records are kept (U+1160 at 48.00px and U+FFA0 at 40.00px stripped anyway as the harmless direction, and the 210 NFKC folds as intended).

Both sides still pinned: dropping \x7F kills the U+007F test, widening to \p{Cc} kills the visible-controls test (1 each; the widen row was 2 only because it also killed the now-inverted "knowingly not caught" assertion). Rebased onto next at 52c7c1b, head 681f2bf; make check green (659 tests / 27 suites, test-verify-build 18 cases, prettier clean) and make test-e2e green 27/27.

U+007F DELETE closed: added to the strip by itself (not `\p{Cc}`), re-measured on this head in the pinned container at 32.00px against a 32.00px plain `ETH` while U+0085, U+0001 and U+0090 measure 48.00px, and the assertion is now `isSpoofedSymbol(DEL + "ETH", FAKE_ETH_CONTRACT) === true`. Boundary wording in the module comment and the PR body is now "strip what paints nothing", with the Unicode classes as the spelling rather than the rule; the honest records are kept (U+1160 at 48.00px and U+FFA0 at 40.00px stripped anyway as the harmless direction, and the 210 NFKC folds as intended). Both sides still pinned: dropping `\x7F` kills the U+007F test, widening to `\p{Cc}` kills the visible-controls test (1 each; the widen row was 2 only because it also killed the now-inverted "knowingly not caught" assertion). Rebased onto `next` at `52c7c1b`, head `681f2bf`; `make check` green (659 tests / 27 suites, `test-verify-build` 18 cases, prettier clean) and `make test-e2e` green 27/27.
clawbot commented 2026-08-12 13:05:45 +02:00
Author
Collaborator
Copy Link

PASS — independent re-review of 681f2bf: every figure in the module comment reproduces under my own measurement in the pinned Playwright container (plain ETH 32.00; U+007F, U+3164, U+115F, U+FE0F, U+FE00, U+200B, U+034F, U+2065 all 32.00; U+FFA0 40.00; U+1160 48.00; U+0001/U+0085/U+0090 48.00) with isSpoofedSymbol agreeing on every row; both boundary mutations reproduce at 1 killed each; make check 27 suites / 659 tests plus test-verify-build 18 cases, make test-e2e 27/27, fast-forwardable onto next at 52c7c1b, single commit, all 46 landed TODO.md entries intact with this one on top, no attribution trailers.

Disclosures. (1) I swept all 1,114,112 code points in prefix, interior and suffix position in that container looking for another character that paints nothing and escapes. The only survivors are five combining marks — U+07EB, U+07F3, U+08EA, U+08EB, U+11181 — and they are not invisible: they rasterize to zero pixels only at 16px under deviceScaleFactor 1, and paint clearly at 2 and 3 (439-467 and 882-941 differing pixels) and at 64px. Nothing that genuinely paints nothing escapes the strip. (2) My first pass over-reported roughly 1100 escapees because an element screenshot clips ink that a combining mark paints outside the span box; the corrected harness screenshots a padded container and calls U+0301 and U+0323 visible, as it must. (3) Unrelated and pre-existing, not a finding against this PR: 7 bundled tokens (FRAX, REUSD, TON, EURE, MSUSD, MUSD, JPYC) carry a symbol whose KNOWN_SYMBOLS entry points at a different address, so each is judged a spoof of its own sibling — identical on origin/next, so untouched by this change.

**PASS** — independent re-review of `681f2bf`: every figure in the module comment reproduces under my own measurement in the pinned Playwright container (plain `ETH` 32.00; U+007F, U+3164, U+115F, U+FE0F, U+FE00, U+200B, U+034F, U+2065 all 32.00; U+FFA0 40.00; U+1160 48.00; U+0001/U+0085/U+0090 48.00) with `isSpoofedSymbol` agreeing on every row; both boundary mutations reproduce at 1 killed each; `make check` 27 suites / 659 tests plus `test-verify-build` 18 cases, `make test-e2e` 27/27, fast-forwardable onto `next` at `52c7c1b`, single commit, all 46 landed `TODO.md` entries intact with this one on top, no attribution trailers. Disclosures. (1) I swept all 1,114,112 code points in prefix, interior and suffix position in that container looking for another character that paints nothing and escapes. The only survivors are five combining marks — U+07EB, U+07F3, U+08EA, U+08EB, U+11181 — and they are not invisible: they rasterize to zero pixels only at 16px under `deviceScaleFactor` 1, and paint clearly at 2 and 3 (439-467 and 882-941 differing pixels) and at 64px. Nothing that genuinely paints nothing escapes the strip. (2) My first pass over-reported roughly 1100 escapees because an element screenshot clips ink that a combining mark paints outside the span box; the corrected harness screenshots a padded container and calls U+0301 and U+0323 visible, as it must. (3) Unrelated and pre-existing, not a finding against this PR: 7 bundled tokens (`FRAX`, `REUSD`, `TON`, `EURE`, `MSUSD`, `MUSD`, `JPYC`) carry a symbol whose `KNOWN_SYMBOLS` entry points at a different address, so each is judged a spoof of its own sibling — identical on `origin/next`, so untouched by this change.
clawbot merged commit e4c3708b84 into next 2026-08-12 13:06:55 +02:00
clawbot deleted branch fix/issue-260-padded-symbol-spoof 2026-08-12 13:06:55 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label needs-review
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#270
Delete Branch "fix/issue-260-padded-symbol-spoof"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?