daybook/issues/014-stored-xss-in-rendered-text-artifacts.md

# 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 | Passes through into the `?raw=1` sidecar verbatim (classic CSV/formula injection). Low severity / debatable — see below. |

## Root cause (markdown specifically)

`create_text_artifact` renders markdown with:

```python
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.