Skip to content

fix(web-ui): sanitize mermaid SVG output before HTML injection (light-tested) - #2611

Open
1688mengdie wants to merge 1 commit into
GCWing:mainfrom
BitFun-SIG:fix/security-mermaid-svg-sanitize
Open

fix(web-ui): sanitize mermaid SVG output before HTML injection (light-tested)#2611
1688mengdie wants to merge 1 commit into
GCWing:mainfrom
BitFun-SIG:fix/security-mermaid-svg-sanitize

Conversation

@1688mengdie

Copy link
Copy Markdown

Summary

Mermaid diagrams render with securityLevel: 'loose', so diagram labels
may carry raw HTML that survives rendering. MermaidBlock passed the
rendered SVG straight into dangerouslySetInnerHTML, letting a crafted
diagram execute scripts, abuse on* event handlers, or navigate via
javascript: URLs inside the renderer context.

This PR adds sanitizeMermaidSvg.ts, an allowlist-based DOM sanitizer
that keeps the structural SVG and text-container elements a rendered
diagram needs while stripping script/iframe/object-style elements,
on* handler attributes, and javascript:/vbscript:/data:text-html
URL schemes. The rendered output is sanitized before it reaches the DOM,
and the XSS vectors (script injection, on* handlers, unsafe URLs,
foreignObject payloads) plus normal-diagram rendering are covered by
tests.

Fixes #2602

Type and Areas

Type: security fix

Areas: Web UI

Verification

  • pnpm --dir src/web-ui run type-check — passed (0 errors).
  • pnpm --dir src/web-ui run lint — passed (0 errors).
  • vitest run MermaidBlock.test.tsx — 6 passed (new sanitizer test suite: XSS vectors + normal rendering).
  • vitest run Markdown.test.tsx MarkdownEditor.test.tsx — 16 passed (existing Markdown render regression suites, no behavior change).

Reviewer Notes

  • The sanitizer is a pure allowlist pass over the rendered SVG markup; diagrams that do not carry HTML labels render identically (covered by the regression suites above).
  • No new dependencies; the sanitizer is implemented against the existing DOM APIs already used by the web UI.
  • Rollback is a revert of the three-file change (new sanitizer module, one-line wiring in MermaidBlock.tsx, test file).

Checklist

  • This PR is focused and does not include secrets, temporary prompts, generated scratch files, or unrelated artifacts.
  • Relevant verification is recorded above, or skipped checks are explained.
  • User-facing strings, docs, and locales are updated where applicable. (Not applicable: no user-facing change beyond the sanitizer itself.)

This is an AI-assisted change.

Mermaid is configured with securityLevel: 'loose', so diagram labels may
carry raw HTML through rendering. The rendered SVG was passed straight
into dangerouslySetInnerHTML, letting a crafted diagram execute scripts,
abuse on* event handlers, or navigate via javascript: URLs.

Add sanitizeMermaidSvg.ts: an allowlist-based DOM sanitizer that keeps
the structural SVG and text-container elements a rendered diagram needs
while stripping script/iframe/object-style elements, on* handler
attributes, and javascript:/vbscript:/data:text-html URL schemes.
Wire it into MermaidBlock so only sanitized markup reaches the DOM, and
cover the XSS vectors (script, on* handlers, unsafe URLs,
foreignObject payloads) plus normal-diagram rendering in tests.

Test: pnpm --dir src/web-ui run type-check; pnpm --dir src/web-ui run lint;
vitest run MermaidBlock.test.tsx (6 passed); Markdown/MarkdownEditor
regression suites (16 passed).
AI: AI-assisted, locally tested (type-check + lint + targeted vitest).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security]: mermaid SVG output is injected into HTML without sanitization (XSS via label HTML in loose mode)

1 participant