From eec3e23099aae0f19892fc13cbe17acf72323ded Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 5 Oct 2026 10:43:06 +0200 Subject: [PATCH] chore: escape every value the views write as markup, and cut symbols on code points (closes #329) The token screen's decimals and holder count, the ETH price, every address total and each balance row's USD value went into innerHTML unescaped, against the rule at the top of src/popup/views/helpers.js. They are escaped now. None could carry markup, but formatUsd() writes a value under a cent as "< $0.01". displaySymbol() counts a symbol in code points, not UTF-16 units, so the cut never leaves half of an emoji, which rendered as U+FFFD. explorerLink() was already removed on next. Model: opus-5-5 --- TODO.md | 10 ++++++++++ src/popup/views/addressDetail.js | 2 +- src/popup/views/addressToken.js | 6 +++--- src/popup/views/helpers.js | 2 +- src/popup/views/home.js | 7 ++++--- src/shared/symbolDisplay.js | 9 +++++++-- tests/addressValue.test.js | 8 ++++++++ tests/balanceLineEscaping.test.js | 7 +++++++ tests/htmlEscape.test.js | 11 +++++++++++ 9 files changed, 52 insertions(+), 10 deletions(-) diff --git a/TODO.md b/TODO.md index 1c0eae4..4a5d041 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,16 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-05: Escaping in the popup's views follows its own rule with no + exceptions ([#329](https://git.eeqj.de/sneak/AutistMask/issues/329)). The + decimals and holder count on a token's screen, and every USD figure (the ETH + price, each total and each balance row's value), went into `innerHTML` + unescaped; they are escaped now. None could carry markup, but `formatUsd()` + writes a value under a cent as `< $0.01`. `displaySymbol()` counts a symbol in + code points rather than UTF-16 units, so a cut never splits an emoji into a + half that renders as U+FFFD. `explorerLink()`, also named in the issue, was + already removed by [#168](https://git.eeqj.de/sneak/AutistMask/issues/168). + - 2026-10-05: A Chrome end-to-end test that fails no longer takes later tests down with it ([#318](https://git.eeqj.de/sneak/AutistMask/issues/318)). Each test that turns a fixture switch on for itself alone (a held or failing gas diff --git a/src/popup/views/addressDetail.js b/src/popup/views/addressDetail.js index 39f1f6e..feeadde 100644 --- a/src/popup/views/addressDetail.js +++ b/src/popup/views/addressDetail.js @@ -66,7 +66,7 @@ function show() { $("address-line").dataset.full = addr.address; attachCopyHandlers($("address-line")); const usdTotal = formatAddressTotal(getAddressValue(addr)); - $("address-usd-total").innerHTML = usdTotal || " "; + $("address-usd-total").innerHTML = escapeHtml(usdTotal) || " "; const ensEl = $("address-ens"); // ENS is now shown inside renderAddressHtml, hide the separate element ensEl.classList.add("hidden"); diff --git a/src/popup/views/addressToken.js b/src/popup/views/addressToken.js index 451e58b..e834e7f 100644 --- a/src/popup/views/addressToken.js +++ b/src/popup/views/addressToken.js @@ -103,7 +103,7 @@ function show() { // USD total for this token only const usdVal = price && amount !== null ? amount * price : null; const usdStr = formatUsd(usdVal); - $("address-token-usd-total").innerHTML = usdStr || " "; + $("address-token-usd-total").innerHTML = escapeHtml(usdStr) || " "; // Single token balance line (no tokenId — not clickable here) $("address-token-balance").innerHTML = balanceLine(symbol, amount, price); @@ -148,9 +148,9 @@ function show() { if (tokenSymbol) infoHtml += `
Symbol: ${tokenSymbol}
`; if (tokenDecimals != null) - infoHtml += `
Decimals: ${tokenDecimals}
`; + infoHtml += `
Decimals: ${escapeHtml(tokenDecimals)}
`; if (tokenHolders != null) - infoHtml += `
Holders: ${Number(tokenHolders).toLocaleString()}
`; + infoHtml += `
Holders: ${escapeHtml(Number(tokenHolders).toLocaleString())}
`; if (projectUrl) infoHtml += `
Website: ${escapeHtml(projectUrl)}
`; contractInfo.innerHTML = infoHtml; diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index bf44d49..e01272a 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -325,7 +325,7 @@ function balanceLine(symbol, amount, price, tokenId) { const qty = amount === null ? "quantity unknown" : amount.toFixed(4); const usd = price && amount !== null - ? formatUsd(amount * price) || " " + ? escapeHtml(formatUsd(amount * price)) || " " : " "; // tokenId is a contract address out of the same explorer JSON, and it // lands inside a quoted attribute. diff --git a/src/popup/views/home.js b/src/popup/views/home.js index 192a5fc..37046f9 100644 --- a/src/popup/views/home.js +++ b/src/popup/views/home.js @@ -63,7 +63,7 @@ function renderTotalValue() { const ethPrice = getPrice("ETH"); if (priceEl) { priceEl.innerHTML = ethPrice - ? formatUsd(ethPrice) + " USD/ETH" + ? escapeHtml(formatUsd(ethPrice) + " USD/ETH") : " "; } @@ -79,7 +79,8 @@ function renderTotalValue() { el.textContent = ethStr + ethUsd; if (subEl) { - subEl.innerHTML = formatAddressTotal(getAddressValue(addr)) || " "; + subEl.innerHTML = + escapeHtml(formatAddressTotal(getAddressValue(addr))) || " "; } } @@ -280,7 +281,7 @@ function walletListHtml() { } html += `
${escapeHtml(addr.address)}
`; const addrTotal = formatAddressTotal(getAddressValue(addr)); - html += `
${addrTotal || " "}
`; + html += `
${escapeHtml(addrTotal) || " "}
`; html += balanceLinesForAddress( addr, state.trackedTokens, diff --git a/src/shared/symbolDisplay.js b/src/shared/symbolDisplay.js index e2f7ba6..bf412da 100644 --- a/src/shared/symbolDisplay.js +++ b/src/shared/symbolDisplay.js @@ -20,6 +20,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. +// +// Characters are counted as code points, not UTF-16 units, so an emoji is +// one character and the cut never falls between the two halves of one: a +// half on its own renders as U+FFFD. const MAX_SYMBOL_LENGTH = 12; @@ -32,8 +36,9 @@ 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) + "…"; + const chars = Array.from(s); + if (chars.length <= MAX_SYMBOL_LENGTH) return s; + return chars.slice(0, MAX_SYMBOL_LENGTH - 1).join("") + "…"; } module.exports = { diff --git a/tests/addressValue.test.js b/tests/addressValue.test.js index 3d6851d..bd0a462 100644 --- a/tests/addressValue.test.js +++ b/tests/addressValue.test.js @@ -194,6 +194,14 @@ describe("the wallet list on Home", () => { clearPrices(); expect(walletListTotal(FULLY_PRICED)).toBe(" "); }); + + // A total under a cent is written "< $0.01", and the "<" is escaped + // here as the removal warning escapes it. + test("a total under a cent is escaped, as on the removal warning", () => { + const tiny = { ...EMPTY, balance: "0.000001" }; + expect(walletListTotal(tiny)).toBe("Total: < $0.01"); + expect(removalWarningTotal(tiny)).toBe("Total: < $0.01"); + }); }); describe("the balance warning on the address-removal confirmation", () => { diff --git a/tests/balanceLineEscaping.test.js b/tests/balanceLineEscaping.test.js index 6acfa69..e1bacf1 100644 --- a/tests/balanceLineEscaping.test.js +++ b/tests/balanceLineEscaping.test.js @@ -66,4 +66,11 @@ describe("balanceLine", () => { expect(html).toContain("1.5000"); expect(html).toContain('data-token="0xabc"'); }); + + // formatUsd() writes a value under a cent as "< $0.01". + test("escapes the USD value along with the symbol", () => { + const html = balanceLine("USDC", 0.001, 1, null); + expect(html).toContain("< $0.01"); + expect(html).not.toContain("< $0.01"); + }); }); diff --git a/tests/htmlEscape.test.js b/tests/htmlEscape.test.js index 69fe008..df0cf46 100644 --- a/tests/htmlEscape.test.js +++ b/tests/htmlEscape.test.js @@ -91,6 +91,17 @@ describe("displaySymbol", () => { expect(displaySymbol(exact)).toBe(exact); }); + // An emoji outside the Basic Multilingual Plane is two UTF-16 units. + // Cutting between them leaves half of one, which renders as U+FFFD. + test("counts an emoji as one character and never cuts one in half", () => { + expect(displaySymbol("🚀".repeat(MAX_SYMBOL_LENGTH))).toBe( + "🚀".repeat(MAX_SYMBOL_LENGTH), + ); + expect(displaySymbol("🚀".repeat(20))).toBe( + "🚀".repeat(MAX_SYMBOL_LENGTH - 1) + "…", + ); + }); + test("substitutes a placeholder for an absent symbol", () => { expect(displaySymbol("")).toBe(UNKNOWN_SYMBOL); expect(displaySymbol(null)).toBe(UNKNOWN_SYMBOL);