High firefox Memory Corruption 🔧 Commit mapped

Overview

High
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionMemory safety bugs present in Firefox 135. Some of these bugs showed evidence of memory corruption and we presume that with enough effort some of these could have been exploited to run arbitrary code.
ComponentDOM
Bug ClassMemory Corruption
Tracker1943179
Fix commitd85b643e72b3 (firefox) +41/-25
CISA KEVNot listed
CreditedAndrew McCreight
Disclosed2025-02-18

Files Changed

  • dom/base/ContentIterator.h
  • dom/base/Selection.cpp
  • editor/libeditor/HTMLEditor.cpp
diff --git a/dom/base/ContentIterator.h b/dom/base/ContentIterator.h
index 67962d41d54..5a648de38c9 100644
--- a/dom/base/ContentIterator.h
+++ b/dom/base/ContentIterator.h
@@ -39,13 +39,15 @@ class ContentIteratorBase {
    * (https://dom.spec.whatwg.org/#concept-tree-inclusive-descendant) of
    * aRoot.
    */
-  virtual nsresult Init(nsINode* aRoot);
+  [[nodiscard]] virtual nsresult Init(nsINode* aRoot);
 
-  virtual nsresult Init(dom::AbstractRange* aRange);
-  virtual nsresult Init(nsINode* aStartContainer, uint32_t aStartOffset,
-                        nsINode* aEndContainer, uint32_t aEndOffset);
-  virtual nsresult Init(const RawRangeBoundary& aStart,
-                        const RawRangeBoundary& aEnd);
+  [[nodiscard]] virtual nsresult Init(dom::AbstractRange* aRange);
+  [[nodiscard]] virtual nsresult Init(nsINode* aStartContainer,
+                                      uint32_t aStartOffset,
+                                      nsINode* aEndContainer,
+                                      uint32_t aEndOffset);
+  [[nodiscard]] virtual nsresult Init(const RawRangeBoundary& aStart,
+                                      const RawRangeBoundary& aEnd);
 
   virtual void First();
   virtual void Last();
@@ -56,7 +58,7 @@ class ContentIteratorBase {
 
   bool IsDone() const { return !mCurNode; }
 
-  virtual nsresult PositionAt(nsINode* aCurNode);
+  [[nodiscard]] virtual nsresult PositionAt(nsINode* aCurNode);
 
  protected:
   enum class Order {
@@ -76,8 +78,8 @@ class ContentIteratorBase {
    * - aStartOffset and aEndOffset are valid for its container.
    * - The start point and the end point are in document order.
    */
-  nsresult InitInternal(const RawRangeBoundary& aStart,
-                        const RawRangeBoundary& aEnd);
+  [[nodiscard]] nsresult InitInternal(const RawRangeBoundary& aStart,
+                                      const RawRangeBoundary& aEnd);
 
   // Recursively get the deepest first/last child of aRoot.  This will return
   // aRoot itself if it has no children.
@@ -108,11 +110,11 @@ class ContentIteratorBase {
 
   void SetEmpty();
 
-  NodeType mCurNode;
-  NodeType mFirst;
-  NodeType mLast;
+  NodeType mCurNode = nullptr;
+  NodeType mFirst = nullptr;
+  NodeType mLast = nullptr;
   // See <https://dom.spec.whatwg.org/#concept-tree-inclusive-ancestor>.
-  NodeType mClosestCommonInclusiveAncestor;
+  NodeType mClosestCommonInclusiveAncestor = nullptr;
 
   Maybe<nsMutationGuard> mMutationGuard;
   Maybe<JS::AutoAssertNoGC> mAssertNoGC;
@@ -227,9 +229,9 @@ class ContentSubtreeIterator final : public SafeContentIteratorBase {
   /**
    * Not supported.
    */
-  virtual nsresult Init(nsINode* aRoot) override;
+  [[nodiscard]] virtual nsresult Init(nsINode* aRoot) override;
 
-  virtual nsresult Init(dom::AbstractRange* aRange) override;
+  [[nodiscard]] virtual nsresult Init(dom::AbstractRange* aRange) override;
 
   /**
    * Initialize the iterator with aRange that does correct things
@@ -252,11 +254,15 @@ class ContentSubtreeIterator final : public SafeContentIteratorBase {
    * Examples of what nodes will be returned can be found
    * at test_content_iterator_subtree_shadow_tree.html.
    */
-  nsresult InitWithAllowCrossShadowBoundary(dom::AbstractRange* aRange);
-  virtual nsresult Init(nsINode* aStartContainer, uint32_t aStartOffset,
-                        nsINode* aEndContainer, uint32_t aEndOffset) override;
-  virtual nsresult Init(const RawRangeBoundary& aStartBoundary,
-                        const RawRangeBoundary& aEndBoundary) override;
+  [[nodiscard]] nsresult InitWithAllowCrossShadowBoundary(
+      dom::AbstractRange* aRange);
+  [[nodiscard]] virtual nsresult Init(nsINode* aStartContainer,
+                                      uint32_t aStartOffset,
+                                      nsINode* aEndContainer,
+                                      uint32_t aEndOffset) override;
+  [[nodiscard]] virtual nsresult Init(
+      const RawRangeBoundary& aStartBoundary,
+      const RawRangeBoundary& aEndBoundary) override;
 
   void Next() override;
   void Prev() override;
@@ -265,7 +271,7 @@ class ContentSubtreeIterator final : public SafeContentIteratorBase {
   // Must override these because we don't do PositionAt
   void Last() override;
 
-  nsresult PositionAt(nsINode* aCurNode) override;
+  [[nodiscard]] nsresult PositionAt(nsINode* aCurNode) override;
 
   friend void ImplCycleCollectionTraverse(nsCycleCollectionTraversalCallback&,
                                           ContentSubtreeIterator&, const char*,
@@ -301,7 +307,7 @@ class ContentSubtreeIterator final : public SafeContentIteratorBase {
   /**
    * Callers must guarantee that mRange isn't nullptr and is positioned.
    */
-  nsresult InitWithRange();
+  [[nodiscard]] nsresult InitWithRange();
 
   // Returns the highest inclusive ancestor of aNode that's in the range
   // (possibly aNode itself).  Returns null if aNode is null, or is not itself
diff --git a/dom/base/Selection.cpp b/dom/base/Selection.cpp
index ff2cff76730..ab3f8ec7941 100644
--- a/dom/base/Selection.cpp
+++ b/dom/base/Selection.cpp
@@ -346,7 +346,10 @@ const nsTHashSet<const nsINode*>& SelectionNodeCache::MaybeCollect(
       const nsINode* endContainer =
           endRef.IsEndOfContainer() ? nullptr : endRef.Container();
       UnsafePreContentIterator iter;
-      iter.Init(range);
+      nsresult rv = iter.Init(range);
+      if (NS_FAILED(rv)) {
+        continue;
+      }
       for (; !iter.IsDone(); iter.Next()) {
         if (const nsINode* node = iter.GetCurrentNode()) {
           // Only collect start and end container if they are fully
@@ -1944,7 +1947,10 @@ nsresult Selection::SelectFrames(nsPresContext* aPresContext,
   }
 
   ContentSubtreeIterator subtreeIter;
-  subtreeIter.InitWithAllowCrossShadowBoundary(&aRange);
+  nsresult rv = subtreeIter.InitWithAllowCrossShadowBoundary(&aRange);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
   if (isFirstContentTextNode && !subtreeIter.IsDone() &&
       subtreeIter.GetCurrentNode() == startContent) {
     subtreeIter.Next();  // first content has already been handled.
diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp
index bb025bbb783..89093f579d7 100644
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -3539,7 +3539,11 @@ already_AddRefed<Element> HTMLEditor::GetSelectedElement(const nsAtom* aTagName,
   }
 
   PostContentIterator postOrderIter;
-  postOrderIter.Init(firstRange);
+  nsresult rv = postOrderIter.Init(firstRange);
+  if (NS_FAILED(rv)) {
+    aRv.Throw(rv);
+    return nullptr;
+  }
 
   RefPtr<Element> lastElementInRange;
   for (nsINode* lastNodeInRange = nullptr; !postOrderIter.IsDone();
Loading diff…