High CVSS 8.8 firefox UAF 🔧 Commit mapped

Overview

High
Severity
8.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionAn attacker could have caused a use-after-free via the Custom Highlight API, leading to a potentially exploitable crash.
ComponentDOM
Bug ClassUAF
Tracker1936982
Fix commitb80feb7671b2 (firefox) +24/-22
CWECWE-416 (Use-after-free)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H
CISA KEVNot listed
CreditedAtte Kettunen
Disclosed2025-02-04

Background

`AbstractRange`
The base class for DOM ranges (nsRange and the static ranges used by the Custom Highlight API) that can register themselves against a common ancestor node so selection/highlight painting can be updated efficiently.
`mRegisteredClosestCommonInclusiveAncestor`
A raw nsINode* member on AbstractRange recording the node under which the range is currently registered for selection tracking.
Custom Highlight API
A Web API that lets script create Highlight objects containing Range/StaticRange objects, which are registered against common ancestor nodes and can be mutated or freed from script during DOM operations.
`UnregisterClosestCommonInclusiveAncestor`
The method that removes a range from its registered ancestor’s range list and clears the associated node flags, previously requiring the caller to pass the node pointer explicitly.

Root Cause Analysis

AbstractRange::UnregisterClosestCommonInclusiveAncestor took an nsINode* aNode argument that every caller supplied as mRegisteredClosestCommonInclusiveAncestor, and it dereferenced aNode to fetch the ancestor range list and clear flags before nulling out the member. Because the registered ancestor is only weakly referenced by the raw pointer, a caller could pass a node that had already been (or was in the process of being) destroyed — for example when a Highlight Range outlived the node it was registered against during Custom Highlight API teardown or DOM mutation — so dereferencing aNode was a use-after-free. The invariant that “the node passed in is still alive and equals the currently registered ancestor” was implicit and unenforced, and the various call sites (unlinking, UnregisterSelection, UpdateCommonAncestorIfNecessary, nsRange::CharacterDataChanged) could violate it.

The fix removes the aNode parameter entirely and instead reads mRegisteredClosestCommonInclusiveAncestor internally into a strong nsCOMPtr (oldClosestCommonInclusiveAncestor), nulls the member first, and operates on the owning reference, guaranteeing the node stays alive for the duration of the unregister and eliminating the dangling-pointer dereference. Early-returning when no ancestor is registered further removes reliance on the old asserted preconditions.

Key insight
The core mistake was letting callers hand in a raw, weakly-held node pointer that could already be freed, then dereferencing it; the fix makes the method the sole owner of that lookup by reading the member into a strong nsCOMPtr that keeps the node alive across the operation.

Attack Path

  1. Create highlighted ranges Script uses the Custom Highlight API to create Range/StaticRange objects inside a Highlight, causing them to register against a closest-common-ancestor node.
  2. Trigger a DOM mutation Script mutates the tree (e.g. character-data changes, node splitting, or removal) so a code path such as CharacterDataChanged or UpdateCommonAncestorIfNecessary needs to unregister the range from its ancestor.
  3. Free the ancestor node Through the same mutation or reference dropping, the previously registered ancestor node is destroyed while the range still holds only a raw pointer to it.
  4. Dereference the freed node UnregisterClosestCommonInclusiveAncestor is called with the stale node pointer and dereferences it to fetch the range list and clear flags, reading and writing freed memory.
  5. Achieve exploitable crash The use-after-free corrupts or reads reclaimed heap memory, yielding a potentially controllable crash the attacker can shape via heap grooming.

Impact Assessment

An attacker running untrusted script in a web page gains a use-after-free of a DOM node in the content process, a high-severity primitive that can lead to an exploitable crash and potentially memory corruption suitable for further exploitation. The precondition is the ability to create ranges via the Custom Highlight API (or otherwise register selection ranges) and then drive DOM mutations that free the registered ancestor while a range remains registered. No special privileges beyond ordinary JavaScript execution are required.

Changed Functions

FunctionChangeNotes
if
dom/base/AbstractRange.cpp
modified
for
dom/base/AbstractRange.cpp
modified
if
dom/base/nsRange.cpp
modified

Files Changed

  • dom/base/AbstractRange.cpp
  • dom/base/AbstractRange.h
  • dom/base/nsRange.cpp

Audit Directions

  • Weak node pointers passed as arguments
    Search for methods that accept a raw nsINode*/nsIContent* matching a weakly-held member and dereference it; prefer reading the member into a strong nsCOMPtr internally instead of trusting caller-supplied lifetimes.
  • Range/selection registration lifetimes
    Audit all Register/Unregister pairs around mRegisteredClosestCommonInclusiveAncestor and Custom Highlight API teardown for paths where the registered node can be freed before unregistration completes.
  • Implicit alive-and-equal invariants
    Look for MOZ_ASSERT/NS_ASSERTION preconditions asserting a pointer equals a member or is non-null, since these mask cases where the object is already dead in release builds; convert such invariants into strong-reference ownership or early-return guards.
diff --git a/dom/base/AbstractRange.cpp b/dom/base/AbstractRange.cpp
index 781eb25bf28..4d72fea8e12 100644
--- a/dom/base/AbstractRange.cpp
+++ b/dom/base/AbstractRange.cpp
@@ -77,8 +77,7 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(AbstractRange)
   // This may introduce additional overhead which is not needed when unlinking,
   // therefore this is done here beforehand.
   if (tmp->mRegisteredClosestCommonInclusiveAncestor) {
-    tmp->UnregisterClosestCommonInclusiveAncestor(
-        tmp->mRegisteredClosestCommonInclusiveAncestor, true);
+    tmp->UnregisterClosestCommonInclusiveAncestor(true);
   }
   MOZ_DIAGNOSTIC_ASSERT(!tmp->isInList(),
                         "Shouldn't be registered now that we're unlinking");
@@ -421,8 +420,7 @@ const nsTArray<WeakPtr<Selection>>& AbstractRange::GetSelections() const {
 void AbstractRange::UnregisterSelection(const Selection& aSelection) {
   mSelections.RemoveElement(&aSelection);
   if (mSelections.IsEmpty() && mRegisteredClosestCommonInclusiveAncestor) {
-    UnregisterClosestCommonInclusiveAncestor(
-        mRegisteredClosestCommonInclusiveAncestor, false);
+    UnregisterClosestCommonInclusiveAncestor();
     MOZ_DIAGNOSTIC_ASSERT(
         !mRegisteredClosestCommonInclusiveAncestor,
         "How can we have a registered common ancestor when we "
@@ -456,18 +454,18 @@ void AbstractRange::RegisterClosestCommonInclusiveAncestor(nsINode* aNode) {
 }
 
 void AbstractRange::UnregisterClosestCommonInclusiveAncestor(
-    nsINode* aNode, bool aIsUnlinking) {
-  MOZ_ASSERT(aNode, "bad arg");
-  NS_ASSERTION(aNode->IsClosestCommonInclusiveAncestorForRangeInSelection(),
-               "wrong node");
-  MOZ_DIAGNOSTIC_ASSERT(aNode == mRegisteredClosestCommonInclusiveAncestor,
-                        "wrong node");
+    bool aIsUnlinking) {
+  if (!mRegisteredClosestCommonInclusiveAncestor) {
+    return;
+  }
+  nsCOMPtr oldClosestCommonInclusiveAncestor =
+      mRegisteredClosestCommonInclusiveAncestor;
+  mRegisteredClosestCommonInclusiveAncestor = nullptr;
   LinkedList<AbstractRange>* ranges =
-      aNode->GetExistingClosestCommonInclusiveAncestorRanges();
+      oldClosestCommonInclusiveAncestor
+          ->GetExistingClosestCommonInclusiveAncestorRanges();
   MOZ_ASSERT(ranges);
 
-  mRegisteredClosestCommonInclusiveAncestor = nullptr;
-
 #ifdef DEBUG
   bool found = false;
   for (AbstractRange* range : *ranges) {
@@ -485,9 +483,11 @@ void AbstractRange::UnregisterClosestCommonInclusiveAncestor(
   // We don't want to waste time unmarking flags on nodes that are
   // being unlinked anyway.
   if (!aIsUnlinking && ranges->isEmpty()) {
-    aNode->ClearClosestCommonInclusiveAncestorForRangeInSelection();
-    UnmarkDescendants(*aNode);
+    oldClosestCommonInclusiveAncestor
+        ->ClearClosestCommonInclusiveAncestorForRangeInSelection();
+    UnmarkDescendants(*oldClosestCommonInclusiveAncestor);
   }
+  oldClosestCommonInclusiveAncestor = nullptr;
 }
 
 void AbstractRange::UpdateCommonAncestorIfNecessary() {
@@ -495,9 +495,8 @@ void AbstractRange::UpdateCommonAncestorIfNecessary() {
   nsINode* newCommonAncestor =
       GetClosestCommonInclusiveAncestor(AllowRangeCrossShadowBoundary::Yes);
   if (newCommonAncestor != oldCommonAncestor) {
-    if (oldCommonAncestor) {
-      UnregisterClosestCommonInclusiveAncestor(oldCommonAncestor, false);
-    }
+    UnregisterClosestCommonInclusiveAncestor();
+
     if (newCommonAncestor) {
       RegisterClosestCommonInclusiveAncestor(newCommonAncestor);
     } else {
diff --git a/dom/base/AbstractRange.h b/dom/base/AbstractRange.h
index c830f239e7d..ae9b6e13276 100644
--- a/dom/base/AbstractRange.h
+++ b/dom/base/AbstractRange.h
@@ -218,8 +218,7 @@ class AbstractRange : public nsISupports,
   /**
    * https://dom.spec.whatwg.org/#concept-tree-inclusive-ancestor
    */
-  void UnregisterClosestCommonInclusiveAncestor(nsINode* aNode,
-                                                bool aIsUnlinking);
+  void UnregisterClosestCommonInclusiveAncestor(bool aIsUnlinking = false);
 
   void UpdateCommonAncestorIfNecessary();
 
diff --git a/dom/base/nsRange.cpp b/dom/base/nsRange.cpp
index 73b1f121542..644a0ff588c 100644
--- a/dom/base/nsRange.cpp
+++ b/dom/base/nsRange.cpp
@@ -508,7 +508,9 @@ void nsRange::CharacterDataChanged(nsIContent* aContent,
       bool isCommonAncestor =
           IsInAnySelection() && mStart.Container() == mEnd.Container();
       if (isCommonAncestor) {
-        UnregisterClosestCommonInclusiveAncestor(mStart.Container(), false);
+        MOZ_DIAGNOSTIC_ASSERT(mStart.Container() ==
+                              mRegisteredClosestCommonInclusiveAncestor);
+        UnregisterClosestCommonInclusiveAncestor();
         RegisterClosestCommonInclusiveAncestor(newStart.Container());
       }
       if (mStart.Container()
@@ -546,8 +548,10 @@ void nsRange::CharacterDataChanged(nsIContent* aContent,
       bool isCommonAncestor =
           IsInAnySelection() && mStart.Container() == mEnd.Container();
       if (isCommonAncestor && !newStart.Container()) {
+        MOZ_DIAGNOSTIC_ASSERT(mStart.Container() ==
+                              mRegisteredClosestCommonInclusiveAncestor);
         // The split occurs inside the range.
-        UnregisterClosestCommonInclusiveAncestor(mStart.Container(), false);
+        UnregisterClosestCommonInclusiveAncestor();
         RegisterClosestCommonInclusiveAncestor(
             mStart.Container()->GetParentNode());
         newEnd.Container()
Loading diff…