Commit ba20446 h

Pat Ambrosio · 26-09-05 10:46

Fold the probe after each decode, not only before the first

Changes: 2 files +57 -6

Changes (Side-by-Side)

Raw

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

    Fold the probe after each decode, not only before the first
    
    "%4Aavascript:alert(1)" survived and ran on click. Not because the
    decoder could not read uppercase hex -- the probe is lowercased before
    decoding, so it arrives as "%4a" and decodes fine. The problem is what it
    decodes to: "J". A letter that was not in the text when it was folded,
    now sitting in front of a check written in lowercase, in a scheme a
    browser reads case-insensitively.
    
    The fold follows the decode now, on every pass, so a letter revealed by
    decoding is folded before it is read. That is the fix: removing it fails
    four tests.
    
    The hex class is widened to accept A-F as well, which changes nothing
    today -- the caller folds first, so narrowing it back passes every test
    that goes through sanitize_url. It is still wrong for a decoder to read
    %6a and not %6A, independently of what its one caller happens to do, so
    there is a test that calls it directly rather than leaving the line
    unexercised.
    
    Unchanged, and checked: only the throwaway probe is decoded. An uppercase
    escape in a real path -- %2F, %3D, a UTF-8 sequence -- reaches the output
    byte for byte.

diff --git a/markdown.awk b/markdown.awk
index 1be3cf5..f001ec1 100755
--- a/markdown.awk
+++ b/markdown.awk
@@ -27,15 +27,21 @@
 # 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) {
+function pct_decode(t,   out, 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.
+	# the sixteen digits gives the nibble values, and index() is
+	# case-sensitive, so the digits are folded before the lookup rather than
+	# the table being widened.
+	#
+	# The pair is matched in either case even though the caller folds the
+	# whole string first: a decoder that only reads %6a and not %6A is wrong
+	# on its own terms, whatever its callers happen to do.
 	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
+	while (match(t, /%[0-9a-fA-F][0-9a-fA-F]/)) {
+		hi = index(HEX, tolower(substr(t, RSTART + 1, 1))) - 1
+		lo = index(HEX, tolower(substr(t, RSTART + 2, 1))) - 1
 		out = out substr(t, 1, RSTART - 1) sprintf("%c", hi * 16 + lo)
 		t = substr(t, RSTART + 3)
 	}
@@ -50,9 +56,16 @@ function sanitize_url(u,   probe, prev) {
 	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.
+	#
+	# Folded again after each pass, not only at the start. Decoding reveals
+	# bytes that were not in the text to fold the first time: "%4a" is
+	# lowercase already and decodes to "J", so "%4Aavascript:" became
+	# "Javascript:" -- past a check written in lowercase, and a scheme a
+	# browser reads case-insensitively. The fold has to follow the decode
+	# that produced the letter.
 	do {
 		prev = probe
-		probe = pct_decode(probe)
+		probe = tolower(pct_decode(probe))
 		gsub(/[\000-\040]/, "", probe)
 	} while (probe != prev)
 	if (probe ~ /^(javascript|data|vbscript|file):/)
diff --git a/test/units.sh b/test/units.sh
index 188013b..bbdd580 100755
--- a/test/units.sh
+++ b/test/units.sh
@@ -681,5 +681,43 @@ 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)'
 
+suite "markdown: decoding reveals letters that must be folded"
+
+# The probe was lowercased once, before decoding. Decoding then produced
+# letters that had never been folded -- "%4a" is lowercase already and decodes
+# to "J" -- so "%4Aavascript:" arrived at the check as "Javascript:" and did
+# not match a pattern written in lowercase. A browser reads a scheme
+# case-insensitively, so it ran. The fold has to follow the decode that
+# produced the letter, on every pass.
+
+it "an uppercase escape at the start is caught"
+assert_not_contains "$(md '[x](%4Aavascript:alert(1))')" '%4Aavascript'
+
+it "a mixed-case pair of escapes is caught"
+assert_not_contains "$(md '[x](%4a%61vascript:alert(1))')" 'alert(1)'
+
+it "an uppercase escape in the middle is caught"
+assert_not_contains "$(md '[x](jav%41script:alert(1))')" 'jav%41script'
+
+it "an uppercase escape in another scheme is caught"
+assert_not_contains "$(md '[x](%56%42script:alert(1))')" 'alert(1)'
+
+it "uppercase hex in a real path is untouched"
+# The decoder reads either case now, but it reads a throwaway copy. What is
+# emitted is what the author wrote, so an uppercase escape in a genuine path
+# survives byte for byte.
+assert_contains "$(md '[ok](https://example.com/A%2FB)')" 'href="https://example.com/A%2FB"'
+
+it "a utf-8 sequence in a path is untouched"
+assert_contains "$(md '[ok](https://example.com/%E2%9C%93)')" 'href="https://example.com/%E2%9C%93"'
+
+it "the decoder reads an uppercase pair on its own terms"
+# sanitize_url folds before decoding, so the decoder is never handed
+# uppercase hex in practice and narrowing this class back passes every other
+# test here. It is still wrong: a decoder that reads %6a and not %6A is
+# incorrect independently of what its one caller happens to do, and the next
+# caller may not fold first. Exercised directly for that reason.
+out=$(printf '%%4Aavascript\n' | "$AWK" -f "$REPO_ROOT/markdown.awk" -e 'BEGIN{ print pct_decode("%4Aavascript") }' 2>/dev/null </dev/null)
+if [ "$out" = "Javascript" ]; then pass; else fail "decoder gave [$out], wanted [Javascript]"; fi
 summary