diff --git a/pkg/sanitize/sanitize.go b/pkg/sanitize/sanitize.go index e6401e4fb3..550672b06e 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,14 +13,34 @@ var policy *bluemonday.Policy var policyOnce sync.Once func Sanitize(input string) string { - return 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 // in user-facing titles or bodies. This includes: // - Unicode tag characters: U+E0001, U+E0020–U+E007F // - BiDi control characters: U+202A–U+202E, U+2066–U+2069 -// - Hidden modifier characters: U+200B, U+200C, U+200E, U+200F, U+00AD, U+FEFF, U+180E, U+2060–U+2064 +// - 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 +// - 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 @@ -27,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) } @@ -179,6 +209,7 @@ func shouldRemoveRune(r rune) bool { 0x200C, // ZERO WIDTH NON-JOINER 0x200E, // LEFT-TO-RIGHT MARK 0x200F, // RIGHT-TO-LEFT MARK + 0x061C, // ARABIC LETTER MARK 0x00AD, // SOFT HYPHEN 0xFEFF, // ZERO WIDTH NO-BREAK SPACE 0x180E: // MONGOLIAN VOWEL SEPARATOR @@ -207,3 +238,40 @@ func shouldRemoveRune(r rune) bool { 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 35b23e6abe..dd128717ca 100644 --- a/pkg/sanitize/sanitize_test.go +++ b/pkg/sanitize/sanitize_test.go @@ -112,6 +112,56 @@ func TestFilterInvisibleCharacters(t *testing.T) { input: "This is a\u200B bug report.\n\nSteps to reproduce:\u200C\n1. Do this\u200E\n2. Do that\u200F", expected: "This is a bug report.\n\nSteps to reproduce:\n1. Do this\n2. Do that", }, + { + name: "text with arabic letter mark", + input: "Hello\u061CWorld", + expected: "HelloWorld", + }, + { + name: "orphaned variation selector after ascii letter", + input: "Hello\uFE0FWorld", + expected: "HelloWorld", + }, + { + name: "ideographic variation selector after non-ideograph base", + input: "Hello\U000E0100World", + expected: "HelloWorld", + }, + { + 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", + }, } for _, tt := range tests { @@ -166,6 +216,17 @@ func TestShouldRemoveRune(t *testing.T) { {name: "before hidden modifier range", rune: 0x205F, expected: false}, {name: "after hidden modifier range", rune: 0x2065, expected: false}, + // Additional directional mark + {name: "arabic letter mark", rune: 0x061C, expected: true}, + + // 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}, {name: "regular ascii digit", rune: '1', expected: false}, @@ -300,3 +361,171 @@ func TestSanitizeRemovesInvisibleCodeFenceMetadata(t *testing.T) { result := Sanitize(input) assert.Equal(t, expected, result) } + +// TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding covers the core +// regression from issue #3101: invisible/bidi characters encoded as HTML +// character entities are decoded by FilterHTMLTags, so the invisible-character +// policy must also run after HTML processing, not only on the raw input. +func TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "decimal entity for zero width space", + input: "Hello​World", + expected: "HelloWorld", + }, + { + name: "hexadecimal entity for zero width space", + input: "Hello​World", + expected: "HelloWorld", + }, + { + name: "hexadecimal entity for zero width space (lowercase hex digits)", + input: "Hello​World", + expected: "HelloWorld", + }, + { + name: "decimal entity for right-to-left override", + input: "Hello‮World", + expected: "HelloWorld", + }, + { + name: "hexadecimal entity for left-to-right override", + input: "Hello‭World", + expected: "HelloWorld", + }, + { + name: "decimal entity for orphaned variation selector", + input: "Hello️World", + expected: "HelloWorld", + }, + { + 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", + expected: "HelloWorld", + }, + { + name: "entity for ordinary ascii character is preserved", + input: "HelloAWorld", + expected: "HelloAWorld", + }, + { + name: "entity for benign unicode character is preserved", + input: "Hello世World", // 世 is 世 + expected: "Hello世World", + }, + { + name: "benign unicode text without entities is untouched", + 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 { + t.Run(tt.name, func(t *testing.T) { + result := Sanitize(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +// 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)) + }) + } +}