clawbot
c8c2af0c6b
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.
42 lines
1.8 KiB
JavaScript
42 lines
1.8 KiB
JavaScript
// HTML escaping for values interpolated into an innerHTML string.
|
|
//
|
|
// Every view in src/popup/views/ builds markup by string concatenation, so
|
|
// this is the only thing standing between a value the wallet did not author
|
|
// and the extension's own DOM. The values that reach it are attacker
|
|
// controlled by design: an ERC-20's symbol() and name() are whatever the
|
|
// contract chooses to return, an ENS name is whatever the resolver returns,
|
|
// and both arrive through the block explorer with no schema.
|
|
//
|
|
// It escapes both quote characters as well as the tag delimiters, because
|
|
// the popup interpolates into attribute values as well as into element
|
|
// text — copyableHtml() writes data-copy="..." and etherscanLinkHtml()
|
|
// writes href="...". A `<`/`>`-only escape leaves an unquoted-attribute
|
|
// break-out intact, and the round trip through a detached element's
|
|
// textContent that used to implement this was exactly that escape: the
|
|
// HTML serializer only escapes `&`, `<`, `>` and U+00A0 in a text node,
|
|
// since a text node has no idea it is about to be pasted inside quotes.
|
|
//
|
|
// Deliberately a pure string function with no DOM dependency: it is called
|
|
// on every rendered row, it is unit-testable without a document, and it
|
|
// cannot be affected by the state of a document that an attacker-supplied
|
|
// string has already been written into.
|
|
|
|
const HTML_ESCAPES = {
|
|
"&": "&",
|
|
"<": "<",
|
|
">": ">",
|
|
'"': """,
|
|
"'": "'",
|
|
};
|
|
|
|
// `&` is escaped first by virtue of being in the same pass: a sequential
|
|
// replace would re-escape the ampersands it had just introduced.
|
|
function escapeHtml(s) {
|
|
if (s === null || s === undefined) return "";
|
|
return String(s).replace(/[&<>"']/g, (c) => HTML_ESCAPES[c]);
|
|
}
|
|
|
|
module.exports = {
|
|
escapeHtml,
|
|
};
|