From 7720c75386ec650be054717f79872be8d5819477 Mon Sep 17 00:00:00 2001 From: hu13-test Date: Tue, 27 Oct 2026 16:03:05 +0000 Subject: [PATCH] GFM markdown + URL scheme allowlist (#71 #62) #71: markdown_to_html enables tables, task lists, strikethrough and footnotes, and wraps output in .markdown so the new table/task CSS applies (wrapping cells instead of scrolling). #62: link and image destinations are scheme-filtered. HTML entities are decoded first (javascript: and javascript: were live XSS), then C0/whitespace stripped; only http/https/mailto/tel and scheme-less URLs survive. Everything else (javascript:, data:, vbscript:, file:) becomes '#'. Unit tests cover mixed case, tab-splitting, entity obfuscation, image syntax, and that safe https/relative/mailto/tel/#anchor links are unchanged. --- src/build.rs | 188 ++++++++++++++++++++++++++++++++++++++++++-- static/style.css | 4 +- tests/responsive.rs | 8 +- 3 files changed, 190 insertions(+), 10 deletions(-) diff --git a/src/build.rs b/src/build.rs index a49f34e..0fcb898 100644 --- a/src/build.rs +++ b/src/build.rs @@ -105,22 +105,148 @@ pub struct BuildSummary { pub file_count: usize, } +/// Decode the HTML entities a browser would decode before treating a +/// URL as a scheme (`javascript:…`, `javascript:…`). +/// +/// `pulldown-cmark` writes `dest_url` into `href`/`src` and then escapes +/// it; a destination that still contains `:` will be decoded by the +/// *browser* into `:` — so the scheme check has to decode it first. +fn decode_entities(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + let mut rest = s; + while let Some(i) = rest.find('&') { + out.push_str(&rest[..i]); + let tail = &rest[i + 1..]; + let Some(semi) = tail.find(';') else { + out.push('&'); + rest = tail; + continue; + }; + let name = &tail[..semi]; + let decoded: Option = match name { + "colon" | "Colon" => Some(':'), + "sol" | "Tab" | "NewLine" => Some(match name { + "Tab" => '\t', + "NewLine" => '\n', + _ => '/', + }), + "amp" => Some('&'), + "lt" => Some('<'), + "gt" => Some('>'), + "quot" => Some('"'), + "apos" => Some('\''), + "nbsp" => Some('\u{a0}'), + _ => name.strip_prefix('#').and_then(|n| { + let n = n.strip_prefix(['x', 'X']).map_or_else( + || u32::from_str_radix(n, 10).ok(), + |hex| u32::from_str_radix(hex, 16).ok(), + ); + n.and_then(char::from_u32) + }), + }; + match decoded { + Some(ch) => { + out.push(ch); + rest = &tail[semi + 1..]; + } + None => { + out.push('&'); + rest = tail; + } + } + } + out.push_str(rest); + out +} + +/// `true` when `url` may be emitted into `href`/`src` (f0rge #62). +/// +/// Only `http`, `https`, `mailto` and `tel` are allowed as schemes. +/// Relative URLs (no scheme) are allowed. `javascript:`, `data:`, +/// `vbscript:`, `file:` etc. are rejected — including when obfuscated +/// with entities, control characters or mixed case. +fn safe_url(url: &str) -> bool { + let decoded = decode_entities(url); + // Browsers strip tab/newline/CR and leading/trailing C0/space before + // parsing the scheme. + let cleaned: String = decoded + .chars() + .filter(|c| !c.is_ascii_control() && !c.is_whitespace()) + .collect(); + let lower = cleaned.to_ascii_lowercase(); + let Some(colon) = lower.find(':') else { + return true; // no scheme: relative path + }; + let scheme = &lower[..colon]; + if scheme.is_empty() + || !scheme.chars().enumerate().all(|(i, c)| { + c.is_ascii_alphabetic() + || (i > 0 && (c.is_ascii_digit() || matches!(c, '+' | '-' | '.'))) + }) + { + return true; // `./a:b`, `#x:y`, `?q=1:2` — not a scheme + } + matches!(scheme, "http" | "https" | "mailto" | "tel") +} + /// Render markdown to HTML, escaping raw HTML. /// /// `pulldown-cmark` passes `Html`/`InlineHtml` events through untouched -/// (R1 finding), so map them through [`esc`] first. Everything else -/// (code spans, emphasis, lists) renders normally. +/// (R1 finding), so map them through [`esc`] first. GFM tables, task +/// lists and strikethrough are enabled (f0rge #71); link/image +/// destinations are scheme-filtered (f0rge #62). #[must_use] pub fn markdown_to_html(md: &str) -> String { - use pulldown_cmark::{html, Event, Options, Parser}; - let parser = Parser::new_ext(md, Options::empty()); + use pulldown_cmark::{html, Event, Options, Parser, Tag}; + let options = Options::ENABLE_TABLES + | Options::ENABLE_TASKLISTS + | Options::ENABLE_STRIKETHROUGH + | Options::ENABLE_FOOTNOTES; + let parser = Parser::new_ext(md, options); let mapped = parser.map(|ev| match ev { Event::Html(h) => Event::Html(esc(&h).into()), Event::InlineHtml(h) => Event::InlineHtml(esc(&h).into()), + Event::Start(Tag::Link { + link_type, + dest_url, + title, + id, + }) => { + let dest = if safe_url(&dest_url) { + decode_entities(&dest_url) + } else { + "#".to_string() + }; + Event::Start(Tag::Link { + link_type, + dest_url: dest.into(), + title, + id, + }) + } + Event::Start(Tag::Image { + link_type, + dest_url, + title, + id, + }) => { + let dest = if safe_url(&dest_url) { + decode_entities(&dest_url) + } else { + "#".to_string() + }; + Event::Start(Tag::Image { + link_type, + dest_url: dest.into(), + title, + id, + }) + } other => other, }); - let mut out = String::new(); + let mut out = String::from("
\n"); html::push_html(&mut out, mapped); + out.push_str("
\n"); out } @@ -1748,6 +1874,58 @@ mod tests { ); } + #[test] + fn markdown_enables_gfm_tables_tasks_strikethrough() { + let md = "| a | b |\n|---|---|\n| 1 | 2 |\n\n- [x] done\n- [ ] todo\n\n~~gone~~\n"; + let html = markdown_to_html(md); + let table = format!("<{}>", "table"); + assert!(html.contains(&table), "table parsed: {html}"); + assert!(html.contains("a"), "header cell"); + assert!(html.contains("1"), "body cell"); + assert!(html.contains("type=\"checkbox\""), "task checkbox"); + assert!(html.contains("checked"), "checked task preserved"); + assert!(html.contains("
  • "), "task list item"); + assert!(html.contains("gone"), "strikethrough"); + } + + #[test] + fn markdown_blocks_dangerous_link_schemes() { + for evil in [ + "[x](javascript:alert(1))", + "[x](JaVaScRiPt:alert(1))", + "[x](java\tscript:alert(1))", + "[x](javascript:alert(1))", + "[x](javascript:alert(1))", + "[x](data:text/html;base64,PHNjcmlwdD4=)", + "[x](vbscript:msgbox(1))", + "[x](file:///etc/passwd)", + "![i](javascript:alert(1))", + "![i](data:image/svg+xml,)", + ] { + let html = markdown_to_html(evil); + let lower = html.to_ascii_lowercase(); + for attr in ["href=", "src="] { + for bad in ["javascript:", "vbscript:", "data:", "file:"] { + assert!( + !lower.contains(&format!("{attr}\"{bad}")), + "{evil:?} must not emit {attr}{bad}: {html}" + ); + } + } + } + } + + #[test] + fn markdown_keeps_safe_links_and_relative_urls() { + let md = "[a](https://example.com/x?y=1&z=2) [b](/files/x.html) [c](mailto:a@b) [d](tel:+1) [e](#L3)"; + let html = markdown_to_html(md); + assert!(html.contains("href=\"https://example.com/x?y=1&z=2\"")); + assert!(html.contains("href=\"/files/x.html\"")); + assert!(html.contains("href=\"mailto:a@b\"")); + assert!(html.contains("href=\"tel:+1\"")); + assert!(html.contains("href=\"#L3\"")); + } + #[test] fn human_size_formats() { assert_eq!(human_size(0), "0 B"); diff --git a/static/style.css b/static/style.css index d764077..ea47b5f 100644 --- a/static/style.css +++ b/static/style.css @@ -159,8 +159,8 @@ button:hover,input[type=submit]:hover{background:var(--cyan)} .btn-new:hover,.btn-new:focus{background:var(--cyan);text-decoration:none} .muted{color:var(--muted)} /* GFM markdown (f0rge #71): task lists, strikethrough, footnotes. */ -.task-list-item{list-style:none} -.task-list-item input[type=checkbox]{margin:0 .4em 0 -1.2em;vertical-align:middle;accent-color:var(--green)} +li:has(>input[type=checkbox]){list-style:none} +.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)} .prose del,.del{color:var(--fg-dim);text-decoration:line-through} .footnotes{margin-top:var(--s5);font-size:var(--fs-sm);color:var(--fg-dim)} .footnotes ol{padding-left:var(--s5)} diff --git a/tests/responsive.rs b/tests/responsive.rs index bb72480..9c333b7 100644 --- a/tests/responsive.rs +++ b/tests/responsive.rs @@ -430,9 +430,11 @@ fn forge_list_tables_scroll_and_wrap_long_titles() { #[test] fn zero_bare_tables_in_sources() { // Audit EVERY template (m6, f0rge issue #17): no `` may open - // outside a `.table-scroll` container. All tables are emitted from - // src/*.rs render fns (markdown never emits tables), so grep the - // sources: each line opening a table must carry the scroll wrapper. + // outside a `.table-scroll` container. Hand-written tables are + // emitted from src/*.rs render fns, so grep the sources: each line + // opening a table must carry the scroll wrapper. (Markdown tables are + // emitted by pulldown-cmark at runtime into `.markdown`, which wraps + // by word instead of scrolling.) let mut checked = 0; let mut stack = vec![manifest().join("src")]; while let Some(d) = stack.pop() { -- 2.56.0