Open Bug 2042751 Opened 2 months ago Updated 5 days ago

Uncomputed canonical-combining-class (0xFF sentinel) sorted in ICU4X collator reachable from [@ js::intl::CompareStrings] via Intl.Collator.prototype.compare

Categories

(Core :: JavaScript: Internationalization API, defect, P2)

defect

Tracking

()

ASSIGNED
Tracking Status
firefox-esr115 --- unaffected
firefox-esr140 --- unaffected
firefox151 --- wontfix
firefox152 --- wontfix
firefox153 --- wontfix
firefox154 --- affected

People

(Reporter: bugmon, Assigned: hsivonen)

References

(Blocks 1 open bug, Regression)

Details

(5 keywords, Whiteboard: [bugmon:bisected,confirmed])

Attachments

(3 files)

Intl.Collator.prototype.compare reaches an engine-invariant violation in SpiderMonkey's string collation path. js::intl::CompareStrings (js/src/builtin/intl/Collator.cpp) forwards two two-byte JS strings to mozilla::intl::Collator::CompareUTF16 (Collator.cpp:707), which calls into the bundled Rust ICU4X collator (third_party/rust/icu_collator). While building collation elements for combining-mark runs, characters whose canonical combining class (CCC) is still the 0xFF "uncomputed" sentinel (placeholder / special non-starter decomposition) are gathered into the look-ahead buffer and passed to CollationElements::prepend_and_sort_non_starter_prefix_of_suffix, which sorts the run with slice.sort_by_key(|cc| cc.ccc()) BEFORE those classes have been computed.

CharacterAndClassAndTrieValue::ccc (third_party/rust/icu_collator/src/elements.rs:855-859) asserts that the class is not the 0xFF sentinel via debug_assert_ne!(ret, CanonicalCombiningClass::from_icu4c_value(0xFF)). Because a gathered placeholder still holds 0xFF, the assertion fails and aborts the process (MOZ_CRASH -> SIGSEGV on a null write, routed through mozglue's Rust panic hook). The bug is reached purely from JavaScript with --fuzzing-safe by calling Intl.Collator.prototype.compare (or String.prototype.localeCompare) on attacker-controlled strings containing certain combining sequences; it is independent of any privileged shell primitive.

The target file js/src/builtin/intl/Collator.cpp is byte-identical to current mozilla-central HEAD; the defect lives in the in-tree third-party Unicode collation crate that Collator.cpp uniquely exercises. On debug builds (Rust debug_assertions on) this is a deterministic abort/DoS; on release builds the debug_assert is compiled out and the same logic error instead silently mis-orders combining marks (reading a not-yet-computed class), corrupting collation/comparison results in security-relevant Unicode normalization code.

Build Info

Affected Code

File: js/src/builtin/intl/Collator.cpp, line 705-715

  } else {
    if (linear2->hasTwoByteChars()) {
      ret = coll->CompareUTF16(linear1->twoByteRange(nogc),   // <- Collator.cpp:707
                               linear2->twoByteRange(nogc));
    } else {
      // We don't have `CompareUTF16Latin1`, so we're responsible for flipping
      // the result here.
      ret = -(coll->CompareLatin1UTF16(linear2->latin1Range(nogc),
                                       linear1->twoByteRange(nogc)));
    }
  }

File: js/src/builtin/intl/Collator.cpp, line 730-753

static bool CollatorCompareFunction(JSContext* cx, unsigned argc, Value* vp) {
  CallArgs args = CallArgsFromVp(argc, vp);
  auto* compare = &args.callee().as<JSFunction>();
  auto collValue = compare->getExtendedSlot(CollatorCompareFunction_Collator);
  Rooted<CollatorObject*> collator(cx, &collValue.toObject().as<CollatorObject>());
  Rooted<JSString*> x(cx, JS::ToString(cx, args.get(0)));
  if (!x) return false;
  Rooted<JSString*> y(cx, JS::ToString(cx, args.get(1)));
  if (!y) return false;
  return CompareStrings(cx, collator, x, y, args.rval());   // <- Collator.cpp:752
}

File: third_party/rust/icu_collator/src/elements.rs, line 855-859

    fn ccc(&self) -> CanonicalCombiningClass {
        let ret = self.c_and_c.ccc();
        debug_assert_ne!(ret, CanonicalCombiningClass::from_icu4c_value(0xFF)); // fails: ret == 255
        ret
    }

File: third_party/rust/icu_collator/src/elements.rs, line 1550-1575

    fn prepend_and_sort_non_starter_prefix_of_suffix(&mut self, c: CharacterAndClassAndTrieValue) {
        let end = 1 + { /* scan non-starter prefix of upcoming */ };
        let start = c.decomposition_starts_with_non_starter() as usize;
        self.upcoming.insert(0, c);
        {
            let slice: &mut [CharacterAndClassAndTrieValue] = &mut self.upcoming[start..end];
            slice.sort_by_key(|cc| cc.ccc());   // sorts by CCC before placeholder CCC (0xFF) is computed
        };
    }

File: third_party/rust/icu_collator/src/elements.rs, line 833-844

    pub fn new_with_trie_val(c: char, trie_val: u32) -> Self {
        if !trie_value_indicates_special_non_starter_decomposition(trie_val) {
            CharacterAndClassAndTrieValue { c_and_c: CharacterAndClass::new_with_trie_value(c, trie_val), trie_val }
        } else {
            // CCC initialized to the 0xFF "uncomputed" sentinel, to be filled in later.
            CharacterAndClassAndTrieValue { c_and_c: CharacterAndClass::new(c, CanonicalCombiningClass::from_icu4c_value(0xFF)), trie_val }
        }
    }

CompareStrings hands two-byte strings to the ICU4X collator (entry point in the analyzed file), and the ICU4X normalization code sorts a combining run by a canonical combining class that has not yet been computed.

Exploit Chain

  1. Script calls let f = new Intl.Collator("en").compare; (or uses String.prototype.localeCompare).
  2. Script calls f(a, b) where a and b are two-byte JS strings containing combining-mark runs (e.g. Kannada vowel sign U+0CCB followed by combining marks, Tibetan U+0F00-block characters, Hangul jamo, or CJK + variation selectors).
  3. CollatorCompareFunction (Collator.cpp:752) stringifies the arguments and calls js::intl::CompareStrings.
  4. Both operands are two-byte, so CompareStrings takes the CompareUTF16 branch (Collator.cpp:707) and forwards the spans to mozilla::intl::Collator::CompareUTF16 -> mozilla_collator_glue_collator_compare_utf16 -> icu_collator compare_utf16/compare_impl.
  5. CollationElements::next normalizes the combining run; characters with special non-starter decomposition / placeholders are inserted into the look-ahead buffer with their canonical combining class set to the 0xFF "uncomputed" sentinel.
  6. prepend_and_sort_non_starter_prefix_of_suffix (elements.rs:1574) runs sort_by_key(|cc| cc.ccc()) over the run before the sentinel is replaced.
  7. CharacterAndClassAndTrieValue::ccc (elements.rs:857) observes the 0xFF sentinel and the debug_assert_ne! fails, aborting via the Rust panic hook -> MOZ_Crash -> SIGSEGV (debug build). On release builds the assert is gone and the 0xFF sentinel is used as a sort key, mis-ordering combining marks.

Steps to Reproduce

  1. Use the debug JS shell with assertions enabled: /firefox/obj-js-debug/dist/bin/js (the evaluator runs it with --fuzzing-safe).
  2. Run the deterministic reproducer collator_poc.js (fixed PRNG seed; exercises Intl.Collator.prototype.compare on combining-mark-heavy two-byte strings).
  3. Observe the abort: MOZ_CRASH(assertion left != right failed ... CanonicalCombiningClass(255)) at third_party/rust/icu_collator/src/elements.rs:857, with js::intl::CompareStrings (Collator.cpp:707) and CollatorCompareFunction (Collator.cpp:752) on the stack, before "done (no crash)" is printed.
  4. Alternatively run minimal_trigger.js: new Intl.Collator("en").compare("ೋ̈́", "ೋ̈́ೋ") (non-deterministic but small).

Security Impact

  • Severity: Low
  • Attacker capability: Web/JS content can force an aborting assertion failure in the JS engine (denial of service) on assertion-enabled builds purely by calling Intl.Collator.prototype.compare or String.prototype.localeCompare with crafted strings containing certain combining-mark sequences. On release builds the same defect silently mis-orders canonical combining marks (a not-yet-computed combining class is used as a sort key), producing incorrect, attacker-influenced collation/comparison results in Unicode normalization code; the underlying flaw is reading a value before it is initialized.
  • Preconditions: None beyond the ability to run JavaScript and call Intl.Collator.prototype.compare / String.prototype.localeCompare with attacker-controlled string arguments. Triggers under --fuzzing-safe with no special flags. The abort manifests on builds where the Rust collator is compiled with debug_assertions (debug builds); release builds turn it into a silent collation-correctness corruption.

ASAN Report

Hit MOZ_CRASH(assertion `left != right` failed
  left: CanonicalCombiningClass(255)
 right: CanonicalCombiningClass(255)) at third_party/rust/icu_collator/src/elements.rs:857
#13 <icu_collator::elements::CharacterAndClassAndTrieValue>::ccc  third_party/rust/icu_collator/src/elements.rs:857:9
#21 prepend_and_sort_non_starter_prefix_of_suffix  third_party/rust/icu_collator/src/elements.rs:1574:19
#22 CollationElements::next  third_party/rust/icu_collator/src/elements.rs:1889:38
#24 CollatorBorrowed::compare_utf16  third_party/rust/icu_collator/src/comparison.rs:797:29
#25 mozilla_collator_glue_collator_compare_utf16  js/src/builtin/intl/collator_glue/src/lib.rs:367:17
#26 mozilla::intl::Collator::CompareUTF16  dist/include/mozilla/intl/Collator.h:69:12
#27 js::intl::CompareStrings(...)  js/src/builtin/intl/Collator.cpp:707:19
#28 CollatorCompareFunction(...)  js/src/builtin/intl/Collator.cpp:752:10
UndefinedBehaviorSanitizer:DEADLYSIGNAL
==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (WRITE)
    #0 MOZ_CrashSequence(void*, long) dist/include/mozilla/Assertions.h:261:3
    #2 RustMozCrash mozglue/static/rust/wrappers.cpp:17:3
    #3 mozglue_static::panic_hook mozglue/static/rust/lib.rs:99:9
==ABORTING
Attached file minimal_trigger.js
Attached file crash_stack.txt
Attached file collator_poc.js
Group: core-security → javascript-core-security

Verified bug as reproducible on mozilla-central 20260527085321-7a75e5d0dc02.
The bug appears to have been introduced in the following build range:

Start: 538490c8d178c5f51a2bff59b68e33cfcce77a9a (20260310202213)
End: 7a30f16eb0153c58b6bc2a987f9e9392f228cb8e (20260310221000)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=538490c8d178c5f51a2bff59b68e33cfcce77a9a&tochange=7a30f16eb0153c58b6bc2a987f9e9392f228cb8e

Keywords: regression
Whiteboard: [bugmon:bisected,confirmed]
Regressed by: icu_collator

:hsivonen, since you are the author of the regressor, bug 1937541, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Flags: needinfo?(hsivonen)

This is a safe assertion crash, but let's keep this hidden, since this is a bad DoS vector is an email with this kind of subject is sent to a Thunderbird user or if Firefox persists a page title like this in browsing history.

Assignee: nobody → hsivonen
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(hsivonen)

Set release status flags based on info from the regressing bug 1937541

(In reply to Henri Sivonen (:hsivonen) from comment #7)

This is a safe assertion crash, but let's keep this hidden, since this is a bad DoS vector is an email with this kind of subject is sent to a Thunderbird user or if Firefox persists a page title like this in browsing history.

Oh, this is just a debug assertion. It's not even a DoS vector, then.

The minimal repro doesn't repro this for me when copied into ICU4X's Rust tests.

Opening up per comment 9.

Group: javascript-core-security
Keywords: sec-other

It seems to me that the root cause is this:

When upcoming originally became potentially longer than one scalar, it always held already-normalized lookahead. The backoff after finding the identical prefix started prepending scalars to upcoming in unnormalized state and there's code to change the contents of longer-than-1 upcoming from unnormalized to normalized when needed.

It seems to me that we now arrive at a place where the code assumes that upcoming is normalized if its length isn't 1, but upcoming actually remains unnormalized.

The main thing is finding the right place for just-in-time normalization of upcoming such that we don't end up uselessly normalizing it when normalizing isn't necessary.

A possible middle ground is making the backoff after identical prefix to decompose the handful of characters (including the Tibetan characters used by the PoC) that have the unusual trait of having a canonical decomposition despite having non-zero canonical combining class.

Blocks: js-lang
Severity: -- → S3
Priority: -- → P2

Set release status flags based on info from the regressing bug 1937541

Duplicate of this bug: 2057442
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: