Commit 9700190 h

Pat Ambrosio · 26-09-05 10:29

Decode percent escapes before deciding what scheme a URL holds

Changes: 2 files +95 -9

Changes (Side-by-Side)

Raw

commit 970019084b963d45fed337993aa819830f649602
Author: Pat Ambrosio <cp.ambrosio@gmail.com>
Date:   Sat Sep 5 10:29:28 2026 +0800

    Decode percent escapes before deciding what scheme a URL holds
    
    A browser percent-decodes an href before it works out which scheme the URL
    names, so a check that reads the raw text is reading something the browser
    never sees. Every one of these was emitted verbatim and ran on click:
    
        [x](%6a%61%76%61%73%63%72%69%70%74:alert(1))
        [x](%6aavascript:alert(1))
        [x](%256a%61%76%61%73%63%72%69%70%74:alert(1))
    
    The probe is decoded until it stops changing, so a percent sign that is
    itself encoded is decoded on the second pass rather than left half done.
    
    Only the probe. The URL that is emitted is the one the author wrote, so a
    space encoded in a path is still encoded when it reaches the browser --
    %20 in a real link must survive, and it is the throwaway copy that gets
    decoded, not the output.
    
    The decode is written with an index() lookup rather than strtonum(): that
    is a gawk extension, and jenny falls back to whatever awk is on the
    machine.
    
    Also fixes a table or a fenced block written after a blockquote being
    swallowed into it. Those rules end with next, which returns before the
    loop that closes open blocks, so the paragraph closed and the quote did
    not. They close it themselves now. A fence inside a quote is still not
    supported -- that is older than this and unchanged.

diff --git a/markdown.awk b/markdown.awk
index 692da99..1be3cf5 100755
--- a/markdown.awk
+++ b/markdown.awk
@@ -15,23 +15,67 @@
 # 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) {
+# The check is made on a throwaway copy, never on the URL that is emitted.
+# The copy is lowercased, stripped of leading control characters and
+# whitespace, has the HTML entities that could hide a colon decoded, and is
+# percent-decoded until no escape remains. A browser decodes an href before it
+# decides what scheme it holds, so the check has to read what the browser will
+# read: "%6a%61%76%61%73%63%72%69%70%74:", "%6aavascript:" and the
+# double-encoded "%256a..." are all javascript: written to slip past a test
+# that reads the raw text.
+#
+# Only the copy is decoded. A legitimate %20 in a path must survive into the
+# output untouched, and it does, because the URL that is returned is the one
+# that came in -- once a scheme is rejected the whole URL is dropped anyway.
+function pct_decode(t,   out, hex, lo, hi, HEX) {
+	# One pass of %xx -> the byte it names, without strtonum: that is a gawk
+	# extension and this has to run under any awk. index() into a table of
+	# the sixteen digits gives the nibble values.
+	HEX = "0123456789abcdef"
+	out = ""
+	while (match(t, /%[0-9a-f][0-9a-f]/)) {
+		hi = index(HEX, substr(t, RSTART + 1, 1)) - 1
+		lo = index(HEX, substr(t, RSTART + 2, 1)) - 1
+		out = out substr(t, 1, RSTART - 1) sprintf("%c", hi * 16 + lo)
+		t = substr(t, RSTART + 3)
+	}
+	return out t
+}
+
+function sanitize_url(u,   probe, prev) {
 	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*58;|&colon;/, ":", probe)
 	gsub(/&#0*x?[0-9a-f]+;?/, "", probe)
+	# Until it stops changing, so "%256a" -- a percent sign that is itself
+	# encoded -- is decoded the second time round rather than left half done.
+	do {
+		prev = probe
+		probe = pct_decode(probe)
+		gsub(/[\000-\040]/, "", probe)
+	} while (probe != prev)
 	if (probe ~ /^(javascript|data|vbscript|file):/)
 		return ""
 	return u
 }
 
+# close_quotes
+# Emits the closing tag for any blockquote still open, and forgets it.
+#
+# The rules that build a fence or a table finish with next, which returns
+# before the loop further down that closes open blocks -- so a fence or a
+# table written after a blockquote was swallowed into it: the paragraph
+# closed, the quote did not, and the block rendered inside the quote it should
+# have followed. Those rules close it themselves, here, rather than leaving a
+# level set for a loop they never reach.
+function close_quotes(   i) {
+	for(i = nl; i > 0 && block[i] == "blockquote"; i--)
+		oprint("</blockquote>");
+	if(i < nl)
+		nl = i;
+}
+
 function escattr(t) {
 	t = eschtml(t);
 	gsub(">", "\\&gt;", t);
@@ -721,6 +765,7 @@ tbl_pending != "" {
 			tbl_align[ai] = table_align(align_spec[ai]);
 		if(text)
 			printp(par ? par : "p");
+		close_quotes();
 		oprint("<div class=\"table-scroll\">");
 		oprint("<table>");
 		oprint("<thead>");
@@ -811,13 +856,15 @@ fence {
 	sub(/^[ \t]+/, "", info);
 	sub(/[ \t].*$/, "", info);
 
-	# Close anything currently open so the block is not nested inside a <p>.
+	# Close anything currently open so the block is not nested inside a <p>,
+	# or inside a blockquote it merely follows.
 	if(text)
 		printp(par ? par : "p");
 	if(code){
 		oprint(code_held "</code></pre>"); code_held = "";
 		code = 0;
 	}
+	close_quotes();
 
 	fence = 1;
 	fence_first = 1;
diff --git a/test/units.sh b/test/units.sh
index 539d164..188013b 100755
--- a/test/units.sh
+++ b/test/units.sh
@@ -642,5 +642,44 @@ 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"'
 
+suite "markdown: percent-encoded schemes are decoded before the check"
+
+# A browser percent-decodes an href before it decides what scheme the URL
+# holds, so a check that reads the raw text reads something the browser never
+# sees. Every one of these was emitted verbatim and ran on click.
+
+it "a fully encoded scheme is caught"
+assert_not_contains "$(md '[x](%6a%61%76%61%73%63%72%69%70%74:alert(1))')" '%6a%61%76%61'
+
+it "a partly encoded scheme is caught"
+assert_not_contains "$(md '[x](%6aavascript:alert(1))')" '%6aavascript'
+
+it "a double-encoded scheme is caught"
+# %256a is a percent sign that is itself encoded: one pass leaves %6a, so the
+# decode has to run until the string stops changing.
+assert_not_contains "$(md '[x](%256a%61%76%61%73%63%72%69%70%74:alert(1))')" 'alert(1)'
+
+it "an encoded data: image is caught"
+assert_not_contains "$(md '![y](%64%61%74%61:text/html,x)')" '%64%61%74%61'
+
+it "a reference definition cannot hide an encoded scheme"
+assert_not_contains "$(md '[a][r]
+
+[r]: %6a%61%76%61%73%63%72%69%70%74:alert(1)')" 'alert(1)'
+
+it "a legitimate percent escape survives into the output"
+# The decode is done on a throwaway copy read only by the scheme test. The URL
+# that is emitted is the one the author wrote, so a space encoded in a path is
+# still encoded when it gets to the browser.
+assert_contains "$(md '[ok](https://example.com/a%20b)')" 'href="https://example.com/a%20b"'
+
+it "a percent escape in an image path survives too"
+assert_contains "$(md '![i](/i/a%20b.png)')" 'src="/i/a%20b.png"'
+
+it "the earlier vectors are still blocked"
+assert_not_contains "$(md '[x](javascript:alert(1))')" 'href="javascript:'
+assert_not_contains "$(md '[x](JaVaScRiPt:alert(1))')" 'alert(1)'
+assert_not_contains "$(md '[x](  javascript:alert(1))')" 'alert(1)'
+
 summary