Skip to content

Remote announce Markdown is rendered as HTML without sanitization (XSS) #12

Open

Description

@lgnap

Contexte

App.js renders the remote announce (marian-m12l/studio/master/ANNOUNCE.md, fetched through EvergreenService) with dangerouslySetInnerHTML={{__html: marked(this.state.announce)}}. marked@^1.0.0 is EOL with known XSS issues (< 2.0) and its output is not sanitized, so anyone able to change that upstream file (or MITM the raw.githubusercontent.com fetch) can execute script in every STUdio user's browser. HTML from the i18n bundles is also injected unsanitized in a few places (lower risk, same fix).

Description

Sanitize every dangerouslySetInnerHTML sink. Upgrade marked to a maintained major and pass its output through DOMPurify.sanitize(...). Apply the same sanitizer to the t(...) HTML sinks so all raw HTML goes through one helper.

Fichiers concernes

  • web-ui/javascript/package.json:23 — bump marked (>= 4, last version still building under react-scripts 3 / Node 12), add dompurify
  • web-ui/javascript/src/utils/html.js (new) — renderMarkdown(md) and sanitizeHtml(html) helpers wrapping DOMPurify
  • web-ui/javascript/src/App.js:12, 204 — use renderMarkdown
  • web-ui/javascript/src/components/PackLibrary.js:415, 425 — use sanitizeHtml(t(...))
  • web-ui/javascript/src/components/diagram/widgets/PackDiagramWidget.js:374-375 — use sanitizeHtml(t(...))
  • web-ui/javascript/src/utils/html.test.js (new) — <script>, onerror= and javascript: payloads are stripped, benign Markdown (links, bold, lists) survives

Plan d'implementation

  1. Add dompurify, bump marked; adapt the import (import { marked } from 'marked' on >= 4).
  2. Create utils/html.js with the two helpers.
  3. Replace the four sinks.
  4. Add unit tests for the helper with XSS payloads and a benign Markdown sample.
  5. yarn test and yarn build green; yarn.lock updated and committed.

Criteres d'acceptation

  • No dangerouslySetInnerHTML receives unsanitized HTML
  • marked >= 4 and dompurify present in package.json and yarn.lock
  • Tests prove script/event-handler/javascript: payloads are removed and normal Markdown still renders
  • yarn test and the Maven front build green on Ubuntu and Windows CI

Complexite

M

Activity

  1. lgnap commented on Sep 11, 2026

    @lgnap
    OwnerAuthor

    Prise en charge par Claude Code (instance tuxedo:40112:d397e957, branche fix/issue-12-frontend-markdown-xss).

  2. lgnap commented on Sep 11, 2026

    @lgnap
    OwnerAuthor

    Implemented in antoinevalentinHA#58 (branch fix/issue-12-frontend-markdown-xss).

    • src/utils/html.js: renderMarkdown() / sanitizeHtml() on DOMPurify, form controls forbidden on top; all five dangerouslySetInnerHTML sinks go through it
    • marked 1.x → 4.3.0 (last line supporting the Node 12 CI), dompurify 3.4.15 added; lockfile diff limited to those entries
    • Wart, documented in code: marked imported by its UMD path because react-scripts 3's jest cannot load the .cjs main and CRA 3.0.1 forbids the config override
    • html.test.js (9 tests): payloads removed, translation markup preserved byte for byte
    • CI green on Ubuntu, Windows and Web UI

    Awaiting review/merge by the upstream owner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions