Commit e518a91 h

Pat Ambrosio · 26-09-05 10:19

Escape by default, and close the attribute and URL holes

Changes: 3 files +185 -27

Changes (Side-by-Side)

Raw

commit e518a9122b6d709032ecfd669f2446fdad7a36c9
Author: Pat Ambrosio <cp.ambrosio@gmail.com>
Date:   Sat Sep 5 10:19:10 2026 +0800

    Escape by default, and close the attribute and URL holes
    
    This renders documents it did not write onto pages other people read, and
    it cannot tell an author's deliberate <div> from an injected <script>. So
    raw HTML is escaped now unless the caller asks for passthrough by name:
    
        awk -v UNSAFE_HTML=1 -f markdown.awk
    
    A caller who forgets the flag is safe; one who wants passthrough has said
    so somewhere greppable. The flag covers whole tags an author wrote. It
    does not cover the attributes this builds itself -- nobody asks for a
    quote breaking out of an attribute -- so those are always escaped.
    
    Every attribute built from document text now goes through escattr(). Six
    sites were concatenating it raw, and a quote closed the attribute early
    so what followed became a live handler:
    
      - citation id, into data-citation
      - footnote reference, into data-fnref, href and id -- three at once
      - fence language, into class
      - footnote list entry, into id and the backref href -- twice
      - citation list entry, the same shape
      - reference definition titles
    
    Three of those were not in the report; they turned up in the audit of
    every attribute site, which is why the audit was worth doing rather than
    patching the named three.
    
    Schemes that execute are dropped: javascript:, data:, vbscript:, file:.
    The test is made on a folded copy -- lowercased, leading control
    characters and whitespace stripped, entity and percent escapes that could
    hide the colon decoded -- because "JaVaScRiPt:", "  javascript:" and
    "java&#115;cript:" are the same scheme written to slip past a prefix
    test.
    
    One subtlety in the reference definitions: sub() reads & in a replacement
    as the text that matched, so an escaped value containing &quot; spliced
    the placeholder into its own replacement and produced "<<rquot;". The
    ampersands are doubled at substitution time now.
    
    Seventeen tests, one per payload, each checked to fail without its fix:
    disabling the scheme filter fails six, disabling the escaping fails five.

diff --git a/markdown.awk b/markdown.awk
index c486c76..692da99 100755
--- a/markdown.awk
+++ b/markdown.awk
@@ -6,6 +6,32 @@
 # See: http://4l77.com/src/md2html.awk
 
 # Attribute values need quotes escaped as well as the usual entities.
+# sanitize_url <url>
+# Empty for any scheme that executes, so a link cannot run code.
+#
+# href="javascript:..." is a script the reader triggers by clicking, and
+# data: and vbscript: and file: are the same problem wearing other names.
+# Nothing in a document written as prose needs them, and a parser whose
+# output is served to other people cannot tell a deliberate one from an
+# injected one.
+#
+# The check is made on a folded copy: lowercased, with leading control
+# characters and whitespace removed, and with the HTML entities and percent
+# escapes that could hide a colon decoded far enough to expose it. A scheme
+# is only a scheme up to its first colon, so anything before that colon is
+# what has to be read -- "java&#115;cript:" and "  JaVaScRiPt:" are the same
+# scheme written to slip past a plain prefix test.
+function sanitize_url(u,   probe) {
+	probe = tolower(u)
+	gsub(/[\000-\040]/, "", probe)
+	gsub(/&#0*9;|&#0*10;|&#0*13;|&tab;|&newline;/, "", probe)
+	gsub(/&#0*58;|&colon;|%3a/, ":", probe)
+	gsub(/&#0*x?[0-9a-f]+;?/, "", probe)
+	if (probe ~ /^(javascript|data|vbscript|file):/)
+		return ""
+	return u
+}
+
 function escattr(t) {
 	t = eschtml(t);
 	gsub(">", "\\&gt;", t);
@@ -34,8 +60,14 @@ function oprint(t){
 		otext = otext "\n" t;
 }
 
-function subref(id){
-	for(; nr > 0 && sub("<<" id, ref[id], otext); nr--);
+function subref(id,   rep){
+	# The stored value is already escaped, so it can contain &quot; -- and
+	# sub() reads & in a replacement as "the text that matched", which would
+	# splice the placeholder back into its own replacement. Doubling the
+	# ampersands makes sub() emit them literally.
+	rep = ref[id]
+	gsub(/&/, "\\\\&", rep)
+	for(; nr > 0 && sub("<<" id, rep, otext); nr--);
 	if(nr == 0 && otext) {
 		print otext;
 		otext = "";
@@ -104,13 +136,22 @@ function nextil(t) {
 			linktext = url;
 			if(match(url, /@/) && !match(url, /^mailto:/))
 				url = "mailto:" url;
-			return t1 "<a href=\"" escattr(url) "\">" linktext "</a>" nextil(t2);
+			return t1 "<a href=\"" escattr(sanitize_url(url)) "\">" linktext "</a>" nextil(t2);
 		}
 	# Html tags
+	# Passed through only when the caller has asked for it. A parser whose
+	# output is served to other people cannot tell an author's deliberate
+	# <div> from an injected <script>, and the safe reading has to be the
+	# one you get without saying anything.
 		if(match(t2, /^[A-Za-z\/!][^>]*>/)){
 			tag = tag substr(t2, RSTART, RLENGTH);
-			t2 = substr(t2, RLENGTH + 1);
-			return t1 tag nextil(t2);
+			t2 = substr(t2, RSTART + RLENGTH);
+			if(UNSAFE_HTML)
+				return t1 tag nextil(t2);
+			# escattr, not eschtml: eschtml leaves > alone because a bare >
+			# is ordinary in prose, but here the whole tag is being shown as
+			# text and a half-escaped tag reads as broken markup.
+			return t1 escattr(tag) nextil(t2);
 		}
 		return t1 "&lt;" nextil(t2);
 	}
@@ -145,7 +186,7 @@ function nextil(t) {
 			}
 			if(match(url, /^<.*>$/))
 				url = substr(url, 2, RLENGTH - 2);
-			return t1 "<img src=\"" escattr(url) "\" alt=\"" escattr(alt) "\"" title " />" nextil(t2);
+			return t1 "<img src=\"" escattr(sanitize_url(url)) "\" alt=\"" escattr(alt) "\"" title " />" nextil(t2);
 		}
 		else{
 			# Referenced
@@ -186,7 +227,7 @@ function nextil(t) {
 		cloc = cite_locator;
 		cite_locator = "";
 		clabel = (cloc != "") ? "(" cloc ", " cnum ")" : "(" cnum ")";
-		return t1 "<a href=\"#cn:" cnum "\" id=\"cnref:" cnum "\" data-citation=\"" cid \
+		return t1 "<a href=\"#cn:" cnum "\" id=\"cnref:" cnum "\" data-citation=\"" escattr(cid) \
 			"\" title=\"see citation\" class=\"citation\">" clabel "</a>" nextil(t2);
 	}
 
@@ -195,7 +236,7 @@ function nextil(t) {
 		match(t2, /^[^\]]*(\[[^\]]*\][^\]]*)*/);
 		linktext = substr(t2, 1, RLENGTH);
 		t2 = substr(t2, RLENGTH + 2);
-    return t1 "<sup class=\"fnref\" data-fnref=\"" linktext "\"><a href=\"#fn-" linktext "\" id=\"fnref-" linktext "\">" linktext "</a></sup>" nextil(t2);
+    return t1 "<sup class=\"fnref\" data-fnref=\"" escattr(linktext) "\"><a href=\"#fn-" escattr(linktext) "\" id=\"fnref-" escattr(linktext) "\">" eschtml(linktext) "</a></sup>" nextil(t2);
 	}
 	# Links
 	if(tag == "["){
@@ -227,7 +268,7 @@ function nextil(t) {
 			if(match(url, /^<.*>$/))
 				url = substr(url, 2, RLENGTH - 2);
 			url = eschtml(url);
-			return t1 "<a href=\"" escattr(url) "\"" title ">" nextil(linktext) "</a>" nextil(pt2);
+			return t1 "<a href=\"" escattr(sanitize_url(url)) "\"" title ">" nextil(linktext) "</a>" nextil(pt2);
 		}
 		else{
 			# Referenced: [text][id], [text][] or the shortcut [text].
@@ -483,9 +524,12 @@ BEGIN {
 	sub(/^[^\]]*\]:[ 	]+/, "");
 	title = "";
 	if(match($0, /".*"$/))
-		title = "\" title=\"" substr($0, RSTART + 1, RLENGTH - 2);
+		title = "\" title=\"" escattr(substr($0, RSTART + 1, RLENGTH - 2));
 	sub(/[ 	]+".*"$/, "");
-	url = eschtml($0);
+	# The url lands in an href or src, so it is escaped as an attribute and
+	# filtered for schemes that execute -- a reference definition is as good
+	# a place to hide a javascript: url as an inline link is.
+	url = escattr(sanitize_url($0));
 	ref[id] = url title;
 
 	subref(id);
@@ -500,9 +544,12 @@ BEGIN {
 	id = tolower(id);
 	title = "";
 	if(match($0, /".*"$/))
-		title = "\" title=\"" substr($0, RSTART + 1, RLENGTH - 2);
+		title = "\" title=\"" escattr(substr($0, RSTART + 1, RLENGTH - 2));
 	sub(/[ 	]+".*"$/, "");
-	url = eschtml($0);
+	# The url lands in an href or src, so it is escaped as an attribute and
+	# filtered for schemes that execute -- a reference definition is as good
+	# a place to hide a javascript: url as an inline link is.
+	url = escattr(sanitize_url($0));
 	ref[id] = url title;
 
 	subref(id);
@@ -537,7 +584,12 @@ BEGIN {
 }
 
 # html
-!html && /^<(address|blockquote|center|dir|div|dl|fieldset|form|h[1-6r]|\
+# Block-level raw HTML, passed through only when the caller has asked for it
+# with -v UNSAFE_HTML=1. Without the flag the line is not recognised as HTML
+# at all and falls through to ordinary paragraph handling, which escapes it --
+# so a document that opens with <script> renders that tag as text rather than
+# running it.
+UNSAFE_HTML && !html && /^<(address|blockquote|center|dir|div|dl|fieldset|form|h[1-6r]|\
 isindex|menu|noframes|noscript|ol|p|pre|table|ul|!--)/ {
 	if(code)
 		oprint(code_held "</code></pre>"); code_held = "";
@@ -554,7 +606,7 @@ isindex|menu|noframes|noscript|ol|p|pre|table|ul|!--)/ {
 	next;
 }
 
-html && (/(^<\/(address|blockquote|center|dir|div|dl|fieldset|form|h[1-6r]|\
+UNSAFE_HTML && html && (/(^<\/(address|blockquote|center|dir|div|dl|fieldset|form|h[1-6r]|\
 isindex|menu|noframes|noscript|ol|p|pre|table|ul).*)|(--)>$/ ||
 (hr && />$/)) {
 	html = 0;
@@ -788,10 +840,10 @@ fence {
 	if(lang == "mermaid" || lang == "mermaid-legend"){
 		fence_raw = 1;
 		fence_close = "</div>";
-		fence_open = "<div class=\"diagram diagram-" eschtml(lang) "\">";
+		fence_open = "<div class=\"diagram diagram-" escattr(lang) "\">";
 	}
 	else if(info != "")
-		fence_open = "<pre><code class=\"language-" eschtml(info) "\">";
+		fence_open = "<pre><code class=\"language-" escattr(info) "\">";
 	else
 		fence_open = "<pre><code>";
 	next;
@@ -1017,8 +1069,8 @@ END {
     print "<ol>";
     for(ci = 1; ci <= cite_n; ci++){
       cid = cite_order[ci];
-      print "<li id=\"cn:" ci "\"><p>" inline(cite_body[cid]) \
-        " <a href=\"#cnref:" ci "\" title=\"return to body\" class=\"reversecitation\">&#160;&#8617;&#xfe0e;</a></p></li>";
+      print "<li id=\"cn:" escattr(ci) "\"><p>" inline(cite_body[cid]) \
+        " <a href=\"#cnref:" escattr(ci) "\" title=\"return to body\" class=\"reversecitation\">&#160;&#8617;&#xfe0e;</a></p></li>";
     }
     print "</ol>";
     print "</div>";
@@ -1027,7 +1079,7 @@ END {
   # Print footnotes
   if(alen(fnref)>0) {
     print "<ul class=\"fn-list\">";
-    for (i in fnref) print "<li id=\"fn-" i "\" class=\"fn-item\"><span class=\"fn-handle\">" i ": </span><span class=\"fn-text\">" inline(fnref[i]) " <a href=\"#fnref-" i "\" class=\"fn-backref\">&#x21A9;&#xFE0E;</a></span></li>";
+    for (i in fnref) print "<li id=\"fn-" escattr(i) "\" class=\"fn-item\"><span class=\"fn-handle\">" eschtml(i) ": </span><span class=\"fn-text\">" inline(fnref[i]) " <a href=\"#fnref-" escattr(i) "\" class=\"fn-backref\">&#x21A9;&#xFE0E;</a></span></li>";
     print "</ul>";
   }
 }
diff --git a/test/regressions.sh b/test/regressions.sh
index cd6f732..04595e9 100755
--- a/test/regressions.sh
+++ b/test/regressions.sh
@@ -13,6 +13,10 @@ md_render() {
   printf '%s\n' "$1" | "$AWK" -f "$REPO_ROOT/markdown.awk"
 }
 
+md_unsafe() {
+  printf '%s\n' "$1" | "$AWK" -v UNSAFE_HTML=1 -f "$REPO_ROOT/markdown.awk"
+}
+
 suite "nested lists"
 
 # The nesting machinery existed but the indent unit was a tab or four spaces,
@@ -89,16 +93,20 @@ assert_contains "$(md_render 'Unpaired ~~ stays.')" 'Unpaired ~~ stays.'
 it "tildes inside code are untouched"
 assert_contains "$(md_render 'A `~~code~~` span.')" '<code>~~code~~</code>'
 
+# These exercise how underscores interact with real tags, which is only
+# observable while tags survive -- so they run in the opt-in passthrough mode.
+# The escaping default has its own tests; what is checked here is the parsing
+# rule, not which mode is the default.
 suite "underscores in inline HTML"
 
 it "an underscore that is a tag's whole content is literal"
 # <span class="us">_</span> writes a visible underscore. It opened an <em>
 # that swallowed the markup after it.
-assert_contains "$(md_render 'x <span class="us">_</span> y')" '<span class="us">_</span>'
+assert_contains "$(md_unsafe 'x <span class="us">_</span> y')" '<span class="us">_</span>'
 
 it "two such underscores do not pair across their tags"
 # They emitted mis-nested "<b><em></b> ... <b></em></b>".
-pair=$(md_render 'A <b>_</b> and <b>_</b> here.')
+pair=$(md_unsafe 'A <b>_</b> and <b>_</b> here.')
 assert_contains "$pair" '<b>_</b> and <b>_</b>'
 assert_not_contains "$pair" '<em>'
 
@@ -109,7 +117,7 @@ it "bold still works"
 assert_contains "$(md_render 'Real __bold__ here.')" '<strong>bold</strong>'
 
 it "emphasis spanning a tag still works"
-assert_contains "$(md_render 'A _em <b>x</b> more_ end.')" '<em>em <b>x</b> more</em>'
+assert_contains "$(md_unsafe 'A _em <b>x</b> more_ end.')" '<em>em <b>x</b> more</em>'
 
 it "intraword underscores are still literal"
 assert_contains "$(md_render 'Use node_modules and snake_case_name.')" 'node_modules and snake_case_name'
diff --git a/test/units.sh b/test/units.sh
index 5f965a6..539d164 100755
--- a/test/units.sh
+++ b/test/units.sh
@@ -16,6 +16,7 @@ suite "markdown: intraword underscores"
 # paragraph carries an anchor -- that has tests of its own, and pinning a
 # content hash in every one of these would make them unreadable and would
 # break them all again on any wording change.
+md_unsafe() { printf '%s\n' "$1" | "$AWK" -v UNSAFE_HTML=1 -f "$REPO_ROOT/markdown.awk"; }
 md() { printf '%s\n' "$1" | "$AWK" -f "$REPO_ROOT/markdown.awk" \
        | "$SED" 's/<p id="p-[0-9a-f]*\(-[0-9]*\)\{0,1\}">/<p>/g'; }
 
@@ -356,12 +357,18 @@ After.')" "<p>After.</p>"
 it "a pipe in prose is not a table"
 assert_not_contains "$(md 'a | b in ordinary prose')" "<table>"
 
-# Markdown deliberately allows inline HTML, in cells as in prose.
-# multimarkdown 6.6.0 emits <th> <b>x</b> </th> for this, and this parser's
-# prose handling passes raw HTML through too, so a cell must not differ.
-it "raw html in a cell passes through, as in prose"
+# A cell must handle raw HTML exactly as prose does. That used to mean both
+# passed it through; it now means both escape it unless the caller asks for
+# passthrough. What matters is that a cell is not a hole in whichever rule is
+# in force.
+it "raw html in a cell is escaped, as in prose"
 assert_contains "$(md '| <b>x</b> |
 |---|
+| a |')" "&lt;b&gt;x&lt;/b&gt;"
+
+it "raw html in a cell passes through when the caller asks, as in prose"
+assert_contains "$(md_unsafe '| <b>x</b> |
+|---|
 | a |')" "<b>x</b>"
 
 it "a table inside a fence is literal"
@@ -545,4 +552,95 @@ assert_contains "$(md '<https://example.com>')" '<a href="https://example.com">h
 it "a quote in an autolink cannot close the attribute"
 assert_contains "$(md '<https://x.com/?a="b">')" 'href="https://x.com/?a=&quot;b&quot;"'
 
+suite "markdown: html is escaped unless the caller opts in"
+
+# The parser renders documents it did not write onto pages other people read,
+# and it cannot tell an author's deliberate <div> from an injected <script>.
+# So escaping is what you get without saying anything, and passthrough is what
+# you have to ask for by name -- a caller who forgets the flag is safe, and a
+# caller who wants raw HTML has said so somewhere greppable.
+
+it "a script tag is shown as text, not run"
+out=$(md '<script>alert(1)</script>')
+assert_contains "$out" '&lt;script&gt;'
+assert_not_contains "$out" '<script>'
+
+it "an inline tag with a handler is shown as text"
+out=$(md 'x <img src=y onerror=alert(1)>')
+assert_not_contains "$out" '<img'
+
+it "a block-level tag is shown as text"
+out=$(md '<div class="x">
+raw
+</div>')
+assert_not_contains "$out" '<div class="x">'
+
+it "the flag brings passthrough back"
+assert_contains "$(md_unsafe 'a <b>bold</b>')" '<b>bold</b>'
+
+it "the flag does not disable attribute escaping"
+# The flag is about whole tags the author wrote. A quote breaking out of an
+# attribute the parser built is never something a caller asked for.
+assert_not_contains "$(md_unsafe 'x[^f" onclick="alert(1)]')" '" onclick="alert(1)"'
+
+suite "markdown: attributes cannot be broken out of"
+
+# Each of these put user text into an attribute without escaping the quote, so
+# the quote closed the attribute early and what followed became a live handler.
+
+it "a citation id cannot carry a handler"
+out=$(md 'x[#c" onclick="alert(1)]
+
+[#c" onclick="alert(1)]: A source')
+assert_not_contains "$out" '" onclick="alert(1)"'
+assert_contains "$out" 'data-citation="c&quot;'
+
+it "a footnote reference cannot carry a handler"
+out=$(md 'x[^f" onclick="alert(1)]
+
+[^f" onclick="alert(1)]: note')
+assert_not_contains "$out" '" onclick="alert(1)"'
+
+it "a footnote list entry cannot carry a handler"
+# The marker and the list entry are built separately; both take the key.
+out=$(md 'a[^k" onclick="alert(1)]
+
+[^k" onclick="alert(1)]: note')
+assert_not_contains "$out" 'id="fn-k" onclick='
+
+it "a fence language cannot carry a handler"
+out=$(md '```x" onload="alert(1)
+code
+```')
+assert_not_contains "$out" '" onload="alert(1)"'
+
+suite "markdown: urls that execute are dropped"
+
+it "a javascript: link is emptied"
+assert_not_contains "$(md '[x](javascript:alert(1))')" 'href="javascript:'
+
+it "case does not smuggle it past"
+assert_not_contains "$(md '[x](JaVaScRiPt:alert(1))')" 'alert(1)'
+
+it "leading whitespace does not smuggle it past"
+assert_not_contains "$(md '[x](  javascript:alert(1))')" 'alert(1)'
+
+it "a data: image is emptied"
+assert_not_contains "$(md '![y](data:text/html,xx)')" 'src="data:'
+
+it "vbscript: is dropped too"
+assert_not_contains "$(md '[z](vbscript:msgbox(1))')" 'href="vbscript:'
+
+it "a reference definition cannot hide one"
+assert_not_contains "$(md '[a][r]
+
+[r]: javascript:alert(1)')" 'href="javascript:'
+
+it "an ordinary link is untouched"
+assert_contains "$(md '[ok](https://example.com)')" 'href="https://example.com"'
+
+it "a relative link is untouched"
+assert_contains "$(md '[ok](/local/page)')" 'href="/local/page"'
+
 summary
+