[1] Use-after-free in Node::m_shadowIncludingRoot via destructor cascade

HighWebCore DOMUAF

Closing a page freed the <html> element out from under its own shadow trees.

9414a97

High. Ordinary page teardown — a document containing a media element — leaves live objects holding root pointers into freed memory, with no exotic script required to reach the state. What gates it below Critical is that turning the stale read into a controlled primitive depends on reclaiming the freed Element allocation with attacker-shaped data.

Every DOM node caches a pointer to the root of the tree it belongs to, so that asking a disconnected node "what is your root?" is a single load rather than a walk up the parent chain. That cache is m_shadowIncludingRoot, a raw Node* that Node::rootNode() dereferences unconditionally whenever the node is no longer in a tree scope — "in a tree scope" meaning reachable from a Document or a ShadowRoot. The contract the whole scheme rests on is that every code path which unlinks a node also re-derives that cache before anyone can read it again.

The angle: a page that loads a media element and then navigates away leaves shadow-tree nodes pointing at a freed root element, dereferenced later from an event-loop task inside the renderer.

When a document is torn down via Document::removedLastRef through removeDetachedChildrenInContainer, the <html> element is removed from the document. Since <html> is still in tree scope at this point, notifyChildNodeRemoved is called, which walks the entire subtree — including shadow roots — and sets m_shadowIncludingRoot to <html> for all descendants. This is correct at that moment.

Then <html> is freed when the loop's RefPtr releases it (children don't ref-count their parents — m_parentNode is CheckedPtr). This triggers a destructor cascade: ~ContainerNode(<html>) processes <body>, then ~ContainerNode(<body>) processes the <video> element, and so on. Each step calls resetShadowIncludingRoot() on the direct child, fixing that node's cache. However, since IsConnected and IsInShadowTree flags were cleared during the initial notifyChildNodeRemoved walk, isInTreeScope() returns false, so notifyChildNodeRemoved is skipped in this case. This means the shadow root and its descendants are never updated — m_shadowIncludingRoot of these nodes still point to the now-freed <html>.

Nodes kept alive by mechanisms other than JS wrappers — such as HTMLMediaElement which survives as an ActiveDOMObject — retain their shadow DOM with dangling m_shadowIncludingRoot pointers. When these nodes are subsequently used (e.g., VTT cue display tree updates via an event loop task), the stale pointer is dereferenced, causing a use-after-free.

No new tests since existing media tests such as media/track/webvtt-parser-does-not-leak.html would hit debug assertions without this fix, and this bug requires a node to be kept alive by C++ code.

Source/WebCore/dom/ContainerNodeAlgorithms.cpp

 void removeDetachedChildrenInContainer(ContainerNode& container)
 {
     ...
         node->setParentNode(nullptr);
         node->resetShadowIncludingRoot();
     ...
         node->setTreeScopeRecursively(Ref<Document> { container.document() });
         if (node->isInTreeScope())
             notifyChildNodeRemoved(container, *node);
+        else if (node->refCount() > 1)
+            node->updateShadowIncludingRootForSubtree();
         ASSERT_WITH_SECURITY_IMPLICATION(!node->isInTreeScope());
     }
 }

Source/WebCore/dom/Node.cpp

+void Node::updateShadowIncludingRootForSubtree()
+{
+    SUPPRESS_UNCOUNTED_LOCAL for (auto* current = this; current; current = NodeTraversal::next(*current, this)) {
+        current->updateShadowIncludingRoot();
+        SUPPRESS_UNCOUNTED_LOCAL if (auto* shadowRoot = current->shadowRoot())
+            shadowRoot->updateShadowIncludingRootForSubtree();
+    }
+}

Source/WebCore/dom/Node.h

+ void updateShadowIncludingRootForSubtree();

Patch Details

Two production changes plus one header declaration. In removeDetachedChildrenInContainer(), the existing if (node->isInTreeScope()) notifyChildNodeRemoved(container, *node); dispatch gains an else branch: when the detached child is not in a tree scope — so the full removal walk is skipped — but its refCount() > 1, meaning something other than the loop's own RefPtr still holds a reference, the new node->updateShadowIncludingRootForSubtree() runs.

That new method is defined in Node.cpp. It walks the node's own subtree with NodeTraversal::next(*current, this), calling the pre-existing updateShadowIncludingRoot() on each node, and — critically — recurses into current->shadowRoot() via shadowRoot->updateShadowIncludingRootForSubtree() so that shadow trees are covered too. Node.h adds the declaration next to movingSteps. The SUPPRESS_UNCOUNTED_LOCAL annotations are static-analysis suppressions for the raw Node* iteration locals, not part of the fix semantics. No new layout test is added.

Cached ancestor/root back-pointer whose maintenance walk is gated on a state flag that an earlier phase of the same teardown already cleared, so the repair pass silently skips part of the structure it was meant to fix.

Background

Where this lives. ContainerNodeAlgorithms.cpp holds the low-level node-removal machinery that sits underneath every DOM mutation and underneath document teardown. removeDetachedChildrenInContainer() is the fast teardown path used when a container is being destroyed: it unlinks each child (setNextSibling(nullptr), setParentNode(nullptr), resetShadowIncludingRoot()), reparents the tree scope to the document, and then conditionally runs the removal notification.

The cached root pointer. m_shadowIncludingRoot is a raw Node* on every Node — visible in SameSizeAsNode as void* shadowIncludingRoot — recording the root of the shadow-including tree the node belongs to. Node::rootNode() in NodeInlines.h reads treeScope().rootNode() when the node is in a tree scope, and dereferences *m_shadowIncludingRoot otherwise. The cache exists so that rootNode() is O(1) for disconnected nodes rather than a parent-chain walk.

Tree scope and isInTreeScope(). A node is "in a tree scope" when it is reachable from a Document or a ShadowRoot; the predicate is backed by the IsConnected and IsInShadowTree state flags on Node.

Two maintenance paths. notifyChildNodeRemoved() is the DOM removal notification entry point. It dispatches to notifyNodeRemovedFromDocument() or notifyNodeRemovedFromTree(), each of which iterates the removed subtree with NodeTraversal::next and recurses into currentNode->shadowRoot() — so it is the path that covers shadow trees. The other path is resetShadowIncludingRoot(), applied to the single detached child in removeDetachedChildrenInContainer().

Parent pointer strength and destructor cascade. m_parentNode is a CheckedPtr (class Node : public ... CanMakeCheckedPtr<Node>), so children do not keep parents alive; a parent is freed as soon as the last owning Ref/RefPtr goes away, even while children still exist. And because ~ContainerNode calls removeDetachedChildrenInContainer() on itself, freeing one container recursively frees its children depth-first, re-entering the same function at each level.

Lifetime beyond the tree. ActiveDOMObject is a lifetime mechanism by which certain objects — HTMLMediaElement among them — are kept alive by the script execution context independently of DOM tree membership or JS wrappers. Media elements and text-track rendering build their controls and cue display boxes inside a ShadowRoot with ShadowRootMode::UserAgent: not visible to page script, but a normal part of the shadow-including tree.

observabilityOfRemovedNode(). An existing helper in the same file that uses node.refCount() > 1 as the signal that a removed node may still be observable through an external RefPtr.

Analysis

The root cause is a predicate that answers two different questions and stops agreeing with itself mid-teardown. isInTreeScope() means "is this node live in the tree" — but the code also uses it, implicitly, as "does this node's cache still need fixing." Phase one of teardown clears the backing flags; phase two then reads them and concludes there is nothing to do.

  Phase 1: <html> detached, still in tree scope
  ────────────────────────────────────────────────────────
  notifyChildNodeRemoved(<html>)
    walks whole subtree INCLUDING shadow roots
      m_shadowIncludingRoot = <html>   (correct now)
    clears IsConnected / IsInShadowTree on every node

  RefPtr drops last ref to <html>  ──►  free(<html>)

  Phase 2: destructor cascade, flags already cleared
  ────────────────────────────────────────────────────────
  ~ContainerNode(<html>) ─► removeDetachedChildrenInContainer(<html>)
    <body>: resetShadowIncludingRoot()      ✓ repaired
            isInTreeScope() == false  ──►  walk SKIPPED
              <video>'s UA shadow root      ✗ still ──► freed <html>

The bug class is a use-after-free through a stale cached raw pointer. notifyChildNodeRemoved() is the only path that recursed into shadowRoot(); once the flags are down, that path is unreachable and the per-child resetShadowIncludingRoot() repairs exactly one node per cascade level. Everything hanging off a shadow root keeps pointing at storage that was freed one frame up the cascade.

What turns a transient inconsistency into an exploitable one is a subtree whose lifetime is not tied to the DOM. The commit names HTMLMediaElement, which survives teardown as an ActiveDOMObject, dragging its user-agent shadow tree — controls, cue display boxes — along with it. A later use from an event-loop task, which the commit attributes to VTT cue display-tree updates, dereferences the freed pointer. The commit-message claim that media/track/webvtt-parser-does-not-leak.html would trip debug assertions without the fix is relayed as-is; the diff contains no test changes, which is consistent with it.

The fix restores the invariant by running an explicit shadow-including subtree walk in exactly the skipped case, gated on refCount() > 1 so it only pays the cost when some external owner can actually observe the node afterward — the same observability heuristic already encoded a few dozen lines above in observabilityOfRemovedNode().

Worth noticing what that gate does to the contract. Nodes with refCount() == 1 still get dangling caches; they just die immediately, so nobody can look. The invariant is now "the cache is correct only for nodes anyone can still see" — weaker and more fragile than "the cache is always correct," and something a future change that extends a node's lifetime by a non-refcount mechanism could quietly falsify.

This vulnerability weakens memory safety inside the WebContent process. The security-model assumption at stake is that m_shadowIncludingRoot is always repaired to a live node before any node can outlive the tree it was detached from — rootNode() dereferences it unconditionally for out-of-tree nodes. Before the fix, ordinary page teardown of a document containing a media element with a user-agent shadow tree left dangling root pointers on nodes that continued to live and continued to be used from event-loop tasks. An attacker who arranged the teardown and then triggered a use of the stale pointer would obtain a use-after-free on a freed Element; if the freed allocation were reclaimed with attacker-shaped data, that could escalate toward a type-confused object read or worse within the renderer.

The REGRESSION prefix plus the ASSERT_WITH_SECURITY_IMPLICATION(!node->isInTreeScope()) already sitting at the end of the loop point at an assertion failure or ASan report on a debug bot rather than a targeted fuzzing campaign — most likely caught during existing test runs and root-caused by hand, with a variant-analysis flavour given the prefix suggests a prior change to the caching scheme.

Insight: m_parentNode being a CheckedPtr is what makes a parent's storage disappear underneath its own children mid-cascade. Any other cached upward pointer on Node inherits that hazard profile for free — the destruction order guarantees a window in which every non-owning upward edge is transiently an address rather than an object.

Audit directions

Denormalized caches of ancestor state whose invalidation is gated on a liveness predicate that an earlier teardown phase already mutated. The invariant at stake is the condition guarding a cache-repair pass must not be a value that the operation being repaired has already changed. Narrow: grep Source/WebCore/dom/ for other members maintained alongside m_shadowIncludingRoot and m_treeScope — check every write site of resetShadowIncludingRoot, updateShadowIncludingRoot, and setTreeScopeRecursively for whether the repair is reachable when isInTreeScope() is already false. Wider: the same shape appears wherever a cached back-pointer or memoized root/owner is refreshed inside a conditional keyed on connectivity flags — TreeScope::rootNode consumers, Node::rootNode, focus/selection anchors, and any RenderObject-side cached container pointer. Widest: this is the general "invalidation predicate consumes state the invalidating operation mutates" class, transferring to ORM dirty-tracking flags cleared before cascade delete, React memoization keyed on a value the effect itself resets, and incremental build systems whose staleness bit is cleared before dependents are visited. In code review, the narrow tell is a cache-fixing call inside an if (isInTreeScope()) / if (isConnected()) branch with no else; the wider tell is any repair pass whose guard reads a flag written earlier in the same call graph; the widest tell is the question "if I clear the dirty bit in phase 1, does phase 2 still know it has work to do?"
Recursive structural walks that cover shadow trees on one code path but not on the sibling path handling the same structure. The invariant is every traversal that maintains per-node state must have identical shadow-tree coverage to every other traversal maintaining that same state. Narrow: enumerate the functions in ContainerNodeAlgorithms.cpp that recurse via currentNode->shadowRoot() — notifyNodeInsertedIntoDocument, notifyNodeInsertedIntoTree, notifyNodeRemovedFromDocument, notifyNodeRemovedFromTree, and the new Node::updateShadowIncludingRootForSubtree — and diff their coverage against any other subtree walk over the same nodes (setTreeScopeRecursively, NodeTraversal::next loops in Document/ContainerNode) that does not descend into shadow roots. Wider: the same asymmetry shows up between composed-tree and node-tree iterators generally — ComposedTreeAncestorIterator vs ElementTraversal, slot-assignment updates vs light-DOM child iteration. Widest: the "two views over one structure with divergent reachability" class, carrying to scene graphs with detached layers, virtual-DOM reconciliation across portals, and filesystem walkers that differ on symlink and mount-point following. In code review, a NodeTraversal::next loop that maintains node state with no accompanying shadowRoot() recursion is the visual tell.
Strong-ownership asymmetry — a child holding a non-owning pointer to a parent whose destructor recursively destroys the child, so the parent's storage is dead while its descendants still run code. The invariant is no cached upward pointer may be read after the owner's destructor has begun. Narrow: in Node.h, m_parentNode is a CheckedPtr via CanMakeCheckedPtr<Node>; audit every other member on Node/ContainerNode/ShadowRoot that points upward or sideways (m_treeScope, m_shadowIncludingRoot, host pointers on ShadowRoot) for reads that can occur during a ~ContainerNode cascade — deletionHasBegun() assertions mark the boundary. Wider: any parent-owns-child hierarchy where children carry raw/checked back-references and the parent's teardown re-enters child code — RenderObject parent chains during render-tree destruction, Frame/FrameTree teardown, AXObject parent caches during cache invalidation. Widest: "in an owner-destroys-children hierarchy, destruction order makes every upward non-owning pointer transiently dangling — anything the destructor calls must not follow one", applying to Rust Weak<Parent> upgraded during Drop, Qt's QObject parent-child deletion, and DI containers disposing scopes. In code review, a read of an upward pointer inside a function reachable from a destructor is the tell.