fix(ui): escape attacker-controlled values in transaction titles#2236
Conversation
Transaction summaries render via v-html so the intentional <b> markup shows. The token symbol, contract method name, and resolved recipient name were interpolated unescaped. A token deployed with an XSS payload as its symbol(), a crafted ABI method name, or an ENS reverse record could execute script when a proposal's execution transactions render. Add an escapeHtml helper and escape those dynamic values, keeping the static formatting markup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the UI against stored XSS by ensuring attacker-controlled values interpolated into transaction title HTML (rendered via v-html) are HTML-escaped while preserving intended static <b> formatting.
Changes:
- Added a shared
escapeHtmlhelper inapps/ui/src/helpers/utils.ts. - Escaped token symbols, contract method names, and ENS-resolved recipient names in
TransactionsListItem.vuebefore rendering viav-html. - Added unit tests for
escapeHtmlinutils.test.ts.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/ui/src/helpers/utils.ts | Introduces escapeHtml helper for safely escaping dynamic strings used in HTML contexts. |
| apps/ui/src/helpers/utils.test.ts | Adds unit tests validating escapeHtml behavior against XSS-like payloads and special characters. |
| apps/ui/src/components/TransactionsListItem.vue | Applies escapeHtml to attacker-controlled fields before composing v-html transaction titles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chai3-bot
left a comment
chai3-bot
left a comment
There was a problem hiding this comment.
Reviewed the XSS fix and the surrounding transaction-title rendering path. No blocking findings.
The token symbol, ABI-derived method name, and resolved recipient name are now escaped before reaching v-html, while the intentional static <b> markup is preserved. The fallback recipient path is escaped as well, and the corresponding expanded transaction details continue to use Vue text interpolation. The helper covers all HTML-sensitive characters and nullish input.
CI is green (lint-build-test and E2E test), and the patch passes git diff --check.
Widen escapeHtml to accept nullish input so callers/tests don't cast, and replace the full <b>_NAME_</b> marker to avoid collisions when an escaped value contains the literal _NAME_ token. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
String.prototype.replace treats $ sequences in a string replacement as special patterns even with a plain-string search. escapeHtml turns & into &, so a recipient name containing $& or $$ would splice the matched <b>_NAME_</b> marker into the output or silently drop characters. Use a function replacer so the escaped value is inserted literally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the <b>_NAME_</b> substitution into a testable replaceNamePlaceholder helper and add regression cases for names containing $&, $$, $` and $'. These fail against a plain string replacement (the marker gets spliced in or characters are dropped) and pass with the function replacer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to #2233. Chaitu asked us to sweep the rest of the frontend for the same class of XSS after the Markdown language-label fix. I audited every
innerHTML/outerHTML/insertAdjacentHTML/document.write/v-html/dangerouslySetInnerHTMLsite in apps/ui.One real issue:
TransactionsListItem.vuebuilds a transaction summary string with intentional<b>markup and renders it viav-html. Three of the interpolated values are attacker-controlled and were going in unescaped:token.symbol- an ERC20's on-chainsymbol(); anyone can deploy a token whose symbol is an<img onerror=...>payloadmethodname - comes from an ABI the proposal author suppliesname- an ENS reverse record the recipient controlsAny of these renders when a viewer opens a proposal that carries such an execution transaction, so it's stored XSS reachable by a proposal author. Fix escapes just the dynamic pieces with a small
escapeHtmlhelper and keeps the static formatting markup.Everything else came back clean (see audit table below), so no other code changes.
Audit
html:false, htmltag/htmlblock rules disabledsanitizeHtml:truesanitizeHtml:truesanitizeHtml:true<b>${name}</b>.innerHTMLTest plan
escapeHtmlunit tests inutils.test.tscovering the<img onerror>payload, all HTML-sensitive chars, plain text, and nullish input.vitest run src/helpers/utils.test.ts-> 36 passed../gqlerrors unrelated).