fix(security): allowlist markdown link/image schemes (LC-154) #189

Merged

nrupard merged 2 commits from feat/lc-154-markdown-link-scheme into main 2026-05-22 21:10:06 +02:00

Conversation 0 Commits 2 Files changed 1 +167 -2

nrupard commented 2026-05-22 20:56:32 +02:00

Owner

Copy link

What

Closes the markdown link-scheme XSS from the LC-148 audit (finding S8).

Problem

pulldown_cmark writes a link destination into the href verbatim, and the renderer's Tag::Link arm passed the event straight through. So a message body [click](javascript:alert(document.cookie)) rendered <a href="javascript:..."> - click-to-execute XSS in any rendered message. (Bare-URL linkify was already safe; this is the explicit [label](dest) path.)

Fix

link_scheme_is_safe: allows relative references (fragment / query / path / scheme-less) and the http / https / mailto schemes; everything else (javascript:, data:, vbscript:, file:, ...) is neutralized by rewriting the destination to #, keeping the visible label but making the link inert. A colon after a / is treated as a path (not a scheme), so path/to:thing-style relative refs are preserved. Applied to both Tag::Link and Tag::Image (img src is defense-in-depth - a javascript: src doesn't execute in modern browsers, but a disallowed scheme has no business there).

Test

javascript: / data: / vbscript: / file: neutralized (case-insensitive), http/https/mailto/relative preserved, plus a link_scheme_is_safe predicate matrix. just check / just test / just test-saas green (the routes_uploads flake is pre-existing; no upload code touched).

Part of the LC-159 post-audit story.

🤖 Generated with Claude Code

## What Closes the markdown link-scheme XSS from the LC-148 audit (finding S8). ## Problem `pulldown_cmark` writes a link destination into the `href` verbatim, and the renderer's `Tag::Link` arm passed the event straight through. So a message body `[click](javascript:alert(document.cookie))` rendered `<a href="javascript:...">` - click-to-execute XSS in any rendered message. (Bare-URL linkify was already safe; this is the explicit `[label](dest)` path.) ## Fix `link_scheme_is_safe`: allows relative references (fragment / query / path / scheme-less) and the `http` / `https` / `mailto` schemes; everything else (`javascript:`, `data:`, `vbscript:`, `file:`, ...) is neutralized by rewriting the destination to `#`, keeping the visible label but making the link inert. A colon after a `/` is treated as a path (not a scheme), so `path/to:thing`-style relative refs are preserved. Applied to both `Tag::Link` and `Tag::Image` (img src is defense-in-depth - a `javascript:` src doesn't execute in modern browsers, but a disallowed scheme has no business there). ## Test `javascript:` / `data:` / `vbscript:` / `file:` neutralized (case-insensitive), `http`/`https`/`mailto`/relative preserved, plus a `link_scheme_is_safe` predicate matrix. `just check` / `just test` / `just test-saas` green (the `routes_uploads` flake is pre-existing; no upload code touched). Part of the LC-159 post-audit story. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

nrupard added 1 commit 2026-05-22 20:56:32 +02:00

fix(security): allowlist markdown link/image schemes (LC-154) ... cd3d54e38d

pulldown_cmark writes a link destination into the href verbatim, and the renderer's Tag::Link arm passed the event through unchanged, so a message body [click](javascript:alert(document.cookie)) rendered a click-to-execute XSS anchor (audit S8). (Bare-URL linkify was already safe; this is the explicit [label](dest) path.)

Adds link_scheme_is_safe: allows relative refs (fragment / query / path / scheme-less) and the http/https/mailto schemes; everything else (javascript:, data:, vbscript:, file:, ...) is neutralized by rewriting the destination to "#", which keeps the visible label but makes the link inert. Applied to both Tag::Link and Tag::Image (image src is defense-in-depth: a javascript: img src does not execute in modern browsers, but a disallowed scheme has no business there). A colon that appears after a "/" is treated as a path, not a scheme, so relative refs like path/to:thing are preserved.

Tests: javascript:/data:/vbscript:/file: neutralized (case-insensitive), http/https/mailto/relative preserved, plus a predicate matrix.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

nrupard added 1 commit 2026-05-22 21:00:40 +02:00

fix(security): also scheme-allowlist the login-body markdown renderer (LC-154) ... 3497de96ee

render_login_body has its own pulldown_cmark loop that passed Tag::Link / Tag::Image through unchanged, so an admin-set login heading body could smuggle a javascript: link to every pre-auth visitor. Apply the same link_scheme_is_safe allowlist there. New test covers it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

nrupard merged commit 925b319ac0 into main 2026-05-22 21:10:06 +02:00

nrupard deleted branch feat/lc-154-markdown-link-scheme 2026-05-22 21:10:06 +02:00

nrupard referenced this pull request from a commit 2026-05-22 21:10:08 +02:00

Merge pull request 'fix(security): allowlist markdown link/image schemes (LC-154)' (#189) from feat/lc-154-markdown-link-scheme into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

psa-systems/lets-chat!189

No description provided.