daybook/issues/014-stored-xss-in-rendered-text-artifacts.md
peter e3c5773e8f Fix #014: sanitize themed text kinds + script-blocking CSP (stored XSS)
markdown/deck/html_fragment rendered author-influenced content verbatim,
so <script>/<img onerror>/etc. executed on the Daybook origin — self-XSS
against the owner's session, and (post-sharing) unauthenticated via /s/.

Two layers, both in artifacts/views.py:
- Create-time allowlist sanitization via nh3 (maintained ammonia/Rust
  sanitizer, not deprecated bleach). Keeps formatting/tables/links/images
  (incl. relative /assets URLs) and codehilite markup; drops script, on*
  handlers, style, unknown tags, and javascript:/data: URLs. html_full is
  deliberately NOT sanitized (documented verbatim escape hatch).
- Serve-time CSP (script-src 'none') for the sanitized kinds, on both
  /a/{id} and the unauthenticated /s/{token} route. Being a response
  header, it also neutralizes scripts in artifacts stored before this fix.
  Not applied to html_full or json/csv (json/csv data is already escaped;
  their viewers need their own JS).

Verified live across all kinds; 7 regression tests added. 102 tests pass.

Not addressed (noted in the issue): CSV formula injection,
CSRF_COOKIE_HTTPONLY (moot with XSS closed), nonce CSP for json/csv/full.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-09-03 10:44:49 +01:00

9.8 KiB
Raw Permalink Blame History

014 — Stored XSS: markdown (and deck/html_fragment) render unsanitized script; no CSP; now reachable unauthenticated via public share links

Status: Fixed Severity: High — stored XSS executing on the Daybook origin, with a concrete escalation chain against the owner's session, and (since the sharing feature shipped in 91ea19d) reachable by unauthenticated third parties.

Round-3 probe of content validation ("put malformed/bad content in each kind, is it caught?"). Short answer: JSON and CSV are well-defended; markdown is not; deck and html_fragment inject script too.

What was tested and what happened

All posted to the test account and fetched back rendered from /a/{id}.

Kind Injected Result
markdown <script>…</script>, <img src=x onerror=…>, <a href="javascript:…"> All pass through verbatim into rendered HTML — executes.
deck <section>…<script>alert(1)</script> Script passes through into the themed deck page — executes.
html_fragment <p>hi</p><script>…</script><svg onload=…> Script/svg onload pass through into the themed page — executes. (The #004 content-shape check blocks <html>/<body>/<!doctype> but never <script>.)
html_full broken/garbage doc with <script> Stored verbatim. By design — documented as "you own all presentation." Not claimed as a bug here.
json {"note":"</SCRIPT><img src=x onerror=…>"} Defended. escape_script_body (re.compile(r"</script", re.IGNORECASE)) turns it into <\/script inside the <script type="application/json"> data block; renderjson shows it as inert text. Case-variation bypass (</SCRIPT>) correctly caught.
csv </script> in a cell Defended. Same escaping via json.dumps + escape_script_body.
csv `=cmd /c calc, +SUM(...)` as cell values

Root cause (markdown specifically)

create_text_artifact renders markdown with:

markdown.markdown(content, extensions=["extra", "sane_lists", "codehilite"], ...)

Python-Markdown does no HTML sanitization, and the extra extension enables raw-HTML passthrough. Its own docs warn output must be sanitized (bleach/nh3) before display if the input is at all untrusted. The rendered HTML is then written to the artifact's .html file and served as-is.

Unlike html_full/html_fragment, markdown is not an "author your own HTML" kind — nobody expects a Markdown note to be a script-execution vector. In an agent-driven system this is exactly the kind most likely to carry indirectly untrusted content: an agent that summarizes a scraped page, an email, or an ingested document can emit a summary containing an attacker-controlled <script>/<img onerror> that ends up verbatim in the posted markdown.

Why this is High, not self-XSS

Two things raise the stakes well past "you attacked your own account":

  1. No Content-Security-Policy header. Rendered artifact responses carry only X-Frame-Options: SAMEORIGIN and X-Content-Type-Options: nosniff — nothing stops inline script execution. (A CSP was probably avoided because the json/csv/deck viewers themselves use inline scripts; a nonce-based CSP that tags Daybook's own inline scripts but not user content would fix both this and the by-design kinds at once.)

  2. Escalation chain against the owner's own session. When the owner views the malicious artifact at /a/{id} (their normal feed preview), the script runs same-origin with the owner's session cookie. Reads are session-authed, so it can GET /api/artifacts and exfiltrate every private artifact's metadata. Worse, the csrftoken cookie is not HttpOnly, so injected JS can read it and forge session+CSRF POSTs — e.g. hit POST /a/{id}/share for every artifact, minting public share links for the owner's entire private feed and exfiltrating them. (Bearer-token writes like PATCH/DELETE are safe — the token isn't in the browser — but share-toggle, mark-read, and keep are session+CSRF and thus reachable.)

  3. Unauthenticated blast radius via sharing (91ea19d). A shared artifact at /s/{token} serves this same rendered HTML with no auth, so a malicious/compromised artifact runs arbitrary JS in the browser of anyone the owner sends the link to — a drive-by/phishing vector on the Daybook origin, not just a self-inflicted one.

Suggested Fix

  • Sanitize rendered output for the non-authoring kinds — at minimum markdown, and I'd argue deck too (a "slide sections" kind isn't obviously an arbitrary-script host). Run the generated HTML through an allowlist sanitizer that keeps normal formatting (headings, lists, tables, code, images, links) but drops everything else — <script>, on* handlers, javascript:/data: URLs. Pygments/codehilite output is a fixed structure, so an allowlist that permits <pre>/<code>/<span> plus the highlight class names preserves highlighting.

    Two things to get right, both confirmed against current (2026) tooling:

    1. Use nh3, not bleach. bleach — the library most people reach for first — is deprecated and no longer receives security fixes (it was built on the unmaintained html5lib). nh3 is the Python binding to Rust's ammonia sanitizer: allowlist-based, actively maintained, ~20× faster, and single-purpose. It exposes exactly the knobs needed here (tags, attributes, allowed_classes, url_schemes).
    2. Allowlist, never blocklist. Do not try to fix this by detecting or stripping "bad" patterns (<script>, known on* handlers, etc.) — that approach loses to case tricks, encoding, novel event handlers, and SVG/MathML vectors, and is the historical source of XSS filter bypasses. Parse-to-DOM + keep-only-known-safe (what nh3/ammonia does) is the only robust model. Validity checking is a separate concern and gives no security benefit — a perfectly valid document can host a malicious <script> — so it must not be mistaken for the fix here (it has a legitimate but lesser role as poster-facing "your html_fragment is malformed" UX feedback, tracked separately if wanted).
  • Decide html_fragment explicitly. It's marketed as themed "reports/cards/tables," not full pages, and it's shareable — either sanitize it like markdown, or if script is intended, document that html_fragment executes arbitrary JS so posters treat it like html_full trust-wise. Right now it's an in-between with neither sanitization nor a warning.

  • Add a Content-Security-Policy to artifact/shared views. A nonce-based policy (nonce on Daybook's own renderjson/gridjs/deck inline scripts, script-src 'self' 'nonce-…') neuters injected inline scripts even in the HTML-authoring kinds, and is the single highest-leverage mitigation since it covers every kind at once.

  • CSV formula injection (low/debatable): consider prefixing cells that begin with = + - @ with a ' in the raw sidecar, or just document it. Many tools treat this as the spreadsheet app's problem; noting it only for completeness, not urging a fix.

Notes

  • html_full is intentionally verbatim and is not being reported as a bug — only flagged so the fix doesn't accidentally start sanitizing it and break its documented contract.
  • json/csv defenses are genuinely solid; this issue is not about them beyond recording that they were tested and held.

Resolution

Fixed with two layers (both in artifacts/views.py):

  1. Allowlist sanitization at create time for the themed kinds (markdown, deck, html_fragment) via nh3 (the maintained ammonia/Rust sanitizer, per the issue's recommendation — not the deprecated bleach). sanitize_html() keeps ordinary formatting, tables, images, links, and the codehilite/Pygments markup, and drops <script>, on* handlers, <style>, unknown tags, and javascript:/data: URLs (url_schemes limited to http/https/mailto; relative /assets/<id> image URLs pass through). Verified live that markdown/deck/html_fragment strip script/onerror/onload/svg while tables, syntax highlighting, and relative images survive. html_full is deliberately not sanitized — it remains the documented verbatim escape hatch.

  2. Script-blocking CSP at serve time for the sanitized kinds: default-src 'self'; script-src 'none'; object-src 'none'; base-uri 'none', set in serve_artifact_file (so it covers both /a/{id} and the unauthenticated /s/{token} share route). Because it's a response header, it also neutralizes any script in artifacts stored before this fix — closing the retroactive gap that create-time sanitization alone would miss. Not applied to html_full (its scripts are by contract) or json/csv (their viewers need JS; their user data is separately escaped via escape_script_body).

Regression tests added in tests/api/test_artifacts.py (7): script/handler stripping for each themed kind, formatting/highlighting/relative-image preservation, html_full staying verbatim, and the CSP header being present for sanitized kinds and absent for html_full.

Not addressed here (separate, lower-priority items the issue noted):

  • CSV formula injection (=cmd|… in a cell) — left as documented behavior.
  • CSRF_COOKIE_HTTPONLY — moot now that the XSS vector is closed, but a cheap extra hardening if wanted (our JS reads the token from a form input, not the cookie, so enabling it wouldn't break anything).
  • A nonce/hash CSP for the json/csv/html_full pages — those keep their current behavior; the strict CSP is scoped to the no-JS themed kinds where it's provably safe.