From c8c2af0c6bb9422376887a564db85a3868826e9f Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 20 Aug 2026 13:47:28 +0200 Subject: [PATCH] harden: escape every interpolation into popup innerHTML, and add default-src to both manifests (closes #307) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- README.md | 47 ++++++-- TODO.md | 22 ++++ manifest/chrome.json | 2 +- manifest/firefox.json | 2 +- src/popup/views/addToken.js | 4 +- src/popup/views/addressDetail.js | 9 +- src/popup/views/addressToken.js | 21 ++-- src/popup/views/confirmTx.js | 20 +++- src/popup/views/deleteAddress.js | 3 +- src/popup/views/helpers.js | 54 ++++++++-- src/popup/views/home.js | 19 ++-- src/popup/views/receive.js | 3 +- src/popup/views/send.js | 3 +- src/popup/views/settings.js | 13 ++- src/popup/views/settingsAddToken.js | 22 ++-- src/popup/views/transactionDetail.js | 19 ++-- src/popup/views/txStatus.js | 23 ++-- src/shared/html.js | 41 +++++++ src/shared/symbolDisplay.js | 43 ++++++++ tests/balanceLineEscaping.test.js | 69 ++++++++++++ tests/e2e/network.js | 23 ++-- tests/e2e/run.js | 153 +++++++++++++++++++++++++++ tests/htmlEscape.test.js | 110 +++++++++++++++++++ tests/manifest.test.js | 78 +++++++++++--- 24 files changed, 706 insertions(+), 97 deletions(-) create mode 100644 src/shared/html.js create mode 100644 src/shared/symbolDisplay.js create mode 100644 tests/balanceLineEscaping.test.js create mode 100644 tests/htmlEscape.test.js diff --git a/README.md b/README.md index 24782a5..5d41a96 100644 --- a/README.md +++ b/README.md @@ -1464,10 +1464,44 @@ policy, but as of now there are none. ### Content Security Policy -Both manifests declare the same policy for extension pages — -`script-src 'self' 'wasm-unsafe-eval'; object-src 'self'` — as an object under +Both manifests declare the same policy for extension pages, as an object under `content_security_policy.extension_pages` in `manifest/chrome.json` (MV3) and as -a bare string in `manifest/firefox.json` (MV2). +a bare string in `manifest/firefox.json` (MV2): + +``` +default-src 'self'; script-src 'self' 'wasm-unsafe-eval'; object-src 'self'; +style-src 'self' 'unsafe-inline'; img-src 'self' data:; +connect-src 'self' https: http:; frame-src 'none'; form-action 'none'; +base-uri 'none' +``` + +`default-src 'self'` is the floor. Without it the policy governed script and +plugins only, and everything else — frames above all — was unrestricted, which +is what let an unescaped token symbol paint a cross-origin iframe over the +wallet's own UI. Escaping is the primary fix for that (see +`src/shared/html.js`); this is the second line, so an escape that does slip +cannot reach the network. + +Four directives are looser than `'self'`, each for a reason that does not +generalise: + +- `style-src 'unsafe-inline'` — `src/popup/index.html` and the view helpers set + presentation through `style="..."` attributes, which CSP blocks without this. + Chrome enforces `style-src` on attributes, not only on `