Moderate firefox Logic Error 🔧 Commit mapped

Overview

Moderate
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionFrame poisoning in the Layout component
ComponentLayout
Bug ClassLogic Error
Tracker2029784
Fix commit315a7d55313c (firefox) +74/-7
CISA KEVNot listed
CreditedMozilla
Disclosed2026-10-13

Changed Functions

FunctionChangeNotes
if
layout/generic/nsFrameSelection.cpp
modified
while
layout/generic/nsFrameSelection.cpp
modified
if
testing/web-platform/tests/input-events/table-selection-crash.html
modified

Files Changed

  • layout/generic/nsFrameSelection.cpp
  • testing/web-platform/tests/input-events/table-selection-crash.html
diff --git a/layout/generic/nsFrameSelection.cpp b/layout/generic/nsFrameSelection.cpp
index 6e62d95df26..38bae54ab54 100644
--- a/layout/generic/nsFrameSelection.cpp
+++ b/layout/generic/nsFrameSelection.cpp
@@ -2771,8 +2771,11 @@ nsresult nsFrameSelection::TableSelection::UnselectCells(
     bool aRemoveOutsideOfCellRange, mozilla::dom::Selection& aNormalSelection) {
   MOZ_ASSERT(aNormalSelection.Type() == SelectionType::eNormal);
 
-  nsTableWrapperFrame* tableFrame =
+  nsTableWrapperFrame* tableFrameCast =
       do_QueryFrame(aTableContent->GetPrimaryFrame());
+  // Declared as weak frame, as the frame may be freed during a call to a
+  // listener.
+  AutoWeakFrame tableFrame(tableFrameCast);
   if (!tableFrame) {
     return NS_ERROR_FAILURE;
   }
@@ -2804,6 +2807,9 @@ nsresult nsFrameSelection::TableSelection::UnselectCells(
             curColIndex < minColIndex || curColIndex > maxColIndex) {
           aNormalSelection.RemoveRangeAndUnselectFramesAndNotifyListeners(
               *range, IgnoreErrors());
+          if (!tableFrame.IsAlive()) {
+            return NS_OK;
+          }
           // Since we've removed the range, decrement pointer to next range
           mSelectedCellIndex--;
         }
@@ -2811,15 +2817,18 @@ nsresult nsFrameSelection::TableSelection::UnselectCells(
       } else {
         // Remove cell from selection if it belongs to the given cells range or
         // it is spanned onto the cells range.
+
+        auto tableFrameStatic =
+            static_cast<nsTableWrapperFrame*>(tableFrame.GetFrame());
         nsTableCellFrame* cellFrame =
-            tableFrame->GetCellFrameAt(curRowIndex, curColIndex);
+            tableFrameStatic->GetCellFrameAt(curRowIndex, curColIndex);
 
         uint32_t origRowIndex = cellFrame->RowIndex();
         uint32_t origColIndex = cellFrame->ColIndex();
         uint32_t actualRowSpan =
-            tableFrame->GetEffectiveRowSpanAt(origRowIndex, origColIndex);
+            tableFrameStatic->GetEffectiveRowSpanAt(origRowIndex, origColIndex);
         uint32_t actualColSpan =
-            tableFrame->GetEffectiveColSpanAt(curRowIndex, curColIndex);
+            tableFrameStatic->GetEffectiveColSpanAt(curRowIndex, curColIndex);
         if (origRowIndex <= static_cast<uint32_t>(maxRowIndex) &&
             maxRowIndex >= 0 &&
             origRowIndex + actualRowSpan - 1 >=
@@ -2830,6 +2839,9 @@ nsresult nsFrameSelection::TableSelection::UnselectCells(
                 static_cast<uint32_t>(minColIndex)) {
           aNormalSelection.RemoveRangeAndUnselectFramesAndNotifyListeners(
               *range, IgnoreErrors());
+          if (!tableFrame.IsAlive()) {
+            return NS_OK;
+          }
           // Since we've removed the range, decrement pointer to next range
           mSelectedCellIndex--;
         }
@@ -2864,8 +2876,11 @@ static nsresult AddCellsToSelection(const nsIContent* aTableContent,
                                     Selection& aNormalSelection) {
   MOZ_ASSERT(aNormalSelection.Type() == SelectionType::eNormal);
 
-  nsTableWrapperFrame* tableFrame =
+  nsTableWrapperFrame* tableFrameCast =
       do_QueryFrame(aTableContent->GetPrimaryFrame());
+  // Declared as weak frame, as the frame may be freed during a call to a
+  // listener.
+  AutoWeakFrame tableFrame(tableFrameCast);
   if (!tableFrame) {  // Check that |table| is a table.
     return NS_ERROR_FAILURE;
   }
@@ -2875,7 +2890,11 @@ static nsresult AddCellsToSelection(const nsIContent* aTableContent,
   while (true) {
     uint32_t col = aStartColumnIndex;
     while (true) {
-      nsTableCellFrame* cellFrame = tableFrame->GetCellFrameAt(row, col);
+      auto cellFrame =
+          tableFrame.IsAlive()
+              ? static_cast<nsTableWrapperFrame*>(tableFrame.GetFrame())
+                    ->GetCellFrameAt(row, col)
+              : nullptr;
 
       // Skip cells that are spanned from previous locations or are already
       // selected
@@ -2884,7 +2903,7 @@ static nsresult AddCellsToSelection(const nsIContent* aTableContent,
         uint32_t origCol = cellFrame->ColIndex();
         if (origRow == row && origCol == col && !cellFrame->IsSelected()) {
           result = SelectCellElement(cellFrame->GetContent(), aNormalSelection);
-          if (NS_FAILED(result)) {
+          if (NS_FAILED(result) || !tableFrame.IsAlive()) {
             return result;
           }
         }
diff --git a/testing/web-platform/tests/input-events/table-selection-crash.html b/testing/web-platform/tests/input-events/table-selection-crash.html
new file mode 100644
index 00000000000..0979c32266e
--- /dev/null
+++ b/testing/web-platform/tests/input-events/table-selection-crash.html
@@ -0,0 +1,48 @@
+<!doctype html>
+<html class="reftest-wait">
+<meta charset="utf-8">
+<title>Listener script that removes a table while a cell is being selected does not crash the process.</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=2029784">
+
+<div id="host" contenteditable="true">
+<table id="tbl">
+<tbody><tr><td id="c00">aaaa</td></tr>
+<tr><td id="c10">bbbb</td></tr>
+</tbody></table>
+</div>
+<script>
+let n = 0;
+window.onload = () => {
+  document.body.offsetHeight;
+  let sel = getSelection();
+  host.focus();
+  sel.collapse(c00.firstChild, 0);
+  sel.modify("move", "forward", "character");
+  sel.modify("extend", "forward", "line");
+};
+window.onmousedown = () => {
+  setTimeout(()=>getSelection().modify("extend","forward","line"),0);
+}
+
+async_test(t => {
+  document.addEventListener("selectstart", (e) => {
+    n++;
+    e.preventDefault();
+    if (n == 3) {
+      tbl.style.display = "none";
+      document.body.offsetHeight;
+    }
+  });
+  new test_driver.Actions()
+      .pointerMove(0, 0, { origin: c00 })
+      .pointerDown()
+      .send();
+  document.documentElement.removeAttribute('class');
+}, "Select and remove the table does not crash.");
+</script>
+</html>
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/testing/web-platform/tests/input-events/table-selection-crash.html b/testing/web-platform/tests/input-events/table-selection-crash.html
new file mode 100644
index 00000000000..0979c32266e
--- /dev/null
+++ b/testing/web-platform/tests/input-events/table-selection-crash.html
@@ -0,0 +1,48 @@
+<!doctype html>
+<html class="reftest-wait">
+<meta charset="utf-8">
+<title>Listener script that removes a table while a cell is being selected does not crash the process.</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=2029784">
+
+<div id="host" contenteditable="true">
+<table id="tbl">
+<tbody><tr><td id="c00">aaaa</td></tr>
+<tr><td id="c10">bbbb</td></tr>
+</tbody></table>
+</div>
+<script>
+let n = 0;
+window.onload = () => {
+  document.body.offsetHeight;
+  let sel = getSelection();
+  host.focus();
+  sel.collapse(c00.firstChild, 0);
+  sel.modify("move", "forward", "character");
+  sel.modify("extend", "forward", "line");
+};
+window.onmousedown = () => {
+  setTimeout(()=>getSelection().modify("extend","forward","line"),0);
+}
+
+async_test(t => {
+  document.addEventListener("selectstart", (e) => {
+    n++;
+    e.preventDefault();
+    if (n == 3) {
+      tbl.style.display = "none";
+      document.body.offsetHeight;
+    }
+  });
+  new test_driver.Actions()
+      .pointerMove(0, 0, { origin: c00 })
+      .pointerDown()
+      .send();
+  document.documentElement.removeAttribute('class');
+}, "Select and remove the table does not crash.");
+</script>
+</html>
Loading diff…