Critical CVSS 9.8 firefox UAF 🔧 Commit mapped

Overview

Critical
Severity
9.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactcritical
DescriptionAn attacker was able to achieve code execution in the content process by exploiting a use-after-free in Animation timelines. We have had reports of this vulnerability being exploited in the wild.
ComponentDOM
Bug ClassUAF
Tracker1923344
Fix commitf13df4bad528 (firefox) +33/-35
CWECWE-416 (Use-after-free)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H
CISA KEVNot listed
CreditedDamien Schaeffer from ESET
Disclosed2024-10-09

Background

Animation timeline
an object such as AnimationTimeline or its subclass DocumentTimeline that drives a set of Animation objects and advances them each frame.
`mAnimationOrder`
an intrusive LinkedList of raw Animation* pointers held by a timeline, whose iteration order defines the sampling order for ticking.
`Tick`
the per-frame method that walks mAnimationOrder and calls Animation::Tick on each animation, which may run script that mutates the very list being iterated.
Use-after-free (UAF)
a memory-safety flaw where code dereferences a pointer to an object that has already been freed, allowing an attacker to control the reused memory.

Root Cause Analysis

AnimationTimeline::Tick iterated mAnimationOrder in place, advancing to the next element via the raw intrusive LinkedListElement<Animation> pointer while calling Animation::Tick on each entry. Animation::Tick can dispatch events and run author script (or otherwise re-enter timeline code), which can remove and destroy Animation objects and mutate mAnimationOrder mid-traversal, so the raw iterator could dereference a freed node when computing getNext(). Because the loop only held bare Animation* pointers with no strong reference, nothing kept the currently-processed or next animation alive across the re-entrant callback, violating the invariant that a list node stays valid for the duration of its own iteration step.

The fix first snapshots the live set into an AutoTArray<RefPtr<Animation>, 32> (animationsToTick), taking a strong reference to every animation before any of them are ticked, and only then iterates the snapshot calling RemoveAnimation directly. The same snapshot-then-iterate pattern is applied to DocumentTimeline::TriggerAllPendingAnimationsNow, ScrollTimelineAnimationTracker::TriggerPendingAnimations, and the DocumentTimeline walk in nsRefreshDriver, and AnimationTimeline::RemoveAnimation is hardened to check isInList() together with the timeline-ownership condition before unlinking.

Key insight
The single core mistake was iterating a live intrusive linked list of raw pointers while each step could re-enter script and free the list’s nodes; the fix breaks that reentrancy hazard by copying every element into a strong-reference (RefPtr) array first, so freed-during-callback objects stay alive and the iterator never walks a dangling node.

Attack Path

  1. Register animations on a timeline Author script creates multiple Animation objects associated with a DocumentTimeline, populating the timeline’s mAnimationOrder linked list.
  2. Trigger a tick The refresh driver invokes nsRefreshDriver::UpdateAndReduceAnimations, which calls DocumentTimeline::WillRefresh and reaches AnimationTimeline::Tick, beginning in-place iteration over mAnimationOrder.
  3. Re-enter during Tick While Animation::Tick dispatches an animation event, attacker script runs and cancels/destroys animations (or reassigns their timeline), freeing the node the loop is about to advance through.
  4. Dereference freed node The loop dereferences the freed LinkedListElement<Animation> via getNext() (or ticks the freed Animation), causing a use-after-free.
  5. Gain control By grooming the heap so the freed slot is reoccupied with attacker-controlled data, the attacker steers the stale dereference toward controlled memory and achieves code execution in the content process.

Impact Assessment

An attacker gains use-after-free-driven memory corruption leading to arbitrary code execution inside the sandboxed content (renderer) process, reachable purely from web content that scripts animations. The only precondition is getting the victim to load a malicious page that registers animations and hooks their events to re-enter during a timeline tick; this vulnerability was reported as exploited in the wild.

Changed Functions

FunctionChangeNotes
for
dom/animation/AnimationTimeline.cpp
modified
for
dom/animation/DocumentTimeline.cpp
modified
for
layout/base/nsRefreshDriver.cpp
modified

Files Changed

  • dom/animation/AnimationTimeline.cpp
  • dom/animation/DocumentTimeline.cpp
  • dom/animation/ScrollTimelineAnimationTracker.cpp
  • layout/base/nsRefreshDriver.cpp

Audit Directions

  • Iterate-while-mutating
    Audit every loop over an intrusive LinkedList or hashset of raw pointers whose body can dispatch events or run script, and require a strong-reference snapshot before iteration.
  • Reentrant callbacks
    Trace callers of Animation::Tick, WillRefresh, and similar per-frame hooks for paths that re-enter DOM/animation code and free objects the caller still references.
  • Raw pointer containers
    Look for collections holding bare Animation*/DocumentTimeline* (e.g. mAnimationOrder, Timelines(), mPendingSet) that are walked without an owning RefPtr, as these are prime UAF candidates.
diff --git a/dom/animation/AnimationTimeline.cpp b/dom/animation/AnimationTimeline.cpp
index a684cd60cfe..9870dfac48c 100644
--- a/dom/animation/AnimationTimeline.cpp
+++ b/dom/animation/AnimationTimeline.cpp
@@ -40,41 +40,33 @@ AnimationTimeline::~AnimationTimeline() { mAnimationOrder.clear(); }
 bool AnimationTimeline::Tick(TickState& aState) {
   bool needsTicks = false;
 
-  nsTArray<Animation*> animationsToRemove;
-
-  for (Animation* animation = mAnimationOrder.getFirst(); animation;
-       animation =
-           static_cast<LinkedListElement<Animation>*>(animation)->getNext()) {
+  AutoTArray<RefPtr<Animation>, 32> animationsToTick;
+  for (Animation* animation : mAnimationOrder) {
     MOZ_ASSERT(mAnimations.Contains(animation),
                "The sampling order list should be a subset of the hashset");
     MOZ_ASSERT(!animation->IsHiddenByContentVisibility(),
                "The sampling order list should not contain any animations "
                "that are hidden by content-visibility");
+    animationsToTick.AppendElement(animation);
+  }
 
+  for (Animation* animation : animationsToTick) {
     // Skip any animations that are longer need associated with this timeline.
     if (animation->GetTimeline() != this) {
-      // If animation has some other timeline, it better not be also in the
-      // animation list of this timeline object!
-      MOZ_ASSERT(!animation->GetTimeline());
-      animationsToRemove.AppendElement(animation);
+      RemoveAnimation(animation);
       continue;
     }
 
     needsTicks |= animation->NeedsTicks();
-    // Even if |animation| doesn't need future ticks, we should still
-    // Tick it this time around since it might just need a one-off tick in
-    // order to dispatch events.
+    // Even if |animation| doesn't need future ticks, we should still Tick it
+    // this time around since it might just need a one-off tick in order to
+    // queue events.
     animation->Tick(aState);
-
     if (!animation->NeedsTicks()) {
-      animationsToRemove.AppendElement(animation);
+      RemoveAnimation(animation);
     }
   }
 
-  for (Animation* animation : animationsToRemove) {
-    RemoveAnimation(animation);
-  }
-
   return needsTicks;
 }
 
@@ -90,11 +82,12 @@ void AnimationTimeline::NotifyAnimationUpdated(Animation& aAnimation) {
 }
 
 void AnimationTimeline::RemoveAnimation(Animation* aAnimation) {
-  MOZ_ASSERT(!aAnimation->GetTimeline() || aAnimation->GetTimeline() == this);
-  if (static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList()) {
+  if (static_cast<LinkedListElement<Animation>*>(aAnimation)->isInList() &&
+      MOZ_LIKELY(!aAnimation->GetTimeline() ||
+                 aAnimation->GetTimeline() == this)) {
+    static_cast<LinkedListElement<Animation>*>(aAnimation)->remove();
     MOZ_ASSERT(mAnimations.Contains(aAnimation),
                "The sampling order list should be a subset of the hashset");
-    static_cast<LinkedListElement<Animation>*>(aAnimation)->remove();
   }
   mAnimations.Remove(aAnimation);
 }
diff --git a/dom/animation/DocumentTimeline.cpp b/dom/animation/DocumentTimeline.cpp
index 81fc2bcb4c4..2684195b887 100644
--- a/dom/animation/DocumentTimeline.cpp
+++ b/dom/animation/DocumentTimeline.cpp
@@ -160,7 +160,12 @@ void DocumentTimeline::NotifyAnimationUpdated(Animation& aAnimation) {
 }
 
 void DocumentTimeline::TriggerAllPendingAnimationsNow() {
+  AutoTArray<RefPtr<Animation>, 32> animationsToTrigger;
   for (Animation* animation : mAnimationOrder) {
+    animationsToTrigger.AppendElement(animation);
+  }
+
+  for (Animation* animation : animationsToTrigger) {
     animation->TryTriggerNow();
   }
 }
@@ -188,9 +193,6 @@ void DocumentTimeline::WillRefresh() {
   // of mDocument's PresShell.
   if (nsRefreshDriver* refreshDriver = GetRefreshDriver()) {
     refreshDriver->EnsureAnimationUpdate();
-  } else {
-    MOZ_ASSERT_UNREACHABLE(
-        "Refresh driver should still be valid at end of WillRefresh");
   }
 }
 
diff --git a/dom/animation/ScrollTimelineAnimationTracker.cpp b/dom/animation/ScrollTimelineAnimationTracker.cpp
index 48cad8cedd2..8ae26781e07 100644
--- a/dom/animation/ScrollTimelineAnimationTracker.cpp
+++ b/dom/animation/ScrollTimelineAnimationTracker.cpp
@@ -13,13 +13,10 @@ namespace mozilla {
 NS_IMPL_CYCLE_COLLECTION(ScrollTimelineAnimationTracker, mPendingSet, mDocument)
 
 void ScrollTimelineAnimationTracker::TriggerPendingAnimations() {
-  for (auto iter = mPendingSet.begin(), end = mPendingSet.end(); iter != end;
-       ++iter) {
-    dom::Animation* animation = *iter;
-
+  for (RefPtr<dom::Animation>& animation :
+       ToTArray<AutoTArray<RefPtr<dom::Animation>, 32>>(mPendingSet)) {
     MOZ_ASSERT(animation->GetTimeline() &&
                !animation->GetTimeline()->IsMonotonicallyIncreasing());
-
     // FIXME: Trigger now may not be correct because the spec says:
     // If a user agent determines that animation is immediately ready, it may
     // schedule the task (i.e. ResumeAt()) as a microtask such that it runs at
@@ -39,9 +36,7 @@ void ScrollTimelineAnimationTracker::TriggerPendingAnimations() {
       // inactive, and this also matches the current spec definition.
       continue;
     }
-
-    // Note: Remove() is legitimately called once per entry during the loop.
-    mPendingSet.Remove(iter);
+    mPendingSet.Remove(animation);
   }
 }
 
diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp
index 8fbda10bc96..fd3f826df16 100644
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -2315,8 +2315,15 @@ void nsRefreshDriver::DetermineProximityToViewportAndNotifyResizeObservers() {
 }
 
 static CallState UpdateAndReduceAnimations(Document& aDocument) {
-  for (DocumentTimeline* timeline : aDocument.Timelines()) {
-    timeline->WillRefresh();
+  {
+    AutoTArray<RefPtr<DocumentTimeline>, 32> timelinesToTick;
+    for (DocumentTimeline* timeline : aDocument.Timelines()) {
+      timelinesToTick.AppendElement(timeline);
+    }
+
+    for (DocumentTimeline* tl : timelinesToTick) {
+      tl->WillRefresh();
+    }
   }
 
   if (nsPresContext* pc = aDocument.GetPresContext()) {
@@ -2346,7 +2353,8 @@ void nsRefreshDriver::UpdateAnimationsAndSendEvents() {
     // [1]:
     // https://drafts.csswg.org/web-animations-1/#update-animations-and-send-events
     nsAutoMicroTask mt;
-    UpdateAndReduceAnimations(*mPresContext->Document());
+    RefPtr doc = mPresContext->Document();
+    UpdateAndReduceAnimations(*doc);
   }
 
   // Hold all AnimationEventDispatcher in mAnimationEventFlushObservers as
Loading diff…