#53 GFM markdown + URL scheme allowlist (#71 #62)

mergedpassopened
details
branches
master ← issue-71-gfm-markdown
issues
382214e0-86e3-4a06-851f-6d00e9d429ca
updated
id
2b56eb82-e562-4fb3-818a-41694f31ef92

Patch

src/build.rs+183 −5
diff --git a/src/build.rs b/src/build.rsindex a49f34e..0fcb898 100644--- a/src/build.rs+++ b/src/build.rs@@ -105,22 +105,148 @@ pub struct BuildSummary {105105     pub file_count: usize,106106 }107107 108+/// Decode the HTML entities a browser would decode before treating a109+/// URL as a scheme (`javascript&colon;…`, `&#106;avascript:…`).110+///111+/// `pulldown-cmark` writes `dest_url` into `href`/`src` and then escapes112+/// it; a destination that still contains `&colon;` will be decoded by the113+/// *browser* into `:` — so the scheme check has to decode it first.114+fn decode_entities(s: &str) -> String {115+    let mut out = String::with_capacity(s.len());116+    let mut rest = s;117+    while let Some(i) = rest.find('&') {118+        out.push_str(&rest[..i]);119+        let tail = &rest[i + 1..];120+        let Some(semi) = tail.find(';') else {121+            out.push('&');122+            rest = tail;123+            continue;124+        };125+        let name = &tail[..semi];126+        let decoded: Option<char> = match name {127+            "colon" | "Colon" => Some(':'),128+            "sol" | "Tab" | "NewLine" => Some(match name {129+                "Tab" => '\t',130+                "NewLine" => '\n',131+                _ => '/',132+            }),133+            "amp" => Some('&'),134+            "lt" => Some('<'),135+            "gt" => Some('>'),136+            "quot" => Some('"'),137+            "apos" => Some('\''),138+            "nbsp" => Some('\u{a0}'),139+            _ => name.strip_prefix('#').and_then(|n| {140+                let n = n.strip_prefix(['x', 'X']).map_or_else(141+                    || u32::from_str_radix(n, 10).ok(),142+                    |hex| u32::from_str_radix(hex, 16).ok(),143+                );144+                n.and_then(char::from_u32)145+            }),146+        };147+        match decoded {148+            Some(ch) => {149+                out.push(ch);150+                rest = &tail[semi + 1..];151+            }152+            None => {153+                out.push('&');154+                rest = tail;155+            }156+        }157+    }158+    out.push_str(rest);159+    out160+}161+162+/// `true` when `url` may be emitted into `href`/`src` (f0rge #62).163+///164+/// Only `http`, `https`, `mailto` and `tel` are allowed as schemes.165+/// Relative URLs (no scheme) are allowed. `javascript:`, `data:`,166+/// `vbscript:`, `file:` etc. are rejected — including when obfuscated167+/// with entities, control characters or mixed case.168+fn safe_url(url: &str) -> bool {169+    let decoded = decode_entities(url);170+    // Browsers strip tab/newline/CR and leading/trailing C0/space before171+    // parsing the scheme.172+    let cleaned: String = decoded173+        .chars()174+        .filter(|c| !c.is_ascii_control() && !c.is_whitespace())175+        .collect();176+    let lower = cleaned.to_ascii_lowercase();177+    let Some(colon) = lower.find(':') else {178+        return true; // no scheme: relative path179+    };180+    let scheme = &lower[..colon];181+    if scheme.is_empty()182+        || !scheme.chars().enumerate().all(|(i, c)| {183+            c.is_ascii_alphabetic()184+                || (i > 0 && (c.is_ascii_digit() || matches!(c, '+' | '-' | '.')))185+        })186+    {187+        return true; // `./a:b`, `#x:y`, `?q=1:2` — not a scheme188+    }189+    matches!(scheme, "http" | "https" | "mailto" | "tel")190+}191+108192 /// Render markdown to HTML, escaping raw HTML.109193 ///110194 /// `pulldown-cmark` passes `Html`/`InlineHtml` events through untouched111-/// (R1 finding), so map them through [`esc`] first. Everything else112-/// (code spans, emphasis, lists) renders normally.195+/// (R1 finding), so map them through [`esc`] first. GFM tables, task196+/// lists and strikethrough are enabled (f0rge #71); link/image197+/// destinations are scheme-filtered (f0rge #62).113198 #[must_use]114199 pub fn markdown_to_html(md: &str) -> String {115-    use pulldown_cmark::{html, Event, Options, Parser};116-    let parser = Parser::new_ext(md, Options::empty());200+    use pulldown_cmark::{html, Event, Options, Parser, Tag};201+    let options = Options::ENABLE_TABLES202+        | Options::ENABLE_TASKLISTS203+        | Options::ENABLE_STRIKETHROUGH204+        | Options::ENABLE_FOOTNOTES;205+    let parser = Parser::new_ext(md, options);117206     let mapped = parser.map(|ev| match ev {118207         Event::Html(h) => Event::Html(esc(&h).into()),119208         Event::InlineHtml(h) => Event::InlineHtml(esc(&h).into()),209+        Event::Start(Tag::Link {210+            link_type,211+            dest_url,212+            title,213+            id,214+        }) => {215+            let dest = if safe_url(&dest_url) {216+                decode_entities(&dest_url)217+            } else {218+                "#".to_string()219+            };220+            Event::Start(Tag::Link {221+                link_type,222+                dest_url: dest.into(),223+                title,224+                id,225+            })226+        }227+        Event::Start(Tag::Image {228+            link_type,229+            dest_url,230+            title,231+            id,232+        }) => {233+            let dest = if safe_url(&dest_url) {234+                decode_entities(&dest_url)235+            } else {236+                "#".to_string()237+            };238+            Event::Start(Tag::Image {239+                link_type,240+                dest_url: dest.into(),241+                title,242+                id,243+            })244+        }120245         other => other,121246     });122-    let mut out = String::new();247+    let mut out = String::from("<div class=\"markdown\">\n");123248     html::push_html(&mut out, mapped);249+    out.push_str("</div>\n");124250     out125251 }126252 @@ -1748,6 +1874,58 @@ mod tests {17481874         );17491875     }17501876 1877+    #[test]1878+    fn markdown_enables_gfm_tables_tasks_strikethrough() {1879+        let md = "| a | b |\n|---|---|\n| 1 | 2 |\n\n- [x] done\n- [ ] todo\n\n~~gone~~\n";1880+        let html = markdown_to_html(md);1881+        let table = format!("<{}>", "table");1882+        assert!(html.contains(&table), "table parsed: {html}");1883+        assert!(html.contains("<th>a</th>"), "header cell");1884+        assert!(html.contains("<td>1</td>"), "body cell");1885+        assert!(html.contains("type=\"checkbox\""), "task checkbox");1886+        assert!(html.contains("checked"), "checked task preserved");1887+        assert!(html.contains("<li>"), "task list item");1888+        assert!(html.contains("<del>gone</del>"), "strikethrough");1889+    }1890+1891+    #[test]1892+    fn markdown_blocks_dangerous_link_schemes() {1893+        for evil in [1894+            "[x](javascript:alert(1))",1895+            "[x](JaVaScRiPt:alert(1))",1896+            "[x](java\tscript:alert(1))",1897+            "[x](javascript&colon;alert(1))",1898+            "[x](&#106;avascript:alert(1))",1899+            "[x](data:text/html;base64,PHNjcmlwdD4=)",1900+            "[x](vbscript:msgbox(1))",1901+            "[x](file:///etc/passwd)",1902+            "![i](javascript:alert(1))",1903+            "![i](data:image/svg+xml,<svg onload=alert(1)>)",1904+        ] {1905+            let html = markdown_to_html(evil);1906+            let lower = html.to_ascii_lowercase();1907+            for attr in ["href=", "src="] {1908+                for bad in ["javascript:", "vbscript:", "data:", "file:"] {1909+                    assert!(1910+                        !lower.contains(&format!("{attr}\"{bad}")),1911+                        "{evil:?} must not emit {attr}{bad}: {html}"1912+                    );1913+                }1914+            }1915+        }1916+    }1917+1918+    #[test]1919+    fn markdown_keeps_safe_links_and_relative_urls() {1920+        let md = "[a](https://example.com/x?y=1&z=2) [b](/files/x.html) [c](mailto:a@b) [d](tel:+1) [e](#L3)";1921+        let html = markdown_to_html(md);1922+        assert!(html.contains("href=\"https://example.com/x?y=1&amp;z=2\""));1923+        assert!(html.contains("href=\"/files/x.html\""));1924+        assert!(html.contains("href=\"mailto:a@b\""));1925+        assert!(html.contains("href=\"tel:+1\""));1926+        assert!(html.contains("href=\"#L3\""));1927+    }1928+17511929     #[test]17521930     fn human_size_formats() {17531931         assert_eq!(human_size(0), "0 B");
static/style.css+2 −2
diff --git a/static/style.css b/static/style.cssindex d764077..ea47b5f 100644--- a/static/style.css+++ b/static/style.css@@ -159,8 +159,8 @@ button:hover,input[type=submit]:hover{background:var(--cyan)}159159 .btn-new:hover,.btn-new:focus{background:var(--cyan);text-decoration:none}160160 .muted{color:var(--muted)}161161 /* GFM markdown (f0rge #71): task lists, strikethrough, footnotes. */162-.task-list-item{list-style:none}163-.task-list-item input[type=checkbox]{margin:0 .4em 0 -1.2em;vertical-align:middle;accent-color:var(--green)}162+li:has(>input[type=checkbox]){list-style:none}163+.markdown input[type=checkbox],li>input[type=checkbox]{width:auto;height:auto;min-height:0;margin:0 .4em 0 -1.2em;vertical-align:middle;accent-color:var(--green)}164164 .prose del,.del{color:var(--fg-dim);text-decoration:line-through}165165 .footnotes{margin-top:var(--s5);font-size:var(--fs-sm);color:var(--fg-dim)}166166 .footnotes ol{padding-left:var(--s5)}
tests/responsive.rs+5 −4
diff --git a/tests/responsive.rs b/tests/responsive.rsindex bb72480..9c333b7 100644--- a/tests/responsive.rs+++ b/tests/responsive.rs@@ -430,9 +430,11 @@ fn forge_list_tables_scroll_and_wrap_long_titles() {430430 #[test]431431 fn zero_bare_tables_in_sources() {432432     // Audit EVERY template (m6, f0rge issue #17): no `<table>` may open433-    // outside a `.table-scroll` container. All tables are emitted from434-    // src/*.rs render fns (markdown never emits tables), so grep the435-    // sources: each line opening a table must carry the scroll wrapper.433+    // outside a `.table-scroll` container. Hand-written tables are434+    // emitted from src/*.rs render fns, so grep the sources: each line435+    // opening a table must carry the scroll wrapper. (Markdown tables are436+    // emitted by pulldown-cmark at runtime into `.markdown`, which wraps437+    // by word instead of scrolling.)436438     let mut checked = 0;437439     let mut stack = vec![manifest().join("src")];438440     while let Some(d) = stack.pop() {439-- 2.56.0

Comments (0)

No comments yet.

Reviews (0)

No reviews yet.

all pulls