diff --git a/CHANGELOG.md b/CHANGELOG.md index dba6cba9..80e2266b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -493,6 +493,37 @@ compatibility (see [RELEASING.md](RELEASING.md)). ### Fixed +- **`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 + code points passed the screen clean and **all 18** survived into `canonical`. + + The one that did flag was flagging for the wrong reason, and it is the #605 bug in + a class #605 did not cover: `U+FDD0` sits in the Arabic Presentation Forms range, + so the script detector read it as a letter and `paypal.evil.com` reported + `scripts=['Latin', 'Arabic']` with `mixed_script=True`. Widening the existing + per-label strip fixes that by construction rather than by special case, because + the strip already runs before `detect_scripts`. + + Reported on the **existing** `has_invisible` field rather than four new ones, so no + binding payload changes: no new field on the Ruby positional tuple and no change to + the Java `jni_sig!` string. `is_invisible_in_hostname` composes the four class + predicates that `src/invisibles.rs` already carried. + + Private use and the variation selectors are included because RFC 5892 puts all four + of the classes added here in DISALLOWED outright. (The pre-existing zero-width set is + not uniformly disallowed — `U+200C`/`U+200D` are CONTEXTJ, conditionally permitted — + and flagging those remains the deliberate fail-closed policy #605 chose, not a reading + of the RFC.) Both have legitimate uses in ordinary text, so + a general-text detector needs its own argument for them; that is tracked separately. + Measured against the full suite including the adversarial-oracle clean corpus: no + false positives. + + The tag block is the reason this is a security fix rather than tidying. + `U+E0061`–`U+E007A` spell arbitrary Latin invisibly, and the screen previously + called such a hostname clean *and* returned the payload intact in `canonical` — the + combination that turns a detector into a laundering step. + - **`PRESETS["ml_normalize"]` was missing two of the nine steps it claims to describe (#600).** `PRESETS` is a hand-maintained Python mirror of the `const STEPS` arrays in `src/presets.rs`; nothing executes it, and it had drifted. The mirror listed seven diff --git a/docs/api/predicates.md b/docs/api/predicates.md index 4b12a118..2d1c35bf 100644 --- a/docs/api/predicates.md +++ b/docs/api/predicates.md @@ -107,7 +107,7 @@ The second element of the tuple returned by `is_suspicious_hostname()`: | `has_confusables` | `bool` | `True` if any label contains a Latin-confusable character | | `bidi_conflict` | `bool` | `True` if the decoded hostname mixes strong LTR and RTL characters (the "BiDi Swap" precondition); **folded into** `suspicious` | | `bidi_control` | `bool` | `True` if the decoded hostname carries a UAX #9 bidi control character — override (`U+202D`/`U+202E`), embedding (`U+202A`–`U+202C`), isolate (`U+2066`–`U+2069`) or directional mark (`U+200E`/`U+200F`/`U+061C`). Disjoint from `bidi_conflict`, which reads strong-direction *letters* only. **Folded into** `suspicious`; the characters are stripped from `canonical` | -| `has_invisible` | `bool` | `True` if the decoded hostname carries a zero-width or invisible-format character — `U+200B`–`U+200D`, `U+2060`–`U+2064`, `U+FEFF`, `U+180E`. Disjoint from `bidi_control`: these carry no direction at all. **Folded into** `suspicious`, and removed before any other field is computed, so they never reach `scripts`, `mixed_script` or `canonical` | +| `has_invisible` | `bool` | `True` if the decoded hostname carries an invisible character of any class: zero-width (`U+200B`–`U+200D`, `U+2060`–`U+2064`, `U+FEFF`, `U+180E`), tag (`U+E0000`–`U+E007F`), variation selector (`U+FE00`–`U+FE0F`, `U+E0100`–`U+E01EF`), noncharacter (`U+FDD0`–`U+FDEF` and the last two of every plane), private use (`U+E000`–`U+F8FF`, planes 15 and 16). Disjoint from `bidi_control`: these carry no direction at all. RFC 5892 puts the tag, variation-selector, noncharacter and private-use classes in DISALLOWED outright; `U+200C`/`U+200D` are CONTEXTJ (conditionally permitted) and the screen flags them anyway, as a deliberate fail-closed policy. **Folded into** `suspicious`, and removed per label before any other field is computed, so they never reach `scripts`, `mixed_script` or `canonical` | | `cross_label_script` | `bool` | `True` if the labels span more than one script; broader/noisier than `bidi_conflict` (fires on benign IDN ccTLDs like `google.рф`), so **not** folded into `suspicious` | | `label_scripts` | `list[list[str]]` | Per-label resolved scripts, left to right | | `whole_script_confusable` | `bool` | `True` if any label is single-script, non-Latin, whose confusable skeleton is entirely Latin (`аррӏе`→`apple`). A graded **signal, not a verdict** — **not** folded into `suspicious` (fires on `ру`→`py`, `оса`→`oca`) | diff --git a/python/disarm/_api.py b/python/disarm/_api.py index a9665e71..c2390e18 100644 --- a/python/disarm/_api.py +++ b/python/disarm/_api.py @@ -1269,13 +1269,21 @@ def is_suspicious_hostname( reads strong-direction *letters* only and is therefore blind to the RLO extension spoof. IDNA2008 disallows every character in the set, so this is folded into ``suspicious`` and the characters are stripped from ``canonical``. - - ``has_invisible``: bool — True if the decoded hostname carries a zero-width or - invisible-format character: ``U+200B``-``U+200D``, ``U+2060``-``U+2064``, - ``U+FEFF`` or ``U+180E``. Disjoint from ``bidi_control`` — these carry no - direction at all, so neither bidi field can see them. Folded into - ``suspicious``. They are removed *before* any other field is computed, so a - hostname whose only non-ASCII is an invisible no longer reports a phantom - script (``U+FEFF`` sits in the Arabic Presentation Forms block). + - ``has_invisible``: bool — True if the decoded hostname carries an invisible + character of any class: zero-width (``U+200B``-``U+200D``, + ``U+2060``-``U+2064``, ``U+FEFF``, ``U+180E``), tag (``U+E0000``-``U+E007F``), + variation selector (``U+FE00``-``U+FE0F``, ``U+E0100``-``U+E01EF``), + noncharacter (``U+FDD0``-``U+FDEF`` and the last two of every plane), or + private use (``U+E000``-``U+F8FF``, planes 15 and 16). Disjoint from + ``bidi_control`` — these carry no direction at all, so neither bidi field can + see them. RFC 5892 puts the tag, variation-selector, noncharacter and + private-use classes in DISALLOWED outright, which is what justifies including + private use and variation selectors here. ``U+200C``/``U+200D`` are the + exception — CONTEXTJ, so conditionally permitted; the screen flags them anyway + as a deliberate fail-closed policy. Folded into ``suspicious``. They are + removed per label *before* any other field is computed, so a hostname whose + only non-ASCII is an invisible no longer reports a phantom script (``U+FEFF`` + sits in the Arabic Presentation Forms block, ``U+FDD0`` in its range). - ``cross_label_script``: bool — True if the labels span more than one distinct script. Broader and noisier than ``bidi_conflict`` (it fires on benign IDN ccTLDs like ``google.рф``), so it is **not** folded into diff --git a/src/api/safety.rs b/src/api/safety.rs index e84f7b94..bd228dcd 100644 --- a/src/api/safety.rs +++ b/src/api/safety.rs @@ -447,13 +447,26 @@ pub struct HostnameAnalysis { /// [`suspicious`](Self::suspicious) and the characters are stripped from /// [`canonical`](Self::canonical). pub bidi_control: bool, - /// Whether the decoded hostname contains a zero-width or invisible-format - /// character — `U+200B`–`U+200D`, `U+2060`–`U+2064`, `U+FEFF` or `U+180E` (#605). + /// Whether the decoded hostname contains an invisible character of any class + /// (#605, widened by #610): + /// + /// - **zero-width** — `U+200B`–`U+200D`, `U+2060`–`U+2064`, `U+FEFF`, `U+180E` + /// - **tag** — `U+E0000`–`U+E007F`, the ASCII-smuggling channel + /// - **variation selector** — `U+FE00`–`U+FE0F`, `U+E0100`–`U+E01EF` + /// - **noncharacter** — `U+FDD0`–`U+FDEF` and the last two of every plane + /// - **private use** — `U+E000`–`U+F8FF`, plane 15 and plane 16 /// /// Disjoint from [`bidi_control`](Self::bidi_control): these carry no direction at /// all, so neither that field nor [`bidi_conflict`](Self::bidi_conflict) can see - /// them. IDNA2008 disallows them, so this is folded into - /// [`suspicious`](Self::suspicious) and the characters are stripped from + /// them. RFC 5892 puts the tag, variation-selector, noncharacter and private-use + /// classes in DISALLOWED outright, which is what justifies including private use + /// and the variation selectors here — both have legitimate uses in ordinary text, + /// so a general-text detector would need its own argument for them. `U+200C` and + /// `U+200D` are the exception: CONTEXTJ, so conditionally permitted. This screen + /// flags them anyway, which is a deliberate fail-closed policy (#605) rather than + /// something the RFC settles. Folded into [`suspicious`](Self::suspicious); the characters are + /// removed per label *before* any other field is computed, so they never reach + /// [`scripts`](Self::scripts), [`mixed_script`](Self::mixed_script) or /// [`canonical`](Self::canonical). pub has_invisible: bool, /// Whether the labels resolve to more than one distinct script (Common / diff --git a/src/hostname.rs b/src/hostname.rs index 4183359c..62e11bf0 100644 --- a/src/hostname.rs +++ b/src/hostname.rs @@ -2,6 +2,32 @@ use unicode_normalization::UnicodeNormalization; use crate::{confusables, invisibles, scripts}; +/// Every invisible class a hostname label may not legitimately contain (#605, #610). +/// +/// The union of the zero-width set with the four class predicates +/// [`invisibles::is_tag`], [`invisibles::is_variation_selector`], +/// [`invisibles::is_noncharacter`] and [`invisibles::is_pua`]. +/// +/// RFC 5892 puts the four classes #610 added in DISALLOWED outright, which is what +/// justifies folding PUA and the variation selectors in here — both have legitimate +/// uses in ordinary text, so a general-text detector needs a separate argument for +/// them. The zero-width set is not uniformly disallowed: `U+200C`/`U+200D` are +/// CONTEXTJ, permitted in the specific joining contexts RFC 5892 Appendix A.1/A.2 +/// describe. This screen flags them anyway (#605), because a spoof screen has no +/// reason to honour a context rule it cannot verify, and that is a deliberate policy +/// rather than a reading of the RFC. +/// +/// Deliberately excludes the UAX #9 bidi controls: those are +/// [`scripts::is_bidi_control`] and are reported through `bidi_control` (#603), so the +/// two predicates partition the space rather than overlap. +fn is_invisible_in_hostname(ch: char) -> bool { + invisibles::is_zero_width(ch) + || invisibles::is_tag(ch) + || invisibles::is_variation_selector(ch) + || invisibles::is_noncharacter(ch) + || invisibles::is_pua(ch) +} + /// Check if a bracketed string is a valid IPv6 literal per RFC 3986 §3.2.2. /// /// Requires: starts with `[`, ends with `]`, content contains `:`, @@ -215,15 +241,25 @@ pub(crate) fn is_suspicious_hostname_opts( } else { raw_label.to_string() }; - // Zero-width / invisible-format characters (#605). Removed HERE, before script - // analysis, rather than later on the joined hostname: U+FEFF sits in the Arabic - // Presentation Forms block and U+180E in the Mongolian block, so leaving them in - // makes `detect_scripts` report a script the reader cannot see and `mixed_script` - // fire on an ASCII-looking host. Stripping first means nothing downstream — - // scripts, mixed_script, confusables, canonical — ever sees them. - if label.chars().any(invisibles::is_zero_width) { + // Invisible characters of every class (#605, widened by #610). Removed HERE, + // before script analysis, rather than later on the joined hostname: U+FEFF sits + // in the Arabic Presentation Forms block, U+180E in the Mongolian block and + // U+FDD0 in the Arabic Presentation Forms range, so leaving them in makes + // `detect_scripts` report a script the reader cannot see and `mixed_script` fire + // on an ASCII-looking host. Stripping first means nothing downstream — scripts, + // mixed_script, confusables, canonical — ever sees them. + // + // RFC 5892 puts the four classes #610 added in DISALLOWED outright, so a hostname + // carrying one is malformed whatever its intent and the screen fails closed. That + // is why PUA and the variation selectors are included here but would need a + // separate argument in a general-text detector, where both have legitimate uses. + // ZWNJ/ZWJ are the exception: CONTEXTJ, so conditionally permitted. Flagging them + // is a policy choice (#605), not something the RFC settles. + // The tag block is the ASCII-smuggling channel: U+E0061-U+E007A spell arbitrary + // Latin invisibly, so returning them in `canonical` would launder the payload. + if label.chars().any(is_invisible_in_hostname) { has_invisible = true; - label.retain(|c| !invisibles::is_zero_width(c)); + label.retain(|c| !is_invisible_in_hostname(c)); } decoded_labels.push(label.clone()); diff --git a/tests/test_hn_features.py b/tests/test_hn_features.py index d01a0734..0c37c129 100644 --- a/tests/test_hn_features.py +++ b/tests/test_hn_features.py @@ -391,10 +391,28 @@ class TestInvisibleSetDoesNotDriftFromItsDocs: exhaustive gate rather than a sampled one. """ - #: The set as documented. Kept literal so a change has to be deliberate. - DOCUMENTED = frozenset( + #: The zero-width singletons (#605). Literal so a change has to be deliberate. + ZERO_WIDTH = frozenset( {0x200B, 0x200C, 0x200D, 0x2060, 0x2061, 0x2062, 0x2063, 0x2064, 0xFEFF, 0x180E} ) + #: Unicode Tags block (#610). A range, because it is one. + TAG_BLOCK = range(0xE0000, 0xE0080) + + @classmethod + def documented(cls) -> frozenset: + """The expected set, restricted to what :meth:`_derive` can afford to probe. + + #610 widened the screen to four more classes, and three of them sit outside + ``Cf``/``Zs``: variation selectors are ``Mn``, noncharacters ``Cn``, PUA + ``Co``. Probing those exhaustively means ~138k round trips into Rust (PUA + alone is 137,468 code points), which is minutes, not the 0.3s this gate is + worth. So this stays exhaustive over the format space it can afford and + ``TestRemainingInvisibleClasses`` covers the other three by representative + member — stated here rather than left as an unexplained gap. + """ + return frozenset(cls.ZERO_WIDTH) | { + cp for cp in cls.TAG_BLOCK if unicodedata.category(chr(cp)) in ("Cf", "Zs") + } ROOT = Path(__file__).resolve().parent.parent #: Every file whose prose enumerates the set. @@ -422,9 +440,10 @@ def _derive() -> frozenset: def test_behaviour_matches_the_documented_set(self): derived = self._derive() - assert derived == self.DOCUMENTED, { - "flagged but undocumented": sorted(f"U+{c:04X}" for c in derived - self.DOCUMENTED), - "documented but not flagged": sorted(f"U+{c:04X}" for c in self.DOCUMENTED - derived), + documented = self.documented() + assert derived == documented, { + "flagged but undocumented": sorted(f"U+{c:04X}" for c in derived - documented), + "documented but not flagged": sorted(f"U+{c:04X}" for c in documented - derived), } @pytest.mark.parametrize("path", PROSE, ids=lambda p: p.name) @@ -437,14 +456,33 @@ def test_prose_enumerates_the_whole_set(self, path): """ text = path.read_text(encoding="utf-8") named = {int(m, 16) for m in re.findall(r"U\+([0-9A-Fa-f]{4,6})", text)} - # Endpoints of the two documented ranges, plus the two singletons. - required = {0x200B, 0x200D, 0x2060, 0x2064, 0xFEFF, 0x180E} + # Endpoints of every documented range, plus the singletons. #610 added the + # tag / variation-selector / noncharacter / PUA classes, so each file has to + # name those too — that is the drift this gate exists to catch. + required = { + 0x200B, + 0x200D, + 0x2060, + 0x2064, + 0xFEFF, + 0x180E, # zero-width + 0xE0000, + 0xE007F, # tag block + 0xFE00, + 0xFE0F, + 0xE0100, + 0xE01EF, # variation selectors + 0xFDD0, + 0xFDEF, # noncharacters + 0xE000, + 0xF8FF, # private use (BMP; the plane ranges are named in prose) + } missing = sorted(f"U+{c:04X}" for c in required - named) assert not missing, f"{path.name} omits {missing}" def test_invisibles_and_bidi_controls_do_not_overlap(self): """The two fields are documented as disjoint. Checked, not asserted in prose.""" - for cp in sorted(self.DOCUMENTED): + for cp in sorted(self.ZERO_WIDTH): _, analysis = is_suspicious_hostname(f"paypal{chr(cp)}.evil.com") assert analysis.has_invisible, f"U+{cp:04X}" assert not analysis.bidi_control, f"U+{cp:04X} claimed by both fields" @@ -522,6 +560,53 @@ def test_ace_path_still_fails_closed(self): assert suspicious +class TestRemainingInvisibleClasses: + """#610: tags, variation selectors, noncharacters and PUA — the classes + #605's zero-width set did not reach.""" + + CLASSES = [ + ("tag", "\U000e0001"), + ("tag", "\U000e0061"), + ("tag", "\U000e007f"), + ("variation-selector", "\ufe00"), + ("variation-selector", "\ufe0f"), + ("variation-selector", "\U000e0100"), + ("noncharacter", "\ufdd0"), + ("noncharacter", "\ufffe"), + ("noncharacter", "\U0001ffff"), + ("pua", "\ue000"), + ("pua", "\uf8ff"), + ("pua", "\U000f0000"), + ] + + @pytest.mark.parametrize(("klass", "ch"), CLASSES) + def test_class_is_flagged(self, klass, ch): + suspicious, d = is_suspicious_hostname(f"paypal{ch}.evil.com") + assert suspicious, klass + assert d.has_invisible, klass + + @pytest.mark.parametrize(("klass", "ch"), CLASSES) + def test_class_never_survives_into_canonical(self, klass, ch): + _, d = is_suspicious_hostname(f"paypal{ch}.evil.com") + assert ch not in d.canonical, klass + + def test_noncharacter_is_not_reported_as_arabic(self): + # The #605 phantom-script bug, in a class #605 did not cover. U+FDD0 sits + # in the Arabic Presentation Forms range, so the script detector read it + # as a letter and mixed_script fired on an ASCII-looking hostname. + _, d = is_suspicious_hostname("paypal\ufdd0.evil.com") + assert d.scripts == ["Latin"] + assert not d.mixed_script + + def test_tag_payload_does_not_reach_canonical(self): + # U+E0061-U+E007A spell arbitrary Latin text invisibly. A screen that + # returns them in `canonical` launders the payload. + secret = "".join(chr(0xE0000 + ord(c)) for c in "evil") + _, d = is_suspicious_hostname(f"paypal{secret}.com") + assert d.has_invisible + assert all(c not in d.canonical for c in secret) + + class TestWholeScriptConfusable: """#545: whole_script_confusable / label_whole_script_confusable — a graded signal (NOT folded into suspicious) naming the whole-script-spoof mechanism."""