[3] SVGProperty animVal detach after stopAnimation

The destructor cleaned up the back-pointer. The reset paths forgot.

Severity: High | Component: WebCore SVG property model | eb617fc

Rated High because script can hold a Ref to an animVal SVGProperty past the destruction of its owning SVGAnimated*Property, and a subsequent len.value read then dereferences a dangling m_owner back-pointer through freed memory from web content; escalation to a controlled read requires reclaiming the freed owner slot under heap grooming, which the diff does not demonstrate, but the dangling read itself is reliably reachable.

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, via a detachAnimVal() helper that mirrors the destructor.

Source/WebCore/svg/properties/SVGAnimatedValueProperty.h

     void stopAnimation(SVGAttributeAnimator& animator) override
     {
         Base::stopAnimation(animator);
         if (!this->isAnimating())
-            m_animVal = nullptr;
+            detachAnimVal();
         else if (m_animVal)
             m_animVal->setValue(m_baseVal->value());
     }
...
     void instanceStartAnimationImpl(...) override
     {
-        if (!this->isAnimating())
+        if (!this->isAnimating()) {
+            detachAnimVal();
             m_animVal = animated.animVal();
+        }
         Base::startAnimation(animator);
     }
...
     void instanceStopAnimationImpl(SVGAttributeAnimator& animator) override
     {
         Base::stopAnimation(animator);
         if (!this->isAnimating())
-            m_animVal = nullptr;
+            detachAnimVal();
     }
+    void detachAnimVal()
+    {
+        // m_animVal may be retained by the bindings after we drop it. Detach it now so its
+        // raw SVGProperty::m_owner back-pointer cannot dangle once |this| is destroyed.
+        if (RefPtr animVal = std::exchange(m_animVal, nullptr))
+            animVal->detach();
+    }

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

+function step1() {
+    s.pauseAnimations(); s.setCurrentTime(0.5);   // enter active interval
+    len = grab();                                 // r.x.animVal, held by JS
+    a.remove(); r.remove(); s.remove();           // reaches stopAnimation() -> m_animVal=nullptr branch
+}
+function step2() {
+    GCController.collect();
+    len.value;                                    // SVGProperty::contextElement() through freed m_owner
+}

Patch Details

A detachAnimVal() helper is added to both SVGAnimatedValueProperty<T> and SVGAnimatedPropertyList<T>. It does std::exchange(m_animVal, nullptr) and, if a value was present, calls animVal->detach(), mirroring what the destructor already does. Three call sites that previously wrote m_animVal = nullptr directly (the non-animating branch of stopAnimation() and instanceStopAnimationImpl()) now call detachAnimVal() instead. In addition, instanceStartAnimationImpl() now calls detachAnimVal() before overwriting m_animVal = animated.animVal(), so a previously-held animVal is detached rather than silently dropped.

Failure to sever a raw back-pointer before dropping the smart-pointer that owns the pointee, when an independent reference lets the pointee outlive the back-pointer's target.

Background

SVG animated attributes are modelled by templated SVGAnimated*Property containers holding two SVGProperty objects: m_baseVal (the static author value, a Ref) and m_animVal (the presentation value while a SMIL/<animate> animation is active, a RefPtr). Each SVGProperty carries a raw m_owner back-pointer to its SVGAnimated*Property, used by SVGProperty::contextElement() to resolve the owning element for value conversions such as SVGLength::valueForBindings. detach() is the operation that clears m_owner. When script reads element.x.animVal, a JS wrapper is created around the m_animVal SVGProperty and holds its own Ref, so the property's lifetime is not controlled by the owner alone. In the SMIL lifecycle, entering an active interval calls startAnimation() (which populates m_animVal) and ending it calls stopAnimation() / instanceStopAnimationImpl(), which on the non-animating branch release m_animVal. Script can drive these transitions synchronously via pauseAnimations(), setCurrentTime(), and node remove().

Analysis

This is a use-after-free rooted in a lifetime/ownership violation. ensureAnimVal() creates the animVal with this as its raw m_owner. The owning container holds the animVal via RefPtr m_animVal, and the destructor calls m_animVal->detach() to sever the back-pointer before this dies. But stopAnimation() and instanceStopAnimationImpl() cleared the reference with m_animVal = nullptr without detaching first. Because the animVal is exposed to script and its JS wrapper holds an independent Ref to the same SVGProperty, dropping the owner's RefPtr does not destroy the property; it survives with a now-stale m_owner still pointing at the container. When the container and its owning element are subsequently destroyed, m_owner dangles.

The regression test drives it precisely: create an <animate> on an element's x, call pauseAnimations() and setCurrentTime(0.5) to enter the active interval so m_animVal exists, grab r.x.animVal into a JS variable (pinning the SVGProperty via the wrapper's independent Ref), then remove() the animate/rect/svg so stopAnimation() takes the !isAnimating() branch and drops the owner's RefPtr without detaching. After the owning container and element are freed, reading len.value flows through SVGProperty::contextElement() via the dangling m_owner. The immediate observed effect is a reliable web-content-triggerable dangling-pointer read. To weaponize, an attacker would reclaim the freed owner allocation with controlled data between the free and the .value read, so the m_owner dereference and the subsequent contextElement() pointer chain land on attacker-shaped memory.

This vulnerability weakens memory safety within the WebContent renderer. The security model assumes a SVGProperty's m_owner is valid whenever it is dereferenced (the owner outlives every reachable use, or the pointer is detached before the owner dies); before the fix, script could hold a Ref to an animVal past its owner's destruction. Any resulting primitive stays inside the renderer sandbox and would require a separate escape.

The same defect appears verbatim in two sibling template classes and is fixed identically, a strong signal of a copy-pasted teardown idiom across the SVG property family. The structural issue is that SVGProperty uses a raw back-pointer whose invalidation is manual, and the destructor was the only place that consistently performed it while the animation stop/restart paths open-coded m_animVal = nullptr.

Note: The wrapper holding an independent Ref and the exact contextElement()/valueForBindings dereference chain are inferred from the DOM model and the test rather than shown in the provided source; the missing detach on the reset paths is directly visible in the diff.

Audit directions

Raw back-pointer whose target can be freed while an independent reference keeps the pointee alive. Audit every assignment of nullptr (or overwrite) to m_animVal/m_baseVal-style RefPtrs in the SVG property tree and confirm each is preceded by detach(). Grep Source/WebCore/svg/properties/ for m_animVal = and = nullptr and compare each site against the class destructor; classes whose dtor calls detach() but whose reset paths do not are candidates.
Manual back-pointer invalidation that must mirror the destructor on every teardown path. Verify SVGProperty::detach() / m_owner clearing is invoked on all owner-release paths, not just ~SVGAnimated*Property. Examine SVGProperty::contextElement() and its callers (e.g. SVGLength::valueForBindings) to enumerate which bindings-reachable reads dereference m_owner, then trace back to every place the owner can die while a wrapper Ref persists.
Bindings wrapper holding an independent Ref that outlives the C++ owner. Audit other WebCore object families where a JS-exposed sub-object stores a raw back-pointer to its parent container; check that parent teardown detaches the child. Start with SVG list item properties (SVGListProperty/SVGPathSegList items) which follow the same owner/animVal ownership shape.
Investigate whether instanceStartAnimationImpl overwrite of m_animVal on shared/<use>-instance animations could previously drop a script-held animVal without detaching; the fix added a detachAnimVal() there too, suggesting instance-animation paths are a distinct trigger worth exercising with <use> cloned trees.