fix: balances.js fabricates decimals 18 before storage, laundering a guess into both approval paths #349
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sneak/AutistMask#349
Reference in New Issue
Block a user
Found by the review of #345 (#345 (comment)). Pre-existing, outside that diff.
src/shared/balances.js:75doesparseInt(item.token.decimals || "18", 10)before writing tostate.wallets[].addresses[].tokenBalances[].decimals.An explorer that reports no decimals — a token whose
decimals()reverts — therefore yields a fabricated 18 that is indistinguishable from a real 18 at read time. Every later reader trusts it as resolved.This defeats the guarantee of an issue already closed as fixed. #306 established "resolve the real scale or refuse to format" for the ERC-20 approval line, and #340 extended it to the swap line — but both read this stored value as an authoritative source, so a fabricated 18 flows straight through and the amount formats at a guessed scale with no refusal. The refusal path is intact; it simply never fires, because the guess was laundered upstream of it.
This is the real remaining hole in the resolve-or-refuse rule.
Open design question, recorded here rather than filed separately
The same review probed the trust chain and confirmed that an explorer-reported scale does size the number the user authorizes: a reported
36renders a 1,000-unit swap as0.000000000000000000000000001, a reported77as1e-68. Type hostility is correctly rejected (256,-1,"abc",null,[],"1e6",6.5all refuse), disagreeing values across addresses refuse, and the bundled list can never be overridden — so this is bounded, and it is the deliberate design already merged for #306, not something new.Still worth deciding explicitly: should an explorer-sourced scale be trusted to size an approval at all, or should a token that is neither bundled nor tracked be treated as unknown scale? Fixing the fabrication below does not answer that. Recommendation: fix the fabrication now, and treat tightening the trust model as a separate decision, since it would change behaviour on the already-merged ERC-20 path too.
Definition of done
decimalsis stored as unknown, not as 18. A real 18 and an absent value must be distinguishable at read time.|| "18"/|| 18/?? 18fallback survives on any path that feeds a displayed amount. Note the recurring falsy-collapse trap of #246: a real scale of0must not be treated as absent.decimalsfor a token reaches a refusal on both the ERC-20 and swap approval lines. Fails against current head — state the mutation and observed result.make checkgreen.Settling the open design question recorded in the issue body, so it does not outlive the fix: an explorer-sourced scale stays trusted. No further work; the resolve-or-refuse rule is complete once the fabrication is gone.
Reasoning: the wallet already trusts the explorer for the balances themselves. An explorer that lies about a token's scale can equally lie about the quantity, so sourcing
decimalsfrom it adds no trust assumption that is not already load-bearing — and the endpoint is user-configurable, which puts it in the same position as the RPC. The residual is a plausible-but-wrong value from a compromised explorer, which is bounded by the checks already in place: hostile types (256,-1,"abc",null,[],"1e6",6.5) refuse, values disagreeing across addresses refuse, and the bundled list can never be overridden.The alternative — treating any token that is neither bundled nor tracked as unknown scale — would refuse to state an amount for most real tokens, which trades a bounded and already-accepted trust dependency for a large loss of usefulness on the screen that most needs to be informative.
What this fix removes is different in kind and is the part that mattered: a scale nobody reported being invented and stored as though it had been.