diff --git a/pkg/sanitize/sanitize.go b/pkg/sanitize/sanitize.go index a634839a..550672b0 100644 --- a/pkg/sanitize/sanitize.go +++ b/pkg/sanitize/sanitize.go @@ -4,6 +4,7 @@ import ( "strings" "sync" "unicode" + "unicode/utf8" "github.com/microcosm-cc/bluemonday" ) @@ -12,15 +13,17 @@ var policy *bluemonday.Policy var policyOnce sync.Once func Sanitize(input string) string { - // FilterInvisibleCharacters runs both before and after HTML processing. - // The first pass strips raw invisible characters so they don't interfere - // with code-fence parsing. HTML sanitization (FilterHTMLTags) decodes - // character entities (e.g. "​" or "​" become U+200B), which - // can introduce invisible or bidirectional characters that were not - // present as literal runes in the original input. The second pass - // filters the fully normalized output so entity-encoded characters - // cannot survive the policy. - return FilterInvisibleCharacters(FilterHTMLTags(FilterCodeFenceMetadata(FilterInvisibleCharacters(input)))) + // The invisible-character and code-fence filters both run before and after + // HTML processing. The first pass strips raw invisible characters so they + // don't interfere with code-fence parsing. HTML sanitization + // (FilterHTMLTags) decodes character entities (e.g. "​" or + // "​" become U+200B), which can introduce invisible or + // bidirectional characters that were not present as literal runes in the + // original input. Those decoded characters can both survive on their own + // and splice previously inert text into a code fence, so the second pass + // re-applies both filters to the fully normalized output. + normalized := FilterHTMLTags(FilterCodeFenceMetadata(FilterInvisibleCharacters(input))) + return FilterCodeFenceMetadata(FilterInvisibleCharacters(normalized)) } // FilterInvisibleCharacters removes invisible or control characters that should not appear @@ -29,7 +32,15 @@ func Sanitize(input string) string { // - BiDi control characters: U+202A–U+202E, U+2066–U+2069 // - BiDi/directional marks: U+200E, U+200F, U+061C // - Hidden modifier characters: U+200B, U+200C, U+00AD, U+FEFF, U+180E, U+2060–U+2064 -// - Variation selectors: U+FE00–U+FE0F, U+E0100–U+E01EF +// - Orphaned variation selectors: U+FE00–U+FE0F, U+E0100–U+E01EF +// +// Variation selectors are filtered contextually rather than unconditionally. +// A selector that forms a plausible variation sequence with the character it +// follows is preserved, so ordinary content such as "✈️", "1️⃣" and CJK +// ideographic variation sequences survive unchanged. Selectors that cannot +// belong to such a sequence — those at the start of the input, those following +// a removed or non-graphic character, and runs of consecutive selectors — are +// removed, which is the shape used to smuggle hidden payloads. func FilterInvisibleCharacters(input string) string { if input == "" { return input @@ -37,10 +48,19 @@ func FilterInvisibleCharacters(input string) string { // Filter runes out := make([]rune, 0, len(input)) + var prev rune + var prevKept bool for _, r := range input { - if !shouldRemoveRune(r) { + keep := false + if isVariationSelector(r) { + keep = prevKept && isValidVariationSequence(prev, r) + } else { + keep = !shouldRemoveRune(r) + } + if keep { out = append(out, r) } + prev, prevKept = r, keep } return string(out) } @@ -215,14 +235,43 @@ func shouldRemoveRune(r rune) bool { if r >= 0x2060 && r <= 0x2064 { return true } - // Variation selectors: U+FE00–U+FE0F - if r >= 0xFE00 && r <= 0xFE0F { - return true - } - // Variation selectors supplement: U+E0100–U+E01EF - if r >= 0xE0100 && r <= 0xE01EF { - return true - } return false } + +// isVariationSelector reports whether r is a Unicode variation selector, either +// from the Variation Selectors block (VS1–VS16) or the Variation Selectors +// Supplement (VS17–VS256). +func isVariationSelector(r rune) bool { + return (r >= 0xFE00 && r <= 0xFE0F) || (r >= 0xE0100 && r <= 0xE01EF) +} + +// isValidVariationSequence reports whether selector can legitimately apply to +// the base character it immediately follows. +// +// A base may carry at most one selector, so a selector following another +// selector is always rejected; consecutive selectors carry no rendering meaning +// and are the primary way arbitrary data is hidden in text. +func isValidVariationSequence(base, selector rune) bool { + if isVariationSelector(base) || !unicode.IsGraphic(base) || unicode.IsSpace(base) { + return false + } + + // The Ideographic Variation Database only registers sequences whose base is + // a CJK ideograph, so supplement selectors are meaningless elsewhere. + if selector >= 0xE0100 { + return unicode.Is(unicode.Han, base) + } + + // Standardized variation sequences use non-ASCII bases, except for the + // keycap bases '#', '*' and the ASCII digits, which take a presentation + // selector (VS15/VS16) only. + if base < utf8.RuneSelf { + if base != '#' && base != '*' && (base < '0' || base > '9') { + return false + } + return selector == 0xFE0E || selector == 0xFE0F + } + + return true +} diff --git a/pkg/sanitize/sanitize_test.go b/pkg/sanitize/sanitize_test.go index 166d132d..dd128717 100644 --- a/pkg/sanitize/sanitize_test.go +++ b/pkg/sanitize/sanitize_test.go @@ -118,19 +118,49 @@ func TestFilterInvisibleCharacters(t *testing.T) { expected: "HelloWorld", }, { - name: "text with variation selector", + name: "orphaned variation selector after ascii letter", input: "Hello\uFE0FWorld", expected: "HelloWorld", }, { - name: "text with variation selector supplement", + name: "ideographic variation selector after non-ideograph base", input: "Hello\U000E0100World", expected: "HelloWorld", }, { - name: "emoji variation selector hidden after emoji (steganography)", - input: "\U0001F600\uFE0F\U000E0101Hi", - expected: "\U0001F600Hi", + name: "variation selector at start of input has no base", + input: "\uFE0FHello", + expected: "Hello", + }, + { + name: "variation selector orphaned by removed zero width space", + input: "\u2708\u200B\uFE0F", + expected: "\u2708", + }, + { + name: "smuggled selector run after emoji keeps only the presentation selector", + input: "\U0001F600\uFE0F\U000E0101\U000E0102Hi", + expected: "\U0001F600\uFE0FHi", + }, + { + name: "emoji presentation sequence is preserved", + input: "Book a flight \u2708\uFE0F today", + expected: "Book a flight \u2708\uFE0F today", + }, + { + name: "text presentation sequence is preserved", + input: "Book a flight \u2708\uFE0E today", + expected: "Book a flight \u2708\uFE0E today", + }, + { + name: "keycap sequence is preserved", + input: "Step 1\uFE0F\u20E3 first", + expected: "Step 1\uFE0F\u20E3 first", + }, + { + name: "registered cjk ideographic variation sequence is preserved", + input: "\u845B\U000E0100\u57CE", + expected: "\u845B\U000E0100\u57CE", }, } @@ -189,19 +219,13 @@ func TestShouldRemoveRune(t *testing.T) { // Additional directional mark {name: "arabic letter mark", rune: 0x061C, expected: true}, - // Range tests - Variation selectors: U+FE00–U+FE0F - {name: "variation selector range start", rune: 0xFE00, expected: true}, - {name: "variation selector range middle", rune: 0xFE05, expected: true}, - {name: "variation selector range end (VS16, emoji presentation)", rune: 0xFE0F, expected: true}, - {name: "before variation selector range", rune: 0xFDFF, expected: false}, - {name: "after variation selector range", rune: 0xFE10, expected: false}, - - // Range tests - Variation selectors supplement: U+E0100–U+E01EF - {name: "variation selector supplement range start", rune: 0xE0100, expected: true}, - {name: "variation selector supplement range middle", rune: 0xE0150, expected: true}, - {name: "variation selector supplement range end", rune: 0xE01EF, expected: true}, - {name: "before variation selector supplement range", rune: 0xE00FF, expected: false}, - {name: "after variation selector supplement range", rune: 0xE01F0, expected: false}, + // Variation selectors are filtered contextually by + // FilterInvisibleCharacters, so shouldRemoveRune never removes them on + // its own. See TestIsValidVariationSequence for that behaviour. + {name: "variation selector range start", rune: 0xFE00, expected: false}, + {name: "variation selector range end (VS16, emoji presentation)", rune: 0xFE0F, expected: false}, + {name: "variation selector supplement range start", rune: 0xE0100, expected: false}, + {name: "variation selector supplement range end", rune: 0xE01EF, expected: false}, // Characters that should NOT be removed {name: "regular ascii letter", rune: 'A', expected: false}, @@ -359,7 +383,7 @@ func TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding(t *testing.T) { expected: "HelloWorld", }, { - name: "hexadecimal entity for zero width space (lowercase x, uppercase hex)", + name: "hexadecimal entity for zero width space (lowercase hex digits)", input: "Hello​World", expected: "HelloWorld", }, @@ -374,15 +398,20 @@ func TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding(t *testing.T) { expected: "HelloWorld", }, { - name: "decimal entity for variation selector", + name: "decimal entity for orphaned variation selector", input: "Hello️World", expected: "HelloWorld", }, { - name: "hexadecimal entity for variation selector supplement", + name: "hexadecimal entity for orphaned variation selector supplement", input: "Hello󠄀World", expected: "HelloWorld", }, + { + name: "entity encoded selector run after emoji is truncated to one selector", + input: "Ship it \U0001F600️󠄁󠄂", + expected: "Ship it \U0001F600\uFE0F", + }, { name: "direct invisible rune alongside entity encoded one", input: "Hello\u200B‎World", @@ -403,6 +432,16 @@ func TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding(t *testing.T) { input: "Hello 世界 🌍 αβγ", expected: "Hello 世界 🌍 αβγ", }, + { + name: "emoji presentation sequence survives the full pipeline", + input: "Book a flight \u2708\uFE0F today", + expected: "Book a flight \u2708\uFE0F today", + }, + { + name: "registered cjk ideographic variation sequence survives the full pipeline", + input: "\u845B\U000E0100\u57CE", + expected: "\u845B\U000E0100\u57CE", + }, } for _, tt := range tests { @@ -412,3 +451,81 @@ func TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding(t *testing.T) { }) } } + +// TestSanitizeRemovesCodeFenceMetadataRevealedByEntityDecoding covers fences +// that only become fences after HTML entity decoding. A leading "`​“" +// is not a fence in the raw input, so the first FilterCodeFenceMetadata pass +// leaves it alone; once the entity is decoded and the zero width space is +// removed the line is a real fence, so the fence filter has to run again. +func TestSanitizeRemovesCodeFenceMetadataRevealedByEntityDecoding(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "decimal entity hides fence delimiter", + input: "`​``steal secrets\nfmt.Println(42)\n```", + expected: "```\nfmt.Println(42)\n```", + }, + { + name: "hexadecimal entity hides fence delimiter", + input: "``​`steal secrets\nfmt.Println(42)\n```", + expected: "```\nfmt.Println(42)\n```", + }, + { + name: "entity hides fence delimiter with disallowed info string", + input: "`​``go;rm -rf /\ncode\n```", + expected: "```\ncode\n```", + }, + { + name: "entity encoded fence keeps a safe info string", + input: "`​``go\nfmt.Println(42)\n```", + expected: "```go\nfmt.Println(42)\n```", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := Sanitize(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestIsValidVariationSequence(t *testing.T) { + tests := []struct { + name string + base rune + selector rune + expected bool + }{ + {name: "emoji presentation selector after symbol", base: 0x2708, selector: 0xFE0F, expected: true}, + {name: "text presentation selector after symbol", base: 0x2708, selector: 0xFE0E, expected: true}, + {name: "presentation selector after emoji", base: 0x1F600, selector: 0xFE0F, expected: true}, + {name: "presentation selector after keycap digit", base: '1', selector: 0xFE0F, expected: true}, + {name: "presentation selector after keycap hash", base: '#', selector: 0xFE0F, expected: true}, + {name: "presentation selector after keycap asterisk", base: '*', selector: 0xFE0E, expected: true}, + {name: "non-presentation selector after keycap digit", base: '1', selector: 0xFE00, expected: false}, + {name: "presentation selector after ascii letter", base: 'a', selector: 0xFE0F, expected: false}, + {name: "presentation selector after ascii punctuation", base: '.', selector: 0xFE0F, expected: false}, + {name: "standardized selector after cjk ideograph", base: '葛', selector: 0xFE00, expected: true}, + + {name: "ideographic selector after cjk ideograph", base: '葛', selector: 0xE0100, expected: true}, + {name: "ideographic selector after cjk compatibility ideograph", base: 0xF900, selector: 0xE0101, expected: true}, + {name: "ideographic selector after emoji", base: 0x1F600, selector: 0xE0100, expected: false}, + {name: "ideographic selector after ascii letter", base: 'a', selector: 0xE0100, expected: false}, + {name: "ideographic selector after greek letter", base: 'α', selector: 0xE0100, expected: false}, + + {name: "selector after another selector", base: 0xFE0F, selector: 0xFE0F, expected: false}, + {name: "ideographic selector after another selector", base: 0xE0100, selector: 0xE0101, expected: false}, + {name: "selector after space", base: ' ', selector: 0xFE0F, expected: false}, + {name: "selector after newline", base: '\n', selector: 0xFE0F, expected: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, isValidVariationSequence(tt.base, tt.selector)) + }) + } +}