Released December 11, 2024

CVE-2024-54534

CVSS 9.8 severity critical CWE-787 vector CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H

Overview

Impact:

Description: Processing maliciously crafted web content may lead to memory corruption

Researchers: Tashita Software Security

Key insight. Storing DOM Nodes as raw non-owning pointers in the touch/wheel EventTargetSet made memory safety depend on every teardown path perfectly pruning the set; the fix removes that fragile obligation by making the container hold auto-nulling weak references, turning a latent use-after-free into a structurally impossible one.

Patch Details

diff --git a/Source/WTF/wtf/WeakHashCountedSet.h b/Source/WTF/wtf/WeakHashCountedSet.h
index a4af95bf87b5..6aa449841455 100644
--- a/Source/WTF/wtf/WeakHashCountedSet.h
+++ b/Source/WTF/wtf/WeakHashCountedSet.h
@@ -64,6 +64,11 @@ public:
     bool remove(const ValueType&);
     bool remove(iterator);
 
+    // Removes the value, regardless of its count.
+    // Returns true if a value was removed.
+    bool removeAll(const ValueType&);
+    bool removeAll(iterator);
+
     // Clears the whole set.
     void clear() { m_impl.clear(); }
 
@@ -111,6 +116,22 @@ inline bool WeakHashCountedSet<Value, WeakPtrImpl>::remove(iterator it)
     return true;
 }
 
+template<typename Value, typename WeakPtrImpl>
+inline bool WeakHashCountedSet<Value, WeakPtrImpl>::removeAll(const ValueType& value)
+{
+    return removeAll(find(value));
+}
+
+template<typename Value, typename WeakPtrImpl>
+inline bool WeakHashCountedSet<Value, WeakPtrImpl>::removeAll(iterator it)
+{
+    if (it == end())
+        return false;
+
+    m_impl.remove(it);
+    return true;
+}
+
 template<typename Value, typename WeakMapImpl>
 size_t containerSize(const WeakHashCountedSet<Value, WeakMapImpl>& container) { return container.computeSize(); }
 
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index 81729be58ab5..4e901c60f3d1 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -3232,11 +3232,11 @@ void Document::willBeRemovedFromFrame()
     commonTeardown();
 
 #if ENABLE(TOUCH_EVENTS)
-    if (m_touchEventTargets && m_touchEventTargets->size() && parentDocument())
+    if (m_touchEventTargets && m_touchEventTargets->computeSize() && parentDocument())
         protectedParentDocument()->didRemoveEventTargetNode(*this);
 #endif
 
-    if (m_wheelEventTargets && m_wheelEventTargets->size() && parentDocument())
+    if (m_wheelEventTargets && m_wheelEventTargets->computeSize() && parentDocument())
         protectedParentDocument()->didRemoveEventTargetNode(*this);
 
     if (RefPtr mediaQueryMatcher = m_mediaQueryMatcher)
@@ -8363,7 +8363,7 @@ void Document::wheelEventHandlersChanged(Node* node)
     UNUSED_PARAM(node);
 #endif
 
-    bool haveHandlers = m_wheelEventTargets && !m_wheelEventTargets->isEmpty();
+    bool haveHandlers = m_wheelEventTargets && !m_wheelEventTargets->isEmptyIgnoringNullReferences();
     page->chrome().client().wheelEventHandlersChanged(haveHandlers);
 }
 
@@ -8372,7 +8372,7 @@ void Document::didAddWheelEventHandler(Node& node)
     if (!m_wheelEventTargets)
         m_wheelEventTargets = makeUnique<EventTargetSet>();
 
-    m_wheelEventTargets->add(&node);
+    m_wheelEventTargets->add(node);
     wheelEventHandlersChanged(&node);
 
     if (RefPtr frame = this->frame())
@@ -8383,9 +8383,9 @@ static bool removeHandlerFromSet(EventTargetSet& handlerSet, Node& node, EventHa
 {
     switch (removal) {
     case EventHandlerRemoval::One:
-        return handlerSet.remove(&node);
+        return handlerSet.remove(node);
     case EventHandlerRemoval::All:
-        return handlerSet.removeAll(&node);
+        return handlerSet.removeAll(node);
     }
     return false;
 }
@@ -8410,7 +8410,7 @@ unsigned Document::wheelEventHandlerCount() const
         return 0;
 
     unsigned count = 0;
-    for (auto& handler : *m_wheelEventTargets)
+    for (auto handler : *m_wheelEventTargets)
         count += handler.value;
 
     return count;
@@ -8422,7 +8422,7 @@ void Document::didAddTouchEventHandler(Node& handler)
     if (!m_touchEventTargets)
         m_touchEventTargets = makeUnique<EventTargetSet>();
 
-    m_touchEventTargets->add(&handler);
+    m_touchEventTargets->add(handler);
 
     if (RefPtr parent = parentDocument()) {
         parent->didAddTouchEventHandler(*this);
@@ -8453,15 +8453,15 @@ void Document::didRemoveEventTargetNode(Node& handler)
 {
 #if ENABLE(TOUCH_EVENTS)
     if (m_touchEventTargets) {
-        m_touchEventTargets->removeAll(&handler);
-        if ((&handler == this || m_touchEventTargets->isEmpty()) && parentDocument())
+        m_touchEventTargets->removeAll(handler);
+        if ((&handler == this || m_touchEventTargets->isEmptyIgnoringNullReferences()) && parentDocument())
             protectedParentDocument()->didRemoveEventTargetNode(*this);
     }
 #endif
 
     if (m_wheelEventTargets) {
-        m_wheelEventTargets->removeAll(&handler);
-        if ((&handler == this || m_wheelEventTargets->isEmpty()) && parentDocument())
+        m_wheelEventTargets->removeAll(handler);
+        if ((&handler == this || m_wheelEventTargets->isEmptyIgnoringNullReferences()) && parentDocument())
             protectedParentDocument()->didRemoveEventTargetNode(*this);
     }
 }
@@ -8473,7 +8473,7 @@ unsigned Document::touchEventHandlerCount() const
         return 0;
 
     unsigned count = 0;
-    for (auto& handler : *m_touchEventTargets)
+    for (auto handler : *m_touchEventTargets)
         count += handler.value;
 
     return count;
@@ -8545,12 +8545,11 @@ Document::RegionFixedPair Document::absoluteRegionForEventTargets(const EventTar
     Region targetRegion;
     bool insideFixedPosition = false;
 
-    for (auto& keyValuePair : *targets) {
-        if (RefPtr node = keyValuePair.key) {
-            auto targetRegionFixedPair = absoluteEventRegionForNode(*node);
-            targetRegion.unite(targetRegionFixedPair.first);
-            insideFixedPosition |= targetRegionFixedPair.second;
-        }
+    for (auto keyValuePair : *targets) {
+        Ref node = keyValuePair.key;
+        auto targetRegionFixedPair = absoluteEventRegionForNode(node);
+        targetRegion.unite(targetRegionFixedPair.first);
+        insideFixedPosition |= targetRegionFixedPair.second;
     }
 
     return RegionFixedPair(targetRegion, insideFixedPosition);
diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
index d4874fbbf3a3..3fe0bb244379 100644
--- a/Source/WebCore/dom/Document.h
+++ b/Source/WebCore/dom/Document.h
@@ -56,7 +56,6 @@
 #include <wtf/Deque.h>
 #include <wtf/FixedVector.h>
 #include <wtf/Forward.h>
-#include <wtf/HashCountedSet.h>
 #include <wtf/HashSet.h>
 #include <wtf/Logger.h>
 #include <wtf/ObjectIdentifier.h>
@@ -64,6 +63,7 @@
 #include <wtf/RobinHoodHashMap.h>
 #include <wtf/TriState.h>
 #include <wtf/UniqueRef.h>
+#include <wtf/WeakHashCountedSet.h>
 #include <wtf/WeakHashMap.h>
 #include <wtf/WeakHashSet.h>
 #include <wtf/WeakListHashSet.h>
@@ -356,7 +356,7 @@ enum class NodeListInvalidationType : uint8_t {
 const auto numNodeListInvalidationTypes = enumToUnderlyingType(NodeListInvalidationType::InvalidateOnAnyAttrChange) + 1;
 
 enum class EventHandlerRemoval : bool { One, All };
-using EventTargetSet = HashCountedSet<Node*>;
+using EventTargetSet = WeakHashCountedSet<Node, WeakPtrImplWithEventTargetData>;
 
 enum class DocumentCompatibilityMode : uint8_t {
     NoQuirksMode = 1,
@@ -1481,8 +1481,8 @@ public:
     WEBCORE_EXPORT unsigned styleRecalcCount() const;
 
 #if ENABLE(TOUCH_EVENTS)
-    bool hasTouchEventHandlers() const { return m_touchEventTargets.get() ? m_touchEventTargets->size() : false; }
-    bool touchEventTargetsContain(Node& node) const { return m_touchEventTargets ? m_touchEventTargets->contains(&node) : false; }
+    bool hasTouchEventHandlers() const { return m_touchEventTargets && m_touchEventTargets->computeSize(); }
+    bool touchEventTargetsContain(Node& node) const { return m_touchEventTargets && m_touchEventTargets->contains(node); }
 #else
     bool hasTouchEventHandlers() const { return false; }
     bool touchEventTargetsContain(Node&) const { return false; }
@@ -1516,7 +1516,7 @@ public:
 #endif
     }
 
-    bool hasWheelEventHandlers() const { return m_wheelEventTargets.get() ? m_wheelEventTargets->size() : false; }
+    bool hasWheelEventHandlers() const { return m_wheelEventTargets && m_wheelEventTargets->computeSize(); }
     const EventTargetSet* wheelEventTargets() const { return m_wheelEventTargets.get(); }

Background

Advisory: Apple Security Advisory

Attribute	Value
CVE	CVE-2024-54534
Bugzilla	277967
Component	WebKit
Bug Class	UAF
Severity	medium
Commit	ccef7b85cc9b91a1…
Advisory	Apple Advisory

Terminology

HashCountedSet — A WTF set that stores each element once along with an integer count of how many times it was added, here used to track how many handlers a Node has registered.
WeakHashCountedSet / WeakPtr — A counted set whose keys are weak references that automatically become null when the referenced object is destroyed, preventing stale-pointer dereference.
EventTargetSet — The Document-level structure (m_touchEventTargets, m_wheelEventTargets) recording every Node that currently has a touch or wheel event handler.
isEmptyIgnoringNullReferences / computeSize — Weak-container accessors that account for entries whose weak reference has already been nulled, so callers see only live members rather than a raw slot count.
Use-after-free — A memory-safety bug where a pointer is dereferenced after the object it referenced has been freed, allowing reads/writes of reallocated attacker-controlled memory.

Root Cause Analysis

The patch replaces the type of EventTargetSet in Document.h from HashCountedSet<Node*> (a counted set of raw, non-owning Node pointers) with WeakHashCountedSet<Node, WeakPtrImplWithEventTargetData> (a counted set of weak references). Document keeps two of these sets, m_touchEventTargets and m_wheelEventTargets, which record every Node in the document that has registered a touch or wheel event handler. The invariant these sets must uphold is that every entry corresponds to a live Node: the set holds only non-owning references, so an entry must be removed before or exactly when the referenced Node is destroyed. With raw Node* keys that invariant was enforced only by explicit bookkeeping calls (didRemoveEventTargetNode / removeHandlerFromSet), and any path where a handler-bearing Node was torn down without correctly pruning these sets left a dangling raw pointer. Subsequent operations that dereference the keys — most clearly Document::absoluteRegionForEventTargets, which before the patch did RefPtr node = keyValuePair.key and called absoluteEventRegionForNode(node), and the iteration loops in wheelEventHandlerCount/touchEventHandlerCount — would then read or ref-count freed memory, a use-after-free. The fix makes the container itself hold weak references: a destroyed Node’s slot is automatically nulled, so stale entries can never be dereferenced as live objects. Consequently the API surface changes throughout Document.cpp: add/remove/removeAll/contains now take Node& instead of Node, size() (which counted possibly-dead slots) is replaced by computeSize() and isEmpty() by isEmptyIgnoringNullReferences() so callers ask about live entries only, and absoluteRegionForEventTargets iterates by value taking a Ref node = keyValuePair.key directly (the weak set never yields a dead key). A new removeAll(const ValueType&)/removeAll(iterator) pair was added to WeakHashCountedSet because that method did not previously exist on the weak container. The fix restores the live-reference invariant structurally rather than relying on every teardown path to remember to prune, which is why it is fundamentally a memory-safety (UAF) fix despite the LogicError label.

Attack Path

1. Register handlers

From attacker-controlled web content, add touch and/or wheel event listeners to one or more DOM nodes so those nodes are inserted into Document::m_touchEventTargets / m_wheelEventTargets.

2. Trigger a teardown path that skips pruning

Manipulate the DOM (node removal, frame/document detachment, or a reentrant teardown during willBeRemovedFromFrame) so a handler-bearing Node is destroyed without its raw-pointer entry being correctly removed from the counted set, leaving a dangling Node* key.

3. Reoccupy the freed slot

Use standard heap grooming (allocate objects of the same size class as the freed Node) so the dangling pointer now points at attacker-influenced memory.

4. Force a dereference of the stale key

Cause the document to walk the target set — e.g. a hit-test / scrolling-region computation calling absoluteRegionForEventTargets, or wheelEventHandlerCount()/touchEventHandlerCount(), or hasTouchEventHandlers() — which reads and, in the pre-patch absoluteRegionForEventTargets, ref-counts the freed Node as if live.

5. Escalate

Convert the resulting use-after-free read/write of a Node object into a stronger primitive (type-confused vtable/refcount manipulation) toward code execution in the WebContent process; this escalation is standard exploitation background, not shown by the diff.

Impact Assessment

The primitive is a use-after-free on a Node object reachable from the WebContent (renderer) process purely through DOM manipulation and event-handler registration, so it is remotely reachable from a malicious web page. Pre-patch, absoluteRegionForEventTargets even performed a ref-count operation (RefPtr node = key) on the stale entry, giving both a dereference and a refcount write on freed memory, which is a comparatively strong UAF that heap grooming can steer toward type confusion and, with additional work, arbitrary code execution. Apple describes the observable effect as memory corruption. The bug is confined to the sandboxed WebContent process; achieving anything beyond a renderer compromise requires a separate sandbox-escape chain.

Audit Directions

Other raw-pointer counted/hash sets of DOM nodes in Document

Grep Document.h/.cpp and related DOM classes for HashCountedSet<Node*>, HashSet<Node*>, Vector<Node*> and similar non-owning containers keyed on Node*/Element*/EventTarget*; each is a candidate for the same dangling-key UAF and should either be weak or provably pruned on teardown.

size()/isEmpty() used as liveness on weak or soon-to-be-weak containers

Search for callers using ->size() or ->isEmpty() on event-target and observer sets where computeSize()/isEmptyIgnoringNullReferences() would be semantically required; a raw size() over a container with dead slots signals stale-entry logic bugs.

Iterators taking a reference to a key then dereferencing

Look for loops of the form RefPtr node = pair.key; … node or auto& over node containers without a liveness guarantee (grep absoluteEventRegionForNode-style callers and other absoluteForEventTargets helpers), which is the exact pattern that dereferenced the freed key here.

removeAll / prune-on-destroy obligations across WebCore observer registries

Audit registries where node destruction must call a manual removeAll/unregister (touch, wheel, IntersectionObserver, mutation, resize observers); enumerate teardown paths (detach, willBeRemovedFromFrame, reentrant document destruction) and confirm every one prunes, or migrate the container to a weak type.

Changed Functions

Function	File	Change	Note
`EventTargetSet (type alias)`	`Source/WebCore/dom/Document.h`	modified	Changed from HashCountedSet<Node*> to WeakHashCountedSet<Node, WeakPtrImplWithEventTargetData>, the core fix converting non-owning raw pointers to auto-nulling weak references.
`Document::hasTouchEventHandlers / touchEventTargetsContain / hasWheelEventHandlers`	`Source/WebCore/dom/Document.h`	modified	Use computeSize() and contains(node) on the weak set instead of size()/contains(&node), so liveness queries count only live entries.
`Document::willBeRemovedFromFrame`	`Source/WebCore/dom/Document.cpp`	modified	size() checks replaced with computeSize() for the weak container when deciding whether to notify the parent document.
`Document::wheelEventHandlersChanged`	`Source/WebCore/dom/Document.cpp`	modified	Uses isEmptyIgnoringNullReferences() instead of isEmpty() to ignore already-dead weak entries.
`Document::didAddWheelEventHandler / didAddTouchEventHandler`	`Source/WebCore/dom/Document.cpp`	modified	add(node) by reference instead of add(&node) by pointer, matching the weak-set API.
`removeHandlerFromSet`	`Source/WebCore/dom/Document.cpp`	modified	remove(node)/removeAll(node) by reference instead of by pointer.
`Document::wheelEventHandlerCount / touchEventHandlerCount`	`Source/WebCore/dom/Document.cpp`	modified	Iterate by value (auto handler) over the weak set; the weak container yields only live key/value pairs.
`Document::didRemoveEventTargetNode`	`Source/WebCore/dom/Document.cpp`	modified	removeAll(handler) by reference and isEmptyIgnoringNullReferences() checks before propagating to the parent document.
`Document::absoluteRegionForEventTargets`	`Source/WebCore/dom/Document.cpp`	modified	Iterates by value and takes Ref node = keyValuePair.key directly instead of RefPtr node = keyValuePair.key with a null check; the weak set guarantees a live key so the freed-pointer dereference path is eliminated.
`WeakHashCountedSet::removeAll(const ValueType&) / removeAll(iterator)`	`Source/WTF/wtf/WeakHashCountedSet.h`	added	New method (with declaration and inline definition) removing a value regardless of count, needed so the weak container can serve the same removeAll usage the raw HashCountedSet provided.