High firefox UAF 🔧 Commit mapped

Overview

High
Severity
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
CISA KEVNot listed
CreditedAtte Kettunen
Disclosed2025-02-04

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
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…