From cde2427e0a4519c76551ee3b68685fce2ca858c0 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 19 Aug 2026 14:17:16 +0200 Subject: [PATCH 1/2] Filter invisible Unicode after HTML entity normalization FilterInvisibleCharacters previously ran only before FilterHTMLTags, so numeric HTML entities (e.g. ​ or ​) that bluemonday decodes into invisible or bidirectional control characters could survive sanitization untouched. Sanitize now applies the invisible-character filter both before HTML processing (so raw invisible characters don't interfere with code-fence parsing) and again after, so entity-decoded characters cannot escape the policy. Also expands the removal set to include: - ARABIC LETTER MARK (U+061C), a directional format character in the same family as the already-covered LRM/RLM marks. - Variation selectors (U+FE00-U+FE0F) and the variation selectors supplement (U+E0100-U+E01EF), which can be used to hide payloads after emoji or other base characters. Fixes #3101 --- pkg/sanitize/sanitize.go | 23 ++++++- pkg/sanitize/sanitize_test.go | 112 ++++++++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/pkg/sanitize/sanitize.go b/pkg/sanitize/sanitize.go index e6401e4fb3..a634839a84 100644 --- a/pkg/sanitize/sanitize.go +++ b/pkg/sanitize/sanitize.go @@ -12,14 +12,24 @@ var policy *bluemonday.Policy var policyOnce sync.Once func Sanitize(input string) string { - return FilterHTMLTags(FilterCodeFenceMetadata(FilterInvisibleCharacters(input))) + // 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)))) } // 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 +// - Variation selectors: U+FE00–U+FE0F, U+E0100–U+E01EF func FilterInvisibleCharacters(input string) string { if input == "" { return input @@ -179,6 +189,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 @@ -204,6 +215,14 @@ 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 } diff --git a/pkg/sanitize/sanitize_test.go b/pkg/sanitize/sanitize_test.go index 35b23e6abe..166d132d33 100644 --- a/pkg/sanitize/sanitize_test.go +++ b/pkg/sanitize/sanitize_test.go @@ -112,6 +112,26 @@ 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: "text with variation selector", + input: "Hello\uFE0FWorld", + expected: "HelloWorld", + }, + { + name: "text with variation selector supplement", + input: "Hello\U000E0100World", + expected: "HelloWorld", + }, + { + name: "emoji variation selector hidden after emoji (steganography)", + input: "\U0001F600\uFE0F\U000E0101Hi", + expected: "\U0001F600Hi", + }, } for _, tt := range tests { @@ -166,6 +186,23 @@ 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}, + + // 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}, + // Characters that should NOT be removed {name: "regular ascii letter", rune: 'A', expected: false}, {name: "regular ascii digit", rune: '1', expected: false}, @@ -300,3 +337,78 @@ 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 x, uppercase hex)", + 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 variation selector", + input: "Hello️World", + expected: "HelloWorld", + }, + { + name: "hexadecimal entity for variation selector supplement", + input: "Hello󠄀World", + expected: "HelloWorld", + }, + { + 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 世界 🌍 αβγ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := Sanitize(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} From 294ba9b42857c0f58405416172742d236a8e82f1 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Wed, 19 Aug 2026 14:48:01 +0200 Subject: [PATCH 2/2] Re-run fence filter and preserve valid variation sequences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on the post-HTML-entity sanitization pass. Entity decoding could still smuggle code-fence metadata past the sanitizer. A first line such as "`​``steal secrets" is not a fence in the raw input, so FilterCodeFenceMetadata left it alone; decoding the entity and stripping the zero width space then produced a real fence with its info string intact. Sanitize now re-runs the fence filter after the input is fully normalized. Filtering every variation selector also corrupted legitimate text: VS15 and VS16 select text or emoji presentation, so "✈️" was reduced to "✈", and the Variation Selectors Supplement encodes registered CJK ideographic variation sequences. Selectors are now filtered contextually. A selector is kept when it can apply to the character it follows, and dropped when it is orphaned, follows a removed or non-graphic character, or continues a run of selectors. Supplement selectors additionally require a CJK ideograph base, matching the Ideographic Variation Database. That keeps the anti-smuggling property, since hidden payloads rely on selector runs, without rewriting valid Unicode. Also corrects a lowercase-hex test case that claimed uppercase digits. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- pkg/sanitize/sanitize.go | 85 ++++++++++++++---- pkg/sanitize/sanitize_test.go | 159 +++++++++++++++++++++++++++++----- 2 files changed, 205 insertions(+), 39 deletions(-) diff --git a/pkg/sanitize/sanitize.go b/pkg/sanitize/sanitize.go index a634839a84..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,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 + + 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 } - // Variation selectors supplement: U+E0100–U+E01EF - if r >= 0xE0100 && r <= 0xE01EF { - return true + + // 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) } - return false + // 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 166d132d33..dd128717ca 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,57 @@ 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 { + 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 { @@ -412,3 +492,40 @@ func TestSanitizeFiltersInvisibleCharactersAfterEntityDecoding(t *testing.T) { }) } } + +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)) + }) + } +}