Skip to content

fix(web): sanitize markdown link attributes to prevent XSS (CWE-79)#17362

Open
kvenux wants to merge 1 commit intoanomalyco:devfrom
kvenux:fix/cwe-79-web-share-markdown-xss
Open

fix(web): sanitize markdown link attributes to prevent XSS (CWE-79)#17362
kvenux wants to merge 1 commit intoanomalyco:devfrom
kvenux:fix/cwe-79-web-share-markdown-xss

Conversation

@kvenux
Copy link
Copy Markdown

@kvenux kvenux commented Mar 13, 2026

Issue for this PR

Closes #17361

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

The custom marked link renderer in content-markdown.tsx interpolates href and title directly into HTML without escaping. Combined with innerHTML rendering, this allows XSS via crafted markdown links.

Fixes:

  • Added escapeHtml() for href and title attributes to prevent attribute breakout

  • Added URL protocol whitelist (https:, http:, mailto:, tel:) to block javascript: URLs

  • Unsafe URLs render as <span> instead of <a>

  • CWE: CWE-79

  • File: packages/web/src/components/share/content-markdown.tsx:12-14

  • Severity: High

How did you verify your code works?

  • Added content-markdown.test.ts with 9 test cases
  • Covers escapeHtml, URL protocol whitelist, attribute injection, normal links
  • All 9 tests PASS

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Found a related PR:

PR #17357 - fix(desktop-electron): sanitize markdown link attributes to prevent XSS (CWE-79)
#17357

This PR addresses the same XSS vulnerability (CWE-79) in markdown link rendering, but for the desktop-electron package instead of the web package. Both PRs implement the same security fixes: escaping HTML attributes, URL protocol whitelisting, and blocking unsafe URLs. They're part of the same security fix across multiple packages.

@kvenux
Copy link
Copy Markdown
Author

kvenux commented Mar 13, 2026

Security Audit: Severity Downgrade to Defense-in-Depth

After thorough testing and analysis, we're downgrading this from High to Low (defense-in-depth).

What we verified

1. Client-side vulnerability is real (code level)

marked.parse() passes raw HTML through unchanged — no sanitization:

$ bun -e 'import { marked } from "marked"; console.log(await marked.parse("<img src=x onerror=alert(1)>"))'
<img src=x onerror=alert(1)>

Combined with innerHTML={html()} in content-markdown.tsx:63 (no DOMPurify), this is a valid XSS sink.

2. Local POC confirms XSS fires in browser

Created poc-xss-share.html that replicates the exact rendering path (marked.parse()innerHTML):

  • <img src=x onerror=...>XSS fired (auto-execute on page load)
  • <svg onload=...>XSS fired (auto-execute on page load)
  • [click](javascript:...) → Clickable link rendered

3. Live opncd.ai share page — XSS does NOT fire

Tested by sharing a session containing <img src=x onerror=alert(document.cookie)> in an AI assistant response:

  • The share page rendered <img src="x"> as a real HTML element (broken image icon)
  • But the onerror attribute was stripped before reaching the client
  • The opncd.ai backend API sanitizes HTML event handler attributes during data transmission/storage

Why this is defense-in-depth, not critical

There are three defense layers blocking exploitation:

Layer Status Blocks XSS?
AI model safety training Active Yes — models refuse/strip XSS payloads
opncd.ai backend sanitization Active Yes — strips event handler attributes
Client-side DOMPurify Missing N/A — this is what the PR adds

The first two layers already prevent exploitation on the production service. The client-side code should have DOMPurify (it's the right practice, and the main app already uses it), but the actual risk on opncd.ai is low.

Potential risk for self-hosted deployments

Self-hosted or enterprise deployments that use a different backend may lack the server-side sanitization layer. In that case, the client-side vulnerability would be directly exploitable.

Recommendation

  • This PR's link renderer fix is correct and worth merging
  • However, the fix is incomplete — it only addresses markdown links (javascript: URIs, attribute injection), not raw HTML injection (<img onerror>, <svg onload>)
  • The proper fix should also add DOMPurify before innerHTML, matching the main app's approach in packages/ui/src/components/markdown.tsx:267
  • Severity label should be updated from High to Low (defense-in-depth)

Test artifacts

  • poc-xss-share.html — standalone browser POC
  • SECURITY_POC.md — full audit report with all findings

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.

fix(web): Share markdown link renderer XSS via unescaped attributes (CWE-79)

2 participants