[3] SVG animVal released without detaching its owner back-pointer

HighWebCore SVG property modelUAF

Three code paths dropped the pointer. Only the destructor remembered to detach.

eb617fc

High. A back-pointer invariant enforced only in the destructor, while three ordinary code paths release the same member — and the released object is one script already holds. No privileged position needed; the attacker controls both the free and the dereference moment, with escalation past a crash gated on reclaiming the freed slot.

SVG attributes are reflected into script as animated-value objects, and the C++ objects behind them form a parent-child pair where the parent owns the child strongly and the child points back at the parent with a raw, unmanaged pointer. An attribute such as x on <rect> surfaces as an SVGAnimatedLength holding baseVal (the parsed attribute) and animVal (the value SMIL animation currently produces); the animVal sub-object is handed out to script, and its JS wrapper takes an independent strong reference. The invariant that makes this safe is that the raw back-pointer must be cleared before the parent can die.

The angle: any web page can hold a JS reference to an animVal object whose owner pointer aims into freed heap, then dereference it at a moment of its own choosing to get a virtual dispatch through attacker-groomable memory.

The commit message spells out the asymmetry:

SVGAnimatedValueProperty<T>::ensureAnimVal() creates the animVal SVGProperty with |this| as the raw SVGProperty::m_owner back-pointer. The destructor clears it via detach(), but stopAnimation() and instanceStopAnimationImpl() set m_animVal = nullptr without detaching first. Since the animVal is exposed to script via SVGAnimatedLength.animVal and the JS wrapper holds an independent Ref, it can outlive the SVGAnimatedValueProperty, leaving m_owner dangling. A later len.value read reaches SVGProperty::contextElement() through freed memory. SVGAnimatedPropertyList<T> has the identical pattern and is fixed the same way.

Source/WebCore/svg/properties/SVGAnimatedValueProperty.h

Source/WebCore/svg/properties/SVGAnimatedPropertyList.h

LayoutTests/svg/animations/animVal-detach-after-stopAnimation-crash.html

Patch Details

Two SVG property templates get a new detachAnimVal() helper that mirrors what their destructors already do. The three sites that dropped or overwrote the m_animVal RefPtr are changed: stopAnimation() and instanceStopAnimationImpl() previously did m_animVal = nullptr; and now call detachAnimVal(); instanceStartAnimationImpl() previously did a bare m_animVal = animated.animVal(); inside if (!this->isAnimating()) and now calls detachAnimVal() before the reassignment (the single-statement if becomes a braced block).

detachAnimVal() is if (RefPtr animVal = std::exchange(m_animVal, nullptr)) animVal->detach(); — it takes a strong reference to the outgoing property, clears the member, and calls SVGProperty::detach(), which nulls SVGProperty::m_owner and resets access/state. A regression test grabs rect.x.animVal mid-animation, removes the animation element and its ancestors, forces GC, then reads len.value.

Lifetime invariant enforced only in the destructor, while non-destructor release paths drop the last owning reference and leave a raw back-pointer in a still-live child.

Background

SVG animated properties. An SVG attribute such as x on <rect> is reflected to script as an SVGAnimatedLength with two sub-objects — baseVal (the parsed attribute value, read/write) and animVal (the value currently produced by SMIL animation, read-only). In WebCore these are the SVGAnimatedValueProperty<T> template (single value) and SVGAnimatedPropertyList<T> (list-valued attributes such as points).

SVGProperty / SVGPropertyOwner. SVGProperty is the base class of the script-visible value objects; it is ThreadSafeRefCounted, so any number of independent strong references can exist. It holds a SVGPropertyOwner* m_owner raw back-pointer, set by attach() (or by the constructor) and cleared by detach(). SVGProperty::contextElement() walks that back-pointer via the virtual SVGPropertyOwner::attributeContextElement() to find the owning SVGElement, which value getters need for unit resolution.

Ownership direction. The animated property owns its children strongly (Ref<PropertyType> m_baseVal, RefPtr<PropertyType> m_animVal), while the children point back weakly and unmanaged. Refcounting therefore keeps the child alive independently of the parent, and only an explicit detach() keeps the back-pointer consistent.

Bindings retention. When script reads rect.x.animVal, the JS wrapper created for the returned SVGLength holds its own Ref to the underlying SVGProperty, so its lifetime is governed by JS reachability and GC, not by the DOM tree.

SMIL start/stop. SVGAttributeAnimator drives startAnimation()/stopAnimation() on the animated property. startAnimation() calls ensureAnimVal() to lazily create the animVal; stopAnimation() runs when the animation ends or the <animate> element is removed from the document. instanceStartAnimationImpl()/instanceStopAnimationImpl() are the equivalents for <use> shadow instances, where the instance borrows the originating element's animVal object.

std::exchange. A single-expression read-and-replace — it returns the old value of a variable while storing a new one. It carries no thread-safety or atomicity guarantee; here it captures the outgoing pointer and clears the member in one step before operating on the old value.

Analysis

This is a use-after-free driven by a dangling raw back-pointer.

  ensureAnimVal()          stopAnimation()             ~SVGAnimatedValueProperty
  ───────────────          ───────────────             ─────────────────────────
  create(this, ReadOnly)   m_animVal = nullptr   (pre-fix: no detach())
    m_owner = this           JS wrapper Ref keeps      free(this)
                             the SVGLength alive          │
                             m_owner still = this ────────┘
                                     │
                    len.value ───────► contextElement()
                                         m_owner->attributeContextElement()
                                         └─► virtual call into freed memory

SVGAnimatedValueProperty<T>::ensureAnimVal() creates the animVal as PropertyType::create(this, SVGPropertyAccess::ReadOnly, ...), installing this as the created SVGProperty's raw m_owner (SVGProperty.h declares SVGPropertyOwner* m_owner { nullptr }; — unmanaged). The list variant creates it as ListType::create(m_baseVal, SVGPropertyAccess::ReadOnly), so the owner is the Ref<ListType> m_baseVal the animated property exclusively holds. The lifetime contract for that back-pointer was enforced in exactly one place: the destructor, if (m_animVal) m_animVal->detach();. But m_animVal is a RefPtr, and the animVal is ThreadSafeRefCounted<SVGProperty> handed out to script, so the bindings wrapper holds an independent Ref (the load-bearing premise here, consistent with the header and the added test's structure). Whenever stopAnimation() / instanceStopAnimationImpl() executed m_animVal = nullptr;, or instanceStartAnimationImpl() overwrote m_animVal, the animated property released its reference without clearing the outgoing property's m_owner — the destructor-only invariant was bypassed on every non-destructor release path.

After the release, the animVal survives on the wrapper's strong reference, still carrying m_owner pointing at the SVGAnimatedValueProperty. When that owner is subsequently destroyed — element torn down, wrapper collected — the pointee is freed, but nothing clears m_owner. SVGProperty::isAttached() still reports true, so SVGProperty::contextElement() takes the m_owner->attributeContextElement() branch: a virtual dispatch through a freed SVGPropertyOwner. SVGProperty::commitChange() has the same shape (m_owner->commitPropertyChange(this)), though it is the weaker route here — the animVal is created SVGPropertyAccess::ReadOnly and pre-fix release left m_access untouched, so read-only guards would normally reject setters on exactly the object left dangling. The contextElement() route is the one fully supported by SVGProperty.h.

The added test is a complete trigger recipe, and each step is doing work:

1. An <svg> contains <rect id="r" x="10"> with <animate attributeName="x">.
2. s.pauseAnimations(); s.setCurrentTime(0.5) puts the animation inside its active interval, so startAnimation() has run ensureAnimVal() and m_animVal exists with m_owner == this.
3. document.getElementById("r").x.animVal inside a separate grab() frame creates a JS wrapper holding an independent Ref<SVGLength> and lets intermediate wrappers fall off the stack so a conservative scan does not pin them.
4. a.remove(); r.remove(); s.remove() ends the animation, so stopAnimation() reaches if (!this->isAnimating()) and, pre-fix, executed m_animVal = nullptr; — the SVGLength survives with m_owner still pointing at the animated property.
5. GCController.collect() (or ordinary GC) collects the SVGRectElement wrapper and tears down the element and its property registry, freeing the SVGAnimatedValueProperty.
6. len.value calls SVGLength::valueForBindings(), which calls SVGProperty::contextElement(); m_owner is non-null so the freed object is dereferenced for a virtual attributeContextElement() call.

instanceStartAnimationImpl() offers a second route: mirroring an animation onto a <use> shadow instance overwrote a previously-created instance m_animVal without detaching it, stranding the same dangling back-pointer without needing an animation to stop.

On exploitability, this is fully web-reachable with an attacker-controlled dereference moment. The immediate observed effect is a virtual dispatch through a freed SVGPropertyOwner and a read of the returned const SVGElement*. If the freed SVGAnimatedValueProperty<SVGLength> lands in a groomable general-purpose heap size class — the supplied headers show no isolated-heap annotation either way — and the attacker reclaims the slot with controlled bytes, which is straightforward since the window between steps 4 and 6 is script-controlled, the vtable load in attributeContextElement() could yield a controlled indirect call. A weaker but potentially more reliable path: if the reclaiming object merely places a controlled pointer where SVGPropertyOwner's fields sit, the returned contextElement() could be treated as a valid SVGElement by the length-resolution code, which would provide a type-confused read anchored at an attacker-chosen address. Both are projections conditional on successful reclaim; absent reclaim, the result is a deterministic renderer crash triggerable at a moment of the attacker's choosing.

This vulnerability weakens memory safety inside the WebContent process. The security model assumption at stake is that a script-reachable DOM reflection object never retains a raw pointer to a C++ owner that can outlive it — WebKit encodes that assumption in SVGProperty::detach() and relies on it being called on every ownership transition. Before the fix, ordinary web content could hold a JS reference to an animVal whose m_owner pointed into freed heap and then dereference it on demand. An attacker who wins the reclaim would obtain a virtual dispatch and a pointer-typed read through attacker-groomed heap memory in the renderer, the standard starting point for building read/write primitives.

Insight

The structural tell is a class whose destructor performs cleanup that no other code path performs — ~SVGAnimatedValueProperty() did m_animVal->detach();, and three non-destructor sites released the same member without it. Whenever a destructor is the sole enforcer of a back-pointer invariant, every early-release site is a latent dangling-pointer bug; the fix's shape (extract the destructor's work into a named helper, call it from every release site) is the general remedy. Worth flagging as a follow-up rather than a defect in this patch: the instance paths operate on an animVal borrowed from the originating property via m_animVal = animated.animVal(), whose m_owner points at the other animated property. detachAnimVal() now calls detach() on that shared object, which would clear the originating property's animVal owner as a side effect; whether the originating element's animVal.value still resolves its context element after a <use> instance stops animating is a behavioural question the supplied context does not settle, and it deserves a targeted test.

The discovery angle reads as pattern auditing of destructor-versus-setter asymmetry: the destructor already contained the detach, so grepping for other writes to m_animVal exposes three sites immediately. The test's construction — a separate grab() frame explicitly commented as avoiding conservative stack-scan pinning, plus a manual GCController.collect() — is a hand-built PoC written after reading the code, not a reduced fuzzer artifact.

Audit directions

• Invariant enforced only in a destructor. Nulling a raw back-pointer, deregistering, or unlinking happens in ~T() while other code paths release the last owning reference to the same child. The invariant is every ownership-drop site must run the same teardown the destructor runs. Narrow: grep Source/WebCore/svg/properties/ for assignments of m_animVal, m_baseVal, or any RefPtr<SVG*Property> member that are not routed through a detach helper — SVGAnimatedProperty.h, SVGValuePropertyList.h, and SVGPropertyList.h are the immediate neighbours; the tell is a member release statement whose type also appears inside the class's destructor body next to a ->detach() call. Wider: the same shape appears with any manual teardown call — clear(), invalidate(), unregister(), removeClient(), setParent(nullptr) — where the destructor calls it but a setter or reset path does not; search WebCore for classes holding RefPtr<X> m_child where X stores a raw parent pointer, and diff the destructor body against every other write to that member. Widest: the reusable invariant is destructor-only cleanup is a bug whenever the object can be released outside destruction; it applies to any refcounted/GC'd codebase with parent-child back-pointers — Chromium's blink::Member/WeakMember pairs, Rust's Rc/Weak graphs where Weak is replaced by a raw index, Objective-C unsafe_unretained delegate fields. Match tell: if a destructor does cleanup work X on a member, and any other line assigns or clears that same member, X is missing there.

• Other consumers of a stale SVGProperty::m_owner, not just contextElement(). SVGProperty.h shows two owner dereferences — contextElement() (m_owner->attributeContextElement()) and commitChange() (m_owner->commitPropertyChange(this)) — plus isAttached(), reattach(), and access()/isReadOnly() state that detach() also resets. Trace every script-reachable binding on SVGLength, SVGNumber, SVGPointList, SVGTransformList etc. that reaches commitChange() (setters on a still-attached-but-orphaned property), and check whether any path can mutate a property whose owner has been released. Match tell: a public binding method on an SVGProperty subclass that calls commitChange() without a preceding isAttached()/read-only guard.

• Owner-scoped teardown run by a borrower. An object is borrowed from one owner and stored in another owner's strong member, where teardown of the borrower runs owner-scoped cleanup on the borrowed object. Narrow: examine instanceStartAnimationImpl/instanceStopAnimationImpl in SVGAnimatedValueProperty.h and SVGAnimatedPropertyList.h — after the fix, the instance path calls detachAnimVal() on an animVal created by (and back-pointing at) the originating animated property, so the tell is a detach()/reset() applied to a member assigned from another object's accessor rather than constructed locally. Wider: the same shape shows up anywhere <use>-style instance mirroring, style-sharing, or cached-object sharing hands out an object that a second holder later tears down — search WebCore for members assigned from other.something() accessors and then cleaned up as if locally owned. Widest: the invariant is only the creating owner may run owner-scoped teardown on a shared object; borrowers may only drop their reference; it applies to any system with shared handles and non-idempotent close semantics — file descriptors passed between components, shared GPU resources, Rust types where Drop performs external deregistration on a cloned handle. Match tell: cleanup that mutates the shared object's state (rather than just dropping a reference) invoked from a code path that did not create it.

• Verify the teardown ordering assumption that makes this class observable. Script must be able to hold the child alive past the parent's death. Investigate which SVG DOM reflections hand out ThreadSafeRefCounted sub-objects to bindings — SVGAnimatedLength.animVal/.baseVal, list item accessors, SVGTransformList item getters — and for each, ask whether the JS wrapper's Ref can outlive the owning element's property registry. Match tell: a WebIDL-exposed getter returning a Ref<SVGProperty subclass> whose owner is a non-refcounted or element-scoped C++ object; those are exactly the objects for which a missed detach() becomes web-reachable rather than internal-only.