From fb9ed2a1b7c91356849dd04ecd9bd4d39532c23c Mon Sep 17 00:00:00 2001 From: Richard Quinn Date: Wed, 26 Aug 2026 23:36:09 +0200 Subject: [PATCH 1/2] fix: close the two CVE rows behind "no single call" (#614, #615) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They had to land together. Each was one of the exactly two vectors that made docs/security/cve-validation.md say no entry point cleared everything, and tests/test_cve_vectors.py:2121 was written to fail loudly when either closed. Fixing one alone would have left the other failing and forced the guidance to be rewritten twice. #614 — strip_obfuscation named confusables instead of folding them. 49 code points appear in both emoji_single.tsv and confusables_to_latin.tsv, and most are not emoji: typographic punctuation, currency, math operators, CJK brackets, reaching the emoji table from CLDR annotationsDerived. strip_obfuscation("€xample.com") produced "euro xample.com", so the spoof and the genuine host stopped being equal rather than becoming equal — CVE-2017-5383 surviving a preset documented as maximum-strength deobfuscation. NOT the fix the issue proposed. Reordering the confusable fold before demojize would break idempotency: punctuation inside emoji NAMES (the apostrophe in "woman's hat") has to be folded by the confusable pass. That is documented three times and pinned by tests/test_presets.py. Instead the overlap is DERIVED at build time as an intersection of the two tables, so it cannot drift the way a curated list would, and demojize skips those rows inside comparison presets only. Standalone demojize is unchanged. build.rs asserts the count is 49, so a table refresh that claims another confusable source fails the build rather than widening the gap silently. #615 — canonicalize cannot cap its way out of an eclipsing mark. The anti-zalgo step is a COUNT, and by count one Arabic shadda is indistinguishable from one acute accent, so no threshold removes CVE-2017-7833's spoof and keeps café. The discriminator was already in disarm's script data: strip a combining mark whose own Script is a specific script differing from its base's; keep Inherited marks, which attach to anything. UTS #39's mixed-script reasoning applied per grapheme rather than per string. canonicalize_strict ONLY. The rule is destructive for scholarly transliteration, IPA and linguistic transcription, where marks from one script legitimately sit on bases of another — the corpus least able to notice. canonicalize stays one short deliberately, asserted by a test rather than left implied. Nine legitimate samples in five scripts, including Arabic WITH its own vowel marks, pass through completely unchanged. The step sits AFTER the confusable fold, and that ordering is load-bearing. Placed before it, `а` (Cyrillic) + U+0489 (Cyrillic mark) agrees on the first pass, then the fold rewrites the base to Latin `a` and the next pass strips the mark — f(f(x)) != f(x). canonicalize_strict_idempotent caught it. Deciding against the FINAL base script is the only stable point. canonicalize_strict and strip_obfuscation now each clear the whole matrix, so TestOneCall is inverted rather than deleted: it now asserts the two sufficient entry points stay sufficient and every other one stays short. The published advice is unchanged; its reason moved from "nothing suffices" to "the two that suffice are the two most destructive ones", which is the same conclusion for a caller who has to forward the text they cleaned. Signed-off-by: Richard Quinn Assisted-by: Claude Code:claude-opus-5 --- CHANGELOG.md | 53 +++++++++++++ build.rs | 52 +++++++++++- docs/security/cve-validation.md | 60 +++++++------- proptest-regressions/presets.txt | 1 + src/emoji.rs | 33 +++++++- src/pipeline.rs | 2 +- src/presets.rs | 132 +++++++++++++++++++++++++++++-- src/tables/emoji_data.rs | 1 + src/tables/mod.rs | 10 +++ src/zalgo.rs | 57 +++++++++++++ tests/test_cve_vectors.py | 121 ++++++++++++++++++++++------ 11 files changed, 455 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02506f88..5b9a2ebc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -493,6 +493,59 @@ compatibility (see [RELEASING.md](RELEASING.md)). ### Fixed +- **The two CVE rows behind "no single call" are closed, and they had to close together + (#614, #615).** Each was one of the exactly two vectors that made + `docs/security/cve-validation.md` say no entry point cleared everything, so fixing one + alone would have left the other failing and forced the guidance to be rewritten twice. + + **#614 — `strip_obfuscation` named confusables instead of folding them.** 49 code + points appear in both `emoji_single.tsv` and `confusables_to_latin.tsv`, and most are + not emoji: typographic punctuation, currency, math operators, CJK brackets. They reach + the emoji table from CLDR `annotationsDerived`, which names non-emoji characters. + `strip_obfuscation("€xample.com")` produced `"euro xample.com"`, so the spoof and the + genuine host stopped being equal rather than becoming equal — CVE-2017-5383 surviving a + preset documented as maximum-strength deobfuscation. + + **Not fixed the way the issue proposed.** Reordering the confusable fold before + `demojize` would break idempotency: punctuation inside emoji *names* (the `’` in + "woman’s hat") has to be folded by the confusable pass. That ordering is documented + three times and pinned by `tests/test_presets.py`. Instead the overlap is derived at + build time as an intersection of the two tables — so it cannot drift the way a curated + list would — and `demojize` skips those rows inside comparison presets only. Standalone + `demojize("I ❤ €5")` still returns `"I red heart euro 5"`, which is what that function + is for. `build.rs` asserts the count is 49, so a table refresh that claims another + confusable source fails the build instead of widening the gap silently. + + **#615 — `canonicalize` cannot cap its way out of an eclipsing mark.** The anti-zalgo + step is a *count*, and by count one Arabic shadda is indistinguishable from one acute + accent, so no threshold removes CVE-2017-7833's spoof and keeps `café`. The + discriminator that works was already in disarm's script data: strip a combining mark + whose own Script is a *specific* script differing from its base's, and keep `Inherited` + marks, which attach to anything. That is UTS #39's mixed-script reasoning applied per + grapheme rather than per string. + + It runs in `canonicalize_strict` **only**. The rule is destructive for scholarly + transliteration, IPA and linguistic transcription, where marks from one script + legitimately sit on bases of another — the corpus least able to notice. `canonicalize` + is deliberately still one short, and there is a test asserting that rather than leaving + it implied. Verified against nine legitimate samples in five scripts, including Arabic + *with* its own vowel marks: all pass through completely unchanged. + + The step sits **after** the confusable fold, and that ordering is load-bearing. Placed + before it, `а` (Cyrillic) + `U+0489` (Cyrillic mark) agrees on the first pass, then the + fold rewrites the base to Latin `a` and the next pass strips the mark — `f(f(x)) != + f(x)`. The property test `canonicalize_strict_idempotent` caught it; deciding against + the *final* base script is the only stable point. + + `canonicalize_strict` and `strip_obfuscation` now each clear the whole matrix, so + `TestOneCall`'s guard is inverted rather than deleted: it asserted that closing a gap + should fail loudly, it did, and it now asserts the two sufficient entry points stay + sufficient while every other one stays short. The published advice is unchanged and its + reason has moved — from "nothing suffices" to "the two that suffice are the two most + destructive ones", which is the same conclusion for a caller who has to forward the + text they cleaned. + + - **`is_suspicious_hostname()` now catches tags, variation selectors, noncharacters and PUA, and stops reporting a noncharacter as Arabic (#610).** Third in the sequence after #603 (bidi controls) and #605 (zero-width). 17 of 18 sampled diff --git a/build.rs b/build.rs index 04f6dcd2..29abd651 100644 --- a/build.rs +++ b/build.rs @@ -9,7 +9,7 @@ //! - str→str maps: `key\tvalue` //! - char sets: `HEXCODEPOINT` -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::env; use std::fmt::Write as _; use std::fs; @@ -288,6 +288,56 @@ fn main() { "EMOJI_SINGLE", "pub", ); + // --- Emoji rows the TR39 confusable table also claims (#614) --- + // 49 code points appear in BOTH emoji_single.tsv and confusables_to_latin.tsv, and + // they are mostly not emoji at all: typographic punctuation (`2010 hyphen`, the + // apostrophes and quotes, `2026 ellipsis`), currency (`20AC euro`), math operators + // and the CJK tortoise-shell brackets. They reach the emoji table from CLDR + // `annotationsDerived`, which names non-emoji characters. + // + // Both entries are legitimate. `demojize("I ❤ €5")` -> "I red heart euro 5" is + // exactly what that function is for. What is wrong is which table wins inside a + // COMPARISON preset: `strip_obfuscation("€xample.com")` named the euro sign instead + // of folding it, so the spoof and the genuine string stopped being equal rather than + // becoming equal, and CVE-2017-5383 survived a preset documented as maximum-strength + // deobfuscation. + // + // Derived as an INTERSECTION rather than read from a curated file, so it cannot drift + // out of date the way a hand-written override list would. The count is asserted: a + // future emoji-table refresh that claims another confusable source fails the build + // with this message instead of silently widening the gap. + { + let emoji = read_char_str_tsv(&data_dir.join("emoji_single.tsv")); + let confusable = read_char_str_tsv(&data_dir.join("confusables_to_latin.tsv")); + let overlap: BTreeSet = emoji + .keys() + .filter(|cp| confusable.contains_key(cp)) + .copied() + .collect(); + assert_eq!( + overlap.len(), + 49, + "emoji_single.tsv ∩ confusables_to_latin.tsv changed: expected the 49 rows \ + reviewed in #614, found {}. A new row means a confusable source is now \ + named instead of folded inside strip_obfuscation. Review it, then update \ + this count.", + overlap.len() + ); + let mut code = String::from( + "/// Code points claimed by BOTH the emoji name table and the TR39 confusable\n\ + /// table (#614). Skipped by `demojize` inside comparison presets so the fold\n\ + /// wins; standalone `demojize` still names them.\n\ + pub(crate) static EMOJI_ROWS_TR39_ALSO_CLAIMS: phf::Set = ", + ); + let mut set = phf_codegen::Set::new(); + for cp in &overlap { + set.entry(*cp); + } + code.push_str(&set.build().to_string()); + code.push_str(";\n"); + fs::write(out_dir.join("emoji_tr39_overlap_phf.rs"), code).unwrap(); + } + // Production matcher (#242 item 4): compact code-point trie. generate_emoji_trie( &data_dir.join("emoji_multi.tsv"), diff --git a/docs/security/cve-validation.md b/docs/security/cve-validation.md index d41819b3..11443dd7 100644 --- a/docs/security/cve-validation.md +++ b/docs/security/cve-validation.md @@ -106,7 +106,7 @@ so the claim passed its own gate. CVE-2017-7833 is the vector that breaks it — a single Arabic vowel mark riding a Latin letter: ```python -from disarm import canonicalize, strip_obfuscation, strip_zalgo, is_zalgo, catalog_key +from disarm import canonicalize, canonicalize_strict, strip_obfuscation, is_zalgo eclipsed = "exaّmple.com" # U+0651 ARABIC SHADDA over the "a" @@ -116,43 +116,41 @@ assert is_zalgo(eclipsed) is False assert canonicalize(eclipsed) != canonicalize("example.com") ``` -The mirror image is just as real. `strip_obfuscation` removes the mark but -*names* punctuation confusables instead of folding them, so U+2010 HYPHEN comes -out as the word "hyphen" and never collapses onto ASCII `-`: +The mirror image was just as real. `strip_obfuscation` removed the mark but +*named* punctuation confusables instead of folding them, so U+2010 HYPHEN came +out as the word "hyphen" and never collapsed onto ASCII `-`. Neither preset +dominated the other, and they failed on different inputs. -```python -assert strip_obfuscation(eclipsed) == strip_obfuscation("example.com") # mark: yes -assert strip_obfuscation("ex‐ample.com") == "ex hyphen ample.com" # hyphen: no -``` - -Neither preset dominates the other, and they fail on different inputs. Reading -that as "5/6 each, pick either" would be the wrong lesson. - -### There is no single call - -Measured across the whole matrix plus both vectors above, **no entry point -clears everything.** `catalog_key` comes closest — it is the only one carrying -both a confusable step and `strip_accents`, so it handles the mark *and* the -hyphen — and it has no format-stripping step, so the Unicode Tags block of -CVE-2025-32711 passes straight through it. +### Two calls now suffice, and the pairing is not a coincidence -The answer is a composition: +Both gaps are closed (#614, #615), and they had to close together: each was one +of the exactly two vectors behind the claim that nothing sufficed, so fixing one +alone would have left the other still failing. ```python -def canonical(text): - return canonicalize(strip_zalgo(text, max_marks=0)) +assert strip_obfuscation(eclipsed) == strip_obfuscation("example.com") +assert strip_obfuscation("ex‐ample.com") == strip_obfuscation("ex-ample.com") -assert canonical(eclipsed) == canonical("example.com") -assert canonical("ex‐ample.com") == canonical("ex-ample.com") -assert canonical(".g‌it/config") == ".git/config" +assert canonicalize_strict(eclipsed) == canonicalize_strict("example.com") +assert canonicalize_strict("ex‐ample.com") == canonicalize_strict("ex-ample.com") ``` -`strip_zalgo(max_marks=0)` supplies the step `canonicalize` lacks; -`canonicalize` supplies the confusable folding and format stripping that -`strip_zalgo` and `catalog_key` lack. That pair clears every vector on this -page, and `TestOneCall` gates both halves of the claim — including a test that -fails if any single entry point ever *does* become sufficient, so the guidance -gets revisited rather than silently left stale. +Measured across the whole matrix, `canonicalize_strict` and `strip_obfuscation` +each clear every row. What they share is the thing that matters: a confusable +fold **and** something that removes a combining mark. `catalog_key` misses only +the Unicode Tags block; `canonicalize` misses only the eclipsing mark. + +**`canonicalize` is deliberately still short by one.** The rule that closes +CVE-2017-7833 keys on the mark's own Script, and that is destructive for +scholarly transliteration, IPA and linguistic transcription, where marks from one +script legitimately sit on bases of another. So it lives in `canonicalize_strict`, +where a caller has already accepted a stricter contract, and not in the preset +meant for text that gets forwarded. + +That also means the advice below has not changed, only its reason. It used to +rest on "nothing suffices". It now rests on "the two that suffice are the two +most destructive ones", which is the same conclusion for a caller who has to +forward the text they cleaned. These 25 vectors are a spot check, not a measurement of the confusable space. Treat this as "no vector here needs a third call", not as coverage. diff --git a/proptest-regressions/presets.txt b/proptest-regressions/presets.txt index 23334dc0..842a43f0 100644 --- a/proptest-regressions/presets.txt +++ b/proptest-regressions/presets.txt @@ -11,3 +11,4 @@ cc da36583310da1e7223530863bfb32f55a617bf2ef42b31a6f08d71d60f8ae38f # shrinks to cc 88f3c26133409f858f19ddb38641040bc1c50665eb05233ddeab4c0ff8440776 # shrinks to s = "£" cc ddb34e6d319f0e4347521cb55a6c0a8582b5cfe49206664033e878768e51c551 # shrinks to s = "𐏈" cc e311270910ff3cb8ed5b4e5c5945101b01a02f59ab600fccedfb473bf498d899 # shrinks to s = "\u{80}" +cc 40dcc66bceeb4cda9984a876398f234fd85cc67e51771690a7306c0517a52526 # shrinks to s = "а\u{489}" diff --git a/src/emoji.rs b/src/emoji.rs index 06570170..d239e74b 100644 --- a/src/emoji.rs +++ b/src/emoji.rs @@ -319,13 +319,30 @@ pub(crate) fn pad_emoji_replacement(result: &mut String, text: &str) { /// materialising the full input for non-ASCII text. pub fn demojize_rust(text: &str, strip_modifiers: bool) -> String { let mut out = String::new(); - demojize_rust_into(text, strip_modifiers, &mut out); + demojize_rust_into(text, strip_modifiers, false, &mut out); out } /// In-place form of [`demojize_rust`] writing into `result` (cleared first), so /// the pipeline can reuse one buffer across steps (#236 item 7). -pub fn demojize_rust_into(text: &str, strip_modifiers: bool, result: &mut String) { +/// `skip_tr39_claimed` (#614): leave the 49 code points the TR39 confusable table +/// also claims for the confusable step to fold, instead of naming them here. +/// +/// Set only by comparison presets. `strip_obfuscation("\u{20AC}xample.com")` named the +/// euro sign — "euro xample.com" — so the spoof and the genuine string stopped being +/// equal rather than becoming equal, and CVE-2017-5383 survived a preset documented as +/// maximum-strength deobfuscation. Standalone `demojize` still names them, because +/// `demojize("I \u{2764} \u{20AC}5")` -> "I red heart euro 5" is what that function is for. +/// +/// Skipping here rather than reordering the steps is deliberate: `normalize_confusables` +/// runs *after* `demojize` so typographic punctuation inside emoji names (the `\u{2019}` +/// in "woman\u{2019}s hat") is folded too, and swapping them would break idempotency. +pub fn demojize_rust_into( + text: &str, + strip_modifiers: bool, + skip_tr39_claimed: bool, + result: &mut String, +) { result.clear(); // Fast path: pure-ASCII text cannot contain emoji. if text.is_ascii() { @@ -343,6 +360,18 @@ pub fn demojize_rust_into(text: &str, strip_modifiers: bool, result: &mut String continue; } + // #614: hand this code point to the confusable fold instead of naming it. + // Emitted verbatim, so the later `confusables` step sees it. + if skip_tr39_claimed && crate::tables::is_tr39_claimed_emoji_row(ch) { + if last_was_emoji && ch.is_alphanumeric() { + result.push(' '); + } + result.push(ch); + last_was_emoji = false; + win.advance(1); + continue; + } + if let Some((name, consumed)) = match_emoji_at(win.as_slice()) { let replacement = strip_modifier_suffix(name, strip_modifiers); pad_emoji_replacement(result, replacement); diff --git a/src/pipeline.rs b/src/pipeline.rs index 8d24acda..3c950f79 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -262,7 +262,7 @@ impl Pipeline { crate::presets::strip_bidi_into(input, out); Ok(true) } else if step == PipelineSteps::DEMOJIZE { - emoji::demojize_rust_into(input, false, out); + emoji::demojize_rust_into(input, false, false, out); Ok(true) } else if step == PipelineSteps::STRIP_ACCENTS { transliterate::strip_accents_into(input, out); diff --git a/src/presets.rs b/src/presets.rs index 23a4134f..92f794e2 100644 --- a/src/presets.rs +++ b/src/presets.rs @@ -77,8 +77,14 @@ enum Step { /// (`ᴔ`→`ǝo`, then `ǝ`→`e`); and the maps chain. Looping the whole core makes /// the preset idempotent. The inner list must not itself contain `FixedPoint`. FixedPoint(&'static [Step]), + /// #615: drop a combining mark whose own script differs from its base's. + /// `canonicalize_strict` only — see `zalgo::strip_cross_script_marks`. + StripCrossScriptMarks, Demojize { only_if_cldr: bool, + /// #614: leave the 49 rows TR39 also claims for the confusable step to fold. + /// Set by comparison presets only; standalone `demojize` still names them. + skip_tr39_claimed: bool, }, } @@ -227,11 +233,18 @@ fn apply_into( Ok(true) } } - Step::Demojize { only_if_cldr } => { + Step::StripCrossScriptMarks => { + zalgo::strip_cross_script_marks_into(input, out); + Ok(true) + } + Step::Demojize { + only_if_cldr, + skip_tr39_claimed, + } => { if only_if_cldr && !ctx.emoji_cldr { return Ok(false); } - emoji::demojize_rust_into(input, false, out); + emoji::demojize_rust_into(input, false, skip_tr39_claimed, out); Ok(true) } } @@ -338,6 +351,11 @@ impl Actionable { m.marks = true; m.strip_accents = true; } + // #615: touches combining marks only, and never a base character. It + // does NOT set `strip_accents`: that flag means "every mark goes", and + // this step keeps `Inherited` marks — the fast path must not treat a + // preset carrying it as one that flattens `café`. + Step::StripCrossScriptMarks => m.marks = true, Step::StripBidi => m.bidi = true, Step::StripZeroWidth => m.zero_width = true, Step::StripInvisible(_) => m.invisible = true, @@ -823,7 +841,10 @@ pub(crate) fn ml_normalize<'a>( // 1. NFKC normalization Step::Nfkc, // 2. Emoji → text (CLDR short names) when emoji_style == "cldr". - Step::Demojize { only_if_cldr: true }, + Step::Demojize { + only_if_cldr: true, + skip_tr39_claimed: false, + }, // 3. Transliterate if lang is set (e.g. "de" for ü→ue, "ja" for kana). // Use Ignore mode: ML pipelines need clean ASCII-ish output, so // characters with no mapping (e.g. katakana ー) should be dropped @@ -843,7 +864,10 @@ pub(crate) fn ml_normalize<'a>( // base is only named on the following call — non-idempotent. The // exposed bases name to plain ASCII ("approximately equal"), so a // single extra pass reaches the fixed point; no iteration is needed. - Step::Demojize { only_if_cldr: true }, + Step::Demojize { + only_if_cldr: true, + skip_tr39_claimed: false, + }, // 5. Unicode case folding (ß→ss, fi→fi, etc.) Step::FoldCase, // 6. Strip non-whitespace controls + zero-width, then fold whitespace (#433). @@ -1263,6 +1287,19 @@ pub(crate) fn canonicalize_strict(text: &str) -> Result, crate::Err // pass would consume (`c`+◌̧+◌̧ → `ç` then `c`). Looping makes the preset a // true fixed point — see `canonicalize` for the full rationale. Step::ConfusablesNfcFixedPoint("latin"), + // 4b. Drop a mark whose own script differs from its base's (#615, + // CVE-2017-7833). The zalgo cap above is a COUNT, and by count one Arabic + // shadda is indistinguishable from one acute accent, so no threshold + // removes the spoof and keeps `café`. + // + // Placed AFTER the confusable fold, not before, and that ordering is + // load-bearing: the fold rewrites the BASE, so a mark that matched its + // base beforehand can stop matching afterwards. `а` (Cyrillic) + U+0489 + // (Cyrillic mark) agrees on the first pass, then the fold makes the base + // Latin `a` and the next pass strips the mark — f(f(x)) != f(x), which + // `canonicalize_strict_idempotent` catches. Deciding against the FINAL + // base script is the only stable point. + Step::StripCrossScriptMarks, // 5. Fold whitespace (#433: fold-only — control/zero-width were already // stripped explicitly above, before the zalgo cap, per #121). The line // controls now fold to a space instead of being deleted, so `a\rb` → `a b`. @@ -1324,6 +1361,8 @@ pub(crate) fn strip_obfuscation(text: &str) -> Result, crate::Error // 5. Demojize — expand emoji to text names with spacing Step::Demojize { only_if_cldr: false, + // #614: this is a comparison preset, so the TR39 fold wins over the name. + skip_tr39_claimed: true, }, // 5b. Strip the #413 smuggling / non-interchange classes. Runs AFTER demojize // so the emoji pass sees flags/presentation selectors intact; whatever @@ -1680,6 +1719,81 @@ mod tests { } } + /// #614: inside a comparison preset the TR39 fold wins over the emoji name. + #[test] + fn strip_obfuscation_folds_the_rows_tr39_also_claims() { + // CVE-2017-5383. The euro sign is not an emoji; it reaches the emoji table + // from CLDR annotationsDerived, which names non-emoji characters. + assert_eq!( + strip_obfuscation("\u{20AC}xample.com").unwrap(), + "example.com" + ); + // Every glyph the CVE names now collapses onto its ASCII form. + for spoof in [ + "ex\u{2010}ample.com", + "ex\u{2011}ample.com", + "ex\u{2212}ample.com", + ] { + assert_eq!( + strip_obfuscation(spoof).unwrap(), + strip_obfuscation("ex-ample.com").unwrap(), + "{spoof:?}" + ); + } + } + + /// The skip is scoped: standalone `demojize` still names them. + #[test] + fn standalone_demojize_still_names_the_claimed_rows() { + let mut out = String::new(); + crate::emoji::demojize_rust_into("I \u{2764} \u{20AC}5", false, false, &mut out); + assert_eq!(out, "I red heart euro 5"); + } + + /// The reason the steps were NOT reordered: punctuation inside an emoji *name* + /// still has to be folded by the confusable pass, or the preset is not idempotent. + #[test] + fn emoji_name_punctuation_is_still_folded() { + let once = strip_obfuscation("\u{1F452}").unwrap(); + assert_eq!(once, "woman's hat"); + assert_eq!(strip_obfuscation(&once).unwrap(), once); + } + + /// #615: a mark whose own script differs from its base's is the CVE-2017-7833 + /// shape, and only `canonicalize_strict` removes it. + #[test] + fn canonicalize_strict_drops_a_cross_script_mark() { + // U+0651 ARABIC SHADDA on a Latin base. + assert_eq!( + canonicalize_strict("exa\u{651}mple.com").unwrap(), + canonicalize_strict("example.com").unwrap() + ); + // U+0E31 THAI MAI HAN AKAT — ccc == 0, so a combining-class test would miss it. + assert_eq!( + canonicalize_strict("exa\u{E31}mple.com").unwrap(), + canonicalize_strict("example.com").unwrap() + ); + } + + /// An `Inherited` mark attaches to anything, so ordinary diacritics survive. + #[test] + fn canonicalize_strict_keeps_ordinary_diacritics() { + for text in ["caf\u{e9}", "na\u{ef}ve", "Vi\u{1ec7}t Nam"] { + assert_eq!(canonicalize_strict(text).unwrap(), text, "{text:?}"); + } + } + + /// `canonicalize` deliberately does NOT get the rule — it is destructive for + /// scholarly transliteration, so it stays behind the stricter contract. + #[test] + fn canonicalize_does_not_get_the_cross_script_rule() { + let eclipsed = "exa\u{651}mple.com"; + assert_ne!( + canonicalize(eclipsed).unwrap(), + canonicalize("example.com").unwrap() + ); + } + #[test] fn preset_golden_fixtures() { // Frozen pre-refactor outputs — lock byte-identity for the #430 byte-stable @@ -2059,13 +2173,19 @@ mod tests { fn no_fold_step_list_is_the_folded_list_minus_fold_case() { const FULL: &[Step; 9] = &[ Step::Nfkc, - Step::Demojize { only_if_cldr: true }, + Step::Demojize { + only_if_cldr: true, + skip_tr39_claimed: false, + }, Step::Transliterate { mode: crate::ErrorMode::Ignore, only_if_lang: true, }, Step::StripAccents, - Step::Demojize { only_if_cldr: true }, + Step::Demojize { + only_if_cldr: true, + skip_tr39_claimed: false, + }, Step::FoldCase, Step::StripControl, Step::StripZeroWidth, diff --git a/src/tables/emoji_data.rs b/src/tables/emoji_data.rs index f736edf8..2be9275d 100644 --- a/src/tables/emoji_data.rs +++ b/src/tables/emoji_data.rs @@ -6,6 +6,7 @@ // Single-codepoint emoji to short name (1727 entries). include!(concat!(env!("OUT_DIR"), "/emoji_single_phf.rs")); +include!(concat!(env!("OUT_DIR"), "/emoji_tr39_overlap_phf.rs")); // Multi-codepoint emoji sequences (2553 entries) as a compact code-point trie // (#242 item 4): the production matcher walks `EMOJI_MULTI_TRIE_*` directly, diff --git a/src/tables/mod.rs b/src/tables/mod.rs index e7ead9ab..7fc12c59 100644 --- a/src/tables/mod.rs +++ b/src/tables/mod.rs @@ -803,6 +803,16 @@ pub fn lookup_emoji_single(ch: char) -> Option<&'static str> { emoji_data::EMOJI_SINGLE.get(&ch).copied() } +/// Whether the TR39 confusable table also claims this emoji row (#614). +/// +/// The set is derived at build time as the intersection of the two tables, and +/// `build.rs` asserts its size, so a table refresh that claims another confusable +/// source fails the build rather than silently widening the gap. +#[inline] +pub fn is_tr39_claimed_emoji_row(ch: char) -> bool { + emoji_data::EMOJI_ROWS_TR39_ALSO_CLAIMS.contains(&(ch as u32)) +} + /// Look up a multi-codepoint emoji sequence by its hex-underscore key (O(1) PHF). /// **Test-only**: the production matcher walks the code-point trie /// (`match_emoji_sequence`); this is retained as the equivalence oracle (#242 diff --git a/src/zalgo.rs b/src/zalgo.rs index ed6184a6..df9959e8 100644 --- a/src/zalgo.rs +++ b/src/zalgo.rs @@ -126,6 +126,63 @@ pub(crate) fn strip_zalgo_into(text: &str, max_marks: usize, out: &mut String) { out.extend(filtered.nfc()); } +/// Remove a combining mark whose own script is a *specific* script differing from the +/// script of the base it attaches to (#615, CVE-2017-7833). +/// +/// The CVE is domain spoofing "through the combination of Arabic and Indic vowel marker +/// characters with Latin characters", which "can obscure non-Latin characters in domain +/// names, making them invisible to most users while avoiding punycode encoding". +/// +/// `strip_zalgo`'s cap cannot reach it. That is a **count**, and by count one Arabic +/// shadda is indistinguishable from one acute accent, so no threshold removes the spoof +/// and keeps `café`. The discriminator the count lacks is already in disarm's script +/// data: +/// +/// | mark | `detect_char_script` | | +/// |---|---|---| +/// | `U+0301` COMBINING ACUTE | `Inherited` | a legitimate diacritic — kept | +/// | `U+0651` ARABIC SHADDA | `Arabic` | the CVE's vector — stripped off a Latin base | +/// | `U+0E31` THAI MAI HAN AKAT | `Thai` | likewise | +/// +/// This is UTS #39's mixed-script reasoning applied at the grapheme level rather than +/// across the whole string. A mark whose script is `Inherited` attaches to anything and +/// is never touched, which is why `café`, `naïve`, `Việt Nam` and Arabic *with* its own +/// vowel marks all pass through unchanged. +/// +/// Deliberately **not** in `canonicalize`, and not public. Scholarly transliteration, IPA +/// and linguistic transcription legitimately place marks from one script on bases of +/// another, and a strip that fires on those would be destructive in exactly the corpus +/// least able to notice. `canonicalize_strict` is where a caller has already accepted a +/// stricter contract. +/// +/// Only the in-place form exists: the preset runner reuses one scratch buffer across +/// steps (#236 item 7), so an owned wrapper would have no caller. +pub(crate) fn strip_cross_script_marks_into(text: &str, out: &mut String) { + out.clear(); + out.reserve(text.len()); + // The script of the most recent non-mark character — what a mark attaches to. + let mut base_script: Option<&'static str> = None; + for ch in text.chars() { + if is_combining_mark(ch) { + let mark_script = crate::scripts::detect_char_script(ch); + // `Inherited` means "takes the script of its base", so it can never + // conflict. `Common` marks (rare) are treated the same way. + let specific = mark_script != "Inherited" && mark_script != "Common"; + if specific && base_script.is_some_and(|b| b != mark_script) { + continue; // cross-script mark on a foreign base — the CVE's shape + } + out.push(ch); + continue; + } + base_script = match crate::scripts::detect_char_script(ch) { + // Punctuation, digits and whitespace do not re-anchor the base script. + "Common" | "Inherited" => base_script, + s => Some(s), + }; + out.push(ch); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/tests/test_cve_vectors.py b/tests/test_cve_vectors.py index b545becb..70b8ced0 100644 --- a/tests/test_cve_vectors.py +++ b/tests/test_cve_vectors.py @@ -56,6 +56,7 @@ canonicalize_strict, catalog_key, collapse_whitespace, + demojize, detect_scripts, escape_html, fold_case, @@ -1058,16 +1059,25 @@ def test_folds_to_the_ascii_prototype(self, spoof: str, ascii_form: str) -> None assert canonicalize(spoof) == ascii_form assert is_confusable(spoof) is True - def test_strip_obfuscation_names_them_instead_of_folding(self) -> None: - """MEASURED LIMIT: the deobfuscation preset is the wrong tool here. + def test_strip_obfuscation_folds_them(self) -> None: + """Inverted by #614. Kept, rather than deleted, so a regression fails loudly. - Naming a glyph is useful when a human has to read what was removed. It - is useless for comparison, because the spoof and the genuine host stop - being equal rather than becoming equal. + This asserted the opposite until the emoji step learned to skip the 49 code + points the TR39 table also claims: `strip_obfuscation` named the glyph + ("ex hyphen ample.com"), so the spoof and the genuine host stopped being equal + rather than becoming equal — useless for comparison, in a preset documented as + maximum-strength deobfuscation. """ - assert strip_obfuscation("ex‐ample.com") == "ex hyphen ample.com" - assert strip_obfuscation("ex-ample.com") == "ex-ample.com" - assert strip_obfuscation("ex‐ample.com") != strip_obfuscation("ex-ample.com") + assert strip_obfuscation("ex‐ample.com") == strip_obfuscation("ex-ample.com") + assert strip_obfuscation("€xample.com") == "example.com" + + def test_standalone_demojize_still_names_them(self) -> None: + """The skip is scoped to comparison presets, and that scoping is the point. + + `demojize("I ❤ €5")` -> "I red heart euro 5" is exactly what that function is + for; only the preset whose job is collision needs the fold to win. + """ + assert demojize("I ❤ €5") == "I red heart euro 5" class TestCombiningMarkEclipse: @@ -1085,19 +1095,48 @@ def test_the_mark_is_really_there_and_really_subthreshold(self) -> None: assert is_zalgo(ECLIPSED_HOST) is False assert has_anomalies(ECLIPSED_HOST) is True - def test_canonicalize_does_not_neutralize_it(self) -> None: - """MEASURED LIMIT — and it corrects guidance this page used to give. + def test_canonicalize_still_does_not_neutralize_it(self) -> None: + """``canonicalize`` is deliberately unchanged by #615. - ``canonicalize`` *caps* combining marks (#429) rather than removing - them, so a single mark survives and the spoof does not collapse onto the - genuine host. The matrix previously contained no vector of this shape, - which is how "canonicalize is the one call" came to be published. + It *caps* combining marks (#429) rather than removing them, so a single mark + survives and the spoof does not collapse onto the genuine host. The cap cannot + be made to reach this: by count, one Arabic shadda is indistinguishable from + one acute accent, so no threshold removes the spoof and keeps ``café``. + + The discriminator that does work — the mark's own Script — is destructive for + scholarly transliteration, IPA and linguistic transcription, where cross-script + marks are legitimate. So it lives in ``canonicalize_strict``, where the caller + has accepted a stricter contract, and not here. """ assert canonicalize(ECLIPSED_HOST) == ECLIPSED_HOST assert canonicalize(ECLIPSED_HOST) != canonicalize(PLAIN_HOST) - assert canonicalize_strict(ECLIPSED_HOST) != canonicalize_strict(PLAIN_HOST) assert strip_format(ECLIPSED_HOST) == ECLIPSED_HOST + def test_canonicalize_strict_neutralizes_it(self) -> None: + """Inverted by #615, and the reason the two-call composition is now history.""" + assert canonicalize_strict(ECLIPSED_HOST) == canonicalize_strict(PLAIN_HOST) + + @pytest.mark.parametrize( + "text", + [ + "café", + "naïve résumé", + "Việt Nam", + "مُحَمَّد", # Arabic WITH its own vowel marks — the likeliest false positive + "ภาษาไทย", + "हिन्दी", + ], + ) + def test_ordinary_diacritics_survive_the_strict_preset(self, text: str) -> None: + """The false-positive question, asked of the corpus most likely to fail it. + + Each passes through completely unchanged, marks included. The rule keys on the + mark's own Script: an ``Inherited`` mark attaches to anything and is never + touched, and a mark whose script matches its base is not cross-script. Only a + *specific* script differing from the base fires, which is the CVE's shape. + """ + assert canonicalize_strict(text) == text + @pytest.mark.parametrize( "defense", [strip_obfuscation, catalog_key], @@ -2460,15 +2499,22 @@ def test_catalog_key_clears_the_ranking_but_not_the_matrix(self) -> None: missed = {cve for cve in NEUTRALIZABLE if not _handles(catalog_key, cve)} assert missed == {"CVE-2025-32711"}, sorted(missed) - def test_the_two_near_misses_fail_on_opposite_vectors(self) -> None: - """The heart of it: neither preset dominates, and each fails alone. + def test_the_mirror_pair_is_closed(self) -> None: + """Inverted by #614 and #615, which is why they had to land together. + + These two used to fail on *opposite* vectors — the heart of the "no single + call" argument. `strip_obfuscation` named a confusable instead of folding it + (CVE-2017-5383); `canonicalize` capped combining marks by count, which cannot + tell an Arabic shadda from an acute accent (CVE-2017-7833). Each issue closed + one, so landing one alone would have left the other still failing and forced + this page to be rewritten twice. - Averaging these two into "5/6 each, pick either" would be the wrong - read — they do not fail on the same input, so neither is a safe default - and the pair is not interchangeable. + `canonicalize` is deliberately still short by one: #615's rule is destructive + for scholarly transliteration, so it went to `canonicalize_strict` only. """ + assert set(self.RANKING_VECTORS) - self._score(strip_obfuscation) == set() + assert set(self.RANKING_VECTORS) - self._score(canonicalize_strict) == set() assert set(self.RANKING_VECTORS) - self._score(canonicalize) == {"CVE-2017-7833"} - assert set(self.RANKING_VECTORS) - self._score(strip_obfuscation) == {"CVE-2017-5383"} def test_the_two_call_composition_clears_them_all(self) -> None: """The non-destructive answer, for text that has to be forwarded.""" @@ -2490,18 +2536,39 @@ def test_the_full_ranking_is_recorded(self) -> None: scores = {name: len(self._score(fn)) for name, fn in candidates.items()} assert scores == { "catalog_key": 6, + # Still 5, deliberately: #615's rule went to canonicalize_strict only, + # because it is destructive for scholarly transliteration and IPA. "canonicalize": 5, - "canonicalize_strict": 5, - "strip_obfuscation": 5, + "canonicalize_strict": 6, # 5 before #615 + "strip_obfuscation": 6, # 5 before #614 "search_key": 5, "normalize_confusables": 4, "strip_format": 1, "ml_normalize": 2, }, scores - def test_no_single_entry_point_clears_everything(self) -> None: - """The claim the page now makes, gated so it cannot quietly stop being true.""" + def test_the_two_sufficient_entry_points_still_are(self) -> None: + """Inverted by #614/#615. Until then this asserted the opposite. + + It read `assert missed, f"{name} now clears everything — update the guidance"`, + written so that closing a gap would fail loudly rather than leave the published + advice stale. It did exactly that, and this is the rewrite it asked for. + + Two entry points now clear the whole matrix, and the pairing is not a + coincidence: both carry a confusable fold *and* something that removes a + combining mark. Everything else is still short, so "clean unconditionally" + survives as advice — the reason has just moved from "nothing suffices" to + "only these two do, and they are the most destructive ones". + """ everything = set(NEUTRALIZABLE) | set(self.RANKING_VECTORS) + sufficient = { + "canonicalize_strict": canonicalize_strict, + "strip_obfuscation": strip_obfuscation, + } + for name, fn in sufficient.items(): + missed = {c for c in everything if not self._clears_any(fn, c)} + assert not missed, f"{name} no longer clears everything: {sorted(missed)}" + candidates = { "catalog_key": catalog_key, "canonicalize": canonicalize, @@ -2515,8 +2582,10 @@ def test_no_single_entry_point_clears_everything(self) -> None: "rag_ingest": get_pipeline("rag_ingest"), } for name, fn in candidates.items(): + if name in sufficient: + continue missed = {c for c in everything if not self._clears_any(fn, c)} - assert missed, f"{name} now clears everything — update the guidance" + assert missed, f"{name} now clears everything too — update the guidance" @pytest.mark.parametrize("cve", NEUTRALIZABLE) def test_the_composition_clears_the_whole_matrix(self, cve: str) -> None: From 5511fbde0f0e9e5b401a8433a9129f44d71cc8ed Mon Sep 17 00:00:00 2001 From: Richard Quinn Date: Thu, 27 Aug 2026 00:38:45 +0200 Subject: [PATCH 2/2] fix: a skipped TR39-claimed row must not fuse onto the emoji name before it (#614) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From review on #632. The separator decision looked at what the character IS, not what it will BECOME. The euro sign is not alphanumeric, so no space was emitted — but TR39 folds it to `e`, so "👒€" produced "woman's hat" + "e" and the later fold turned that into "woman's hate": a word present in neither the input nor any emoji name. Same shape for U+2211 -> `s` ("hats") and U+2200 -> `a` ("hata"). The fix reads the fold target rather than the raw character. Punctuation targets (U+2010 -> `-`) still take no separator, which matches how every other non-alphanumeric is emitted here, and the unspaced form now agrees with the spaced one. Only reachable through the #614 skip path, so it is a defect this branch introduced rather than a pre-existing one. Signed-off-by: Richard Quinn Assisted-by: Claude Code:claude-opus-5 --- src/emoji.rs | 11 ++++++++++- src/presets.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/emoji.rs b/src/emoji.rs index d239e74b..e15a90c8 100644 --- a/src/emoji.rs +++ b/src/emoji.rs @@ -363,7 +363,16 @@ pub fn demojize_rust_into( // #614: hand this code point to the confusable fold instead of naming it. // Emitted verbatim, so the later `confusables` step sees it. if skip_tr39_claimed && crate::tables::is_tr39_claimed_emoji_row(ch) { - if last_was_emoji && ch.is_alphanumeric() { + // The separator decision has to look at what this character will BECOME, + // not what it is. `\u{20AC}` is not alphanumeric, but TR39 folds it to `e`, + // so emitting it bare after an emoji name produced `"woman's hat"` + `"e"` + // -> `"woman's hate"` once the fold ran: a word that was in neither the + // input nor any name. `\u{2211}` -> `s` and `\u{2200}` -> `a` do the same. + // Punctuation targets (`\u{2010}` -> `-`) still take no separator, matching + // how every other non-alphanumeric is emitted here. + let becomes_alphanumeric = crate::tables::lookup_confusable(ch, "latin") + .is_some_and(|t| t.starts_with(char::is_alphanumeric)); + if last_was_emoji && (ch.is_alphanumeric() || becomes_alphanumeric) { result.push(' '); } result.push(ch); diff --git a/src/presets.rs b/src/presets.rs index 92f794e2..9953e9dd 100644 --- a/src/presets.rs +++ b/src/presets.rs @@ -1750,6 +1750,34 @@ mod tests { assert_eq!(out, "I red heart euro 5"); } + /// A skipped row must not fuse onto the emoji name before it (#614 review). + /// + /// The separator decision has to look at what the character will BECOME. `\u{20AC}` + /// is not alphanumeric, but TR39 folds it to `e`, so emitting it bare after a name + /// produced "woman's hat" + "e" -> "woman's hate" once the fold ran — a word present + /// in neither the input nor any emoji name. + #[test] + fn a_skipped_row_does_not_fuse_onto_the_preceding_name() { + for (input, expected) in [ + ("\u{1F452}\u{20AC}", "woman's hat e"), + ("\u{1F452}\u{2211}", "woman's hat s"), + ("\u{1F452}\u{2200}", "woman's hat a"), + ] { + assert_eq!(strip_obfuscation(input).unwrap(), expected, "{input:?}"); + } + // A punctuation target takes no separator, matching every other + // non-alphanumeric emitted here. + assert_eq!( + strip_obfuscation("\u{1F452}\u{2010}").unwrap(), + "woman's hat-" + ); + // And the spaced form agrees with the unspaced one. + assert_eq!( + strip_obfuscation("\u{1F452}\u{20AC}").unwrap(), + strip_obfuscation("\u{1F452} \u{20AC}").unwrap() + ); + } + /// The reason the steps were NOT reordered: punctuation inside an emoji *name* /// still has to be folded by the confusable pass, or the preset is not idempotent. #[test]