A hostile ERC-20's symbol() reached an innerHTML string unescaped, and neither manifest declared default-src, so an attacker deploying a token with 1,000+ holders and airdropping one unit could render a full-viewport cross-origin iframe over the wallet's own UI, on screens where the user types their password. escapeHtml is now a pure string replace over & < > " ' — the old version round-tripped through textContent, which escapes neither quote, while already being used inside data-copy="...". All 19 files in src/popup/views/ were audited: beyond the reported symbol site, the explorer-supplied directionLabel in all three transaction lists, wallet.name, addr.ensName, the blockie data: URIs and two ad-hoc quote-only escapes were also unescaped. Explorer URLs now go through one helper that percent-encodes the path segment. Both manifests add default-src 'self', frame-src 'none', form-action 'none' and base-uri 'none'. Three loosenings are pinned in tests/manifest.test.js and justified in README.md: style-src 'unsafe-inline' (39 static style attributes; Firefox implements neither style-src-attr nor 'unsafe-hashes'), img-src data: (blockies), connect-src https: http: (user-configurable RPC). Note frame-src 'none' blocks a frame loading, not the element existing, so the zero-iframe assertion is a claim about the escaping alone; the test asserts the element count and the literal rendered text separately, taking the count before any click an overlay could intercept. Verified: make check 39 suites / 811 tests, test-e2e 55/55 including the WebAssembly-under-CSP assertion, test-e2e-firefox 8/8, zero CSP violations asserted rather than merely unobserved. Reverting only balanceLine's interpolation reproduces the attack as 2 iframes on the address screen.
44 lines
1.9 KiB
JavaScript
44 lines
1.9 KiB
JavaScript
// The length bound on a token symbol as displayed.
|
|
//
|
|
// A symbol is whatever an ERC-20's symbol() returns and the wallet fetches
|
|
// it from the block explorer, which imposes no length: src/shared/balances.js
|
|
// takes `item.token.symbol` as given. A kilobyte-long symbol is a real
|
|
// return value, and rendering it pushes every amount off the row, scrolls
|
|
// the balance list past the screen, and hides the figures the user is there
|
|
// to read.
|
|
//
|
|
// This is a layout bound, not a security control. Escaping is what makes a
|
|
// hostile symbol inert (see src/shared/html.js), and isSpoofedSymbol() is
|
|
// what catches one impersonating a known ticker; neither job belongs here
|
|
// and neither is done here. Truncating an unescaped symbol would still be
|
|
// an injection, just a shorter one.
|
|
//
|
|
// 12 characters, which is the bound lookupTokenInfo() in
|
|
// src/shared/balances.js already applies when it stores a symbol read
|
|
// straight off a contract; the explorer path was the one with no bound at
|
|
// all. The longest symbol across the 512 entries of the bundled list is 10
|
|
// (MSYRUPUSDP), so nothing the wallet ships as a real token is ever
|
|
// truncated. The ellipsis is what tells the user the name they are looking
|
|
// at is not the whole name — worth knowing before they send to it.
|
|
|
|
const MAX_SYMBOL_LENGTH = 12;
|
|
|
|
// The placeholder for a token whose symbol the explorer did not report.
|
|
// balances.js already substitutes this; repeated here so a symbol that
|
|
// arrives empty from anywhere else displays the same way rather than as a
|
|
// blank gap in the row.
|
|
const UNKNOWN_SYMBOL = "???";
|
|
|
|
function displaySymbol(symbol) {
|
|
const s = symbol === null || symbol === undefined ? "" : String(symbol);
|
|
if (s.length === 0) return UNKNOWN_SYMBOL;
|
|
if (s.length <= MAX_SYMBOL_LENGTH) return s;
|
|
return s.slice(0, MAX_SYMBOL_LENGTH - 1) + "…";
|
|
}
|
|
|
|
module.exports = {
|
|
displaySymbol,
|
|
MAX_SYMBOL_LENGTH,
|
|
UNKNOWN_SYMBOL,
|
|
};
|