security: harden URL schemes in non-HTML renderers#255
Merged
Conversation
Follow-up to the always-on HTML hardening: the Markdown, plain-text and ANSI renderers emitted link destinations and image sources verbatim, so a dangerous scheme (javascript:/vbscript:/data:/file:) carried straight into a Markdown or terminal export - markup that gets rendered or clicked elsewhere. - Add Djot\Util\UrlSafety with the shared scheme denylist + sanitize helpers. - MarkdownRenderer / AnsiRenderer blank a dangerous link/image URL; PlainText falls back to the link text. Safe and relative URLs are untouched. - Refactor HtmlRenderer to delegate its baseline to UrlSafety (single source). This is the renderer-side fix for the carry-through; the HtmlToDjot importer is deliberately not a sanitizer (its output is re-rendered through these now-hardened renderers).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #255 +/- ##
=========================================
Coverage 92.35% 92.35%
- Complexity 3628 3630 +2
=========================================
Files 108 109 +1
Lines 10250 10253 +3
=========================================
+ Hits 9466 9469 +3
Misses 784 784 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the always-on HTML hardening (#253). Closes the real carry-through behind the
HtmlToDjotimporter concern.Problem
The HTML renderer blanks dangerous URL schemes, but the Markdown / plain-text / ANSI renderers emitted link destinations and image sources verbatim. A
javascript:/vbscript:/data:/file:URL therefore carried straight into a Markdown or terminal export - markup that is rendered or clicked somewhere else (the same XSS path the importer worried about, just on the output side).The importer itself is deliberately not a sanitizer (documented at
HtmlToDjotline 27); its output is re-rendered through these renderers, which is the correct place to enforce the denylist. Doing it here covers every source - imported HTML, hand-written Djot, anything - not just the importer.Change
Djot\Util\UrlSafety- the shared dangerous-scheme denylist (javascript,vbscript,data,file) plushasDangerousScheme()/sanitize(). Scheme detection strips C0 controls + spaces, sojava\tscript:cannot evade.MarkdownRenderer/AnsiRendererblank a dangerous link/image URL;PlainTextRendererfalls back to the link text. Safe and relative URLs are untouched.HtmlRendererto delegate its always-on baseline toUrlSafety(single source of truth; behavior unchanged).