CVE-2025-1414

High CVSS 6.5 firefox Memory Corruption 🔧 Commit mapped

Overview

High

Severity

6.5

CVSS

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

CWECWE-787 (Out-of-bounds write)

CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:L

CISA KEVNot listed

CreditedAndrew McCreight

Disclosed2025-02-18

Background

`ContentIteratorBase`: the DOM base class that walks nodes in a subtree or range, with Init() entry points that must succeed before the iterator is traversed.
`nsresult`: Gecko’s status-code return type whose failure states are tested with the NS_FAILED macro.
`[[nodiscard]]`: a C++ attribute that makes the compiler warn when a function’s return value is silently ignored.
`RawRangeBoundary`: a lightweight (node, offset) pair describing a range endpoint that InitInternal validates before positioning the iterator.

Root Cause Analysis

Several callers in dom/base/Selection.cpp and editor/libeditor/HTMLEditor.cpp invoked the various ContentIteratorBase::Init overloads (and ContentSubtreeIterator::InitWithAllowCrossShadowBoundary) without inspecting the returned nsresult. When Init failed — for example on a detached or improperly ordered RawRangeBoundary, or after a mutation invalidated the range — the iterator’s cursor members were left in an invalid state, yet the caller proceeded straight into a for (; !iter.IsDone(); iter.Next()) traversal. Because the members mCurNode, mFirst, mLast, and mClosestCommonInclusiveAncestor were declared without initializers, a partially-failed Init could leave them holding uninitialized or stale NodeType pointers, so IsDone() and GetCurrentNode() returned garbage and the loop dereferenced freed or bogus node memory.

The fix marks every Init/PositionAt/InitInternal/InitWithRange entry point [[nodiscard]], so ignoring the status now fails to compile, and updates the three call sites to continue/return on NS_FAILED. Default member initializers (= nullptr) additionally guarantee the cursor is a safe sentinel even if a future path escapes early. Together these ensure no traversal ever runs on an unsuccessfully initialized iterator.

Key insight

The single mistake was treating iterator initialization as infallible and iterating on its result regardless of failure, letting uninitialized cursor pointers reach a live traversal loop; the fix forces callers to honor the failure code via [[nodiscard]] and hardens the cursor members with nullptr defaults so failure can no longer surface as memory corruption.

Attack Path

Shape a failing range Script constructs or mutates a DOM Selection (or an editor operation) so a range endpoint becomes detached, cross-shadow, or misordered, guaranteeing the next Init will return a failure nsresult.
Trigger the unchecked call site The attacker drives selection/frame recomputation or HTMLEditor::GetSelectedElement, causing iter.Init(range) / InitWithAllowCrossShadowBoundary to fail while its return value is discarded.
Enter traversal on a bad cursor Control falls into for (; !iter.IsDone(); iter.Next()) where mCurNode is uninitialized or stale, so IsDone() is false and iteration begins over invalid node pointers.
Force use-after-free / OOB read Freeing or reallocating the referenced nodes before or during the loop turns GetCurrentNode() dereferences into reads/writes of controlled freed memory, yielding memory corruption exploitable toward code execution.

Impact Assessment

An attacker who can run script in a content page gains a use-after-free / uninitialized-pointer dereference in the content process’s DOM/editor machinery, reachable purely through selection and editing operations. Successful exploitation corrupts heap memory that can be groomed to gain control of execution within the sandboxed content process, with no privileged preconditions beyond loading attacker-controlled markup and script.

Files Changed

dom/base/ContentIterator.h
dom/base/Selection.cpp
editor/libeditor/HTMLEditor.cpp

Audit Directions

Unchecked fallible init
Grep for iterator, parser, and range Init/Reset calls whose nsresult is discarded, especially inside or just before a while/for (; !x.IsDone(); ...) loop.
Uninitialized owning pointers
Flag class members holding NodeType/raw node pointers declared without default initializers, where a partially-completed setup could leave them dangling.
Missing `[[nodiscard]]` on fallible APIs
Audit virtual methods returning nsresult that lack [[nodiscard]], since overridable failure paths are the easiest for callers to silently ignore.

Patch

view commit

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…

References

Vendor advisory Fix commit Bug tracker NVD

On This Page