CVE-2024-27856

Overview

Impact:

Description: Processing a file may lead to unexpected app termination or arbitrary code execution

Researchers: Maksymilian Motyl of Immunity Systems, Junsung Lee working with Trend Micro Zero Day Initiative, and ajajfxhj

Attribute	Value
CVE	CVE-2024-27856
Bugzilla	268765
Component	WebCore
Bug Class	LogicError
Severity	critical
Commit	0d0caf9579718608…
Advisory	Apple Advisory

Root Cause Analysis

The issue was addressed with improved checks.

Files Changed

Source Files

Source/WebCore/dom/ContainerNode.cpp

Test Files

LayoutTests/fast/dom/set-attribute-and-normalize-in-event-expected.txt
LayoutTests/fast/dom/set-attribute-and-normalize-in-event.html

Patch Preview

diff --git a/LayoutTests/fast/dom/set-attribute-and-normalize-in-event-expected.txt b/LayoutTests/fast/dom/set-attribute-and-normalize-in-event-expected.txt
new file mode 100644
index 000000000000..74c1f9350acf
--- /dev/null
+++ b/LayoutTests/fast/dom/set-attribute-and-normalize-in-event-expected.txt
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: RangeError: Maximum call stack size exceeded.
+CONSOLE MESSAGE: RangeError: Maximum call stack size exceeded.
+
diff --git a/LayoutTests/fast/dom/set-attribute-and-normalize-in-event.html b/LayoutTests/fast/dom/set-attribute-and-normalize-in-event.html
new file mode 100644
index 000000000000..5ec7cdccafa5
--- /dev/null
+++ b/LayoutTests/fast/dom/set-attribute-and-normalize-in-event.html
@@ -0,0 +1,17 @@
+<script>
+    function runTest() {
+        if (window.testRunner)
+            window.testRunner.dumpAsText();
+
+        marqueeElement.addEventListener("DOMSubtreeModified", () => {
+            try { hrElement.before(hrElement); } catch (e) { }
+            marqueeElement.normalize();
+        });
+
+        marqueeElement.setAttribute("a", "");
+    }
+</script>
+
+<body onload=runTest()>
+    <marquee id="marqueeElement">
+        <hr id="hrElement" width="1"></hr>
diff --git a/Source/WebCore/dom/ContainerNode.cpp b/Source/WebCore/dom/ContainerNode.cpp
index 30c7e9e66ba8..d3372f8d09d1 100644
--- a/Source/WebCore/dom/ContainerNode.cpp
+++ b/Source/WebCore/dom/ContainerNode.cpp
@@ -890,10 +890,11 @@ ExceptionOr<void> ContainerNode::appendChildWithoutPreInsertionValidityCheck(Nod
 
 ExceptionOr<void> ContainerNode::insertChildrenBeforeWithoutPreInsertionValidityCheck(NodeVector&& newChildren, Node* nextChild)
 {
+    RefPtr refChild = nextChild;
     for (auto& child : newChildren) {
         if (RefPtr oldParent = child->parentNode()) {
-            if (nextChild == child.ptr())
-                nextChild = child->nextSibling();
+            if (refChild.get() == child.ptr())
+                refChild = child->nextSibling();
             if (auto result = oldParent->removeChild(child); result.hasException())
                 return result.releaseException();
         }
@@ -910,14 +911,14 @@ ExceptionOr<void> ContainerNode::insertChildrenBeforeWithoutPreInsertionValidity
 
     ChildListMutationScope mutation(*this);
     for (auto& child : newChildren) {
-        if (nextChild && nextChild->parentNode() != this) // Event listeners moved nextChild elsewhere.
+        if (refChild && refChild->parentNode() != this) // Event listeners moved nextChild elsewhere.
             break;
         if (child->parentNode()) // Event listeners inserted this child elsewhere.
             break;
-        executeNodeInsertionWithScriptAssertion(*this, child.get(), nextChild, ChildChange::Source::API, ReplacedAllChildren::No, [&] {
+        executeNodeInsertionWithScriptAssertion(*this, child.get(), refChild.get(), ChildChange::Source::API, ReplacedAllChildren::No, [&] {
             child->setTreeScopeRecursively(treeScope());
-            if (nextChild)
-                insertBeforeCommon(*nextChild, child.get());
+            if (refChild)
+                insertBeforeCommon(*refChild, child.get());
             else
                 appendChildCommon(child);
         });