From 6eb376c4e511ff1e8ca1b802cec5cbbb5df7ab85 Mon Sep 17 00:00:00 2001
From: Tzarial <zork@google.com>
Date: Thu, 14 May 2026 21:11:30 -0700
Subject: [PATCH] [ui/compositor] Fix heap-use-after-free in LayerAnimationSequence

This CL fixes a heap-use-after-free bug in LayerAnimationSequence where
the sequence could be destroyed re-entrantly during its own execution.

The vulnerability existed in several methods (Progress, ProgressToEnd,
Abort, and Start) that call out to the LayerAnimationDelegate or its
LayerAnimationElements. These out-calls can trigger observers (e.g.,
aura::WindowObserver or LayoutManager) to re-enter the LayerAnimator
and stop or preempt the running sequence, causing it to be deleted.

When control returned to LayerAnimationSequence, it would continue
accessing member variables or iterating through its elements, resulting
in a use-after-free.

Changes:
- Added base::WeakPtr<LayerAnimationSequence> guards around all
  potentially re-entrant calls in LayerAnimationSequence::Progress,
  ProgressToEnd, Abort, and Start.
- The sequence now early-returns or breaks out of loops if it detects
  it has been destroyed during an out-call.
- Added a regression test ProgressToEndInWhileLoopFreesSequenceUAF in
  layer_animator_unittest.cc which specifically reproduces the UAF
  via a re-entrant StopAnimatingProperty call.

Fixed: 501524262
Change-Id: I6f7d68edbd0230770c4a2dc4472a4fde133a139b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7846156
Commit-Queue: Tzarial <zork@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1631050}
---

diff --git a/ui/compositor/layer_animation_sequence.cc b/ui/compositor/layer_animation_sequence.cc
index 6d793ff51..ffa98f8 100644
--- a/ui/compositor/layer_animation_sequence.cc
+++ b/ui/compositor/layer_animation_sequence.cc
@@ -57,7 +57,11 @@
          "indefinite amount of time without any actual animated content";
 
   elements_[0]->set_requested_start_time(start_time_);
+  base::WeakPtr<LayerAnimationSequence> alive(AsWeakPtr());
   elements_[0]->Start(delegate, animation_group_id_);
+  if (!alive) {
+    return;
+  }
 
   NotifyStarted();
 
@@ -75,6 +79,8 @@
   if (last_element_ == 0)
     last_start_ = start_time_;
 
+  base::WeakPtr<LayerAnimationSequence> alive(AsWeakPtr());
+
   const base::TimeDelta total_duration = GetTotalDurationOfAllElements();
   const auto animation_should_progress = [this, total_duration]() {
     // A repeating animation with zero total duration results in an infinite
@@ -95,6 +101,11 @@
     // Let the element we're passing finish.
     if (elements_[current_index]->ProgressToEnd(delegate))
       redraw_required = true;
+
+    if (!alive) {
+      return;
+    }
+
     last_start_ += element_duration;
     ++last_element_;
     last_progressed_fraction_ =
@@ -109,15 +120,22 @@
       animation_group_id_ = cc::AnimationIdProvider::NextGroupId();
       elements_[current_index]->Start(delegate, animation_group_id_);
     }
-    base::WeakPtr<LayerAnimationSequence> alive(AsWeakPtr());
-    if (elements_[current_index]->Progress(now, delegate))
-      redraw_required = true;
-    if (!alive)
+
+    if (!alive) {
       return;
+    }
+
+    if (elements_[current_index]->Progress(now, delegate)) {
+      redraw_required = true;
+    }
+
+    if (!alive) {
+      return;
+    }
+
     last_progressed_fraction_ =
         elements_[current_index]->last_progressed_fraction();
   }
-
   // Since the delegate may be deleted due to the notifications below, it is
   // important that we schedule a draw before sending them.
   if (redraw_required)
@@ -166,10 +184,17 @@
   if (elements_.empty())
     return;
 
+  base::WeakPtr<LayerAnimationSequence> alive(AsWeakPtr());
+
   size_t current_index = last_element_ % elements_.size();
   while (current_index < elements_.size()) {
     if (elements_[current_index]->ProgressToEnd(delegate))
       redraw_required = true;
+
+    if (!alive) {
+      return;
+    }
+
     last_progressed_fraction_ =
         elements_[current_index]->last_progressed_fraction();
     ++current_index;
@@ -199,9 +224,13 @@
 }
 
 void LayerAnimationSequence::Abort(LayerAnimationDelegate* delegate) {
+  base::WeakPtr<LayerAnimationSequence> alive(AsWeakPtr());
   size_t current_index = last_element_ % elements_.size();
   while (current_index < elements_.size()) {
     elements_[current_index]->Abort(delegate);
+    if (!alive) {
+      return;
+    }
     ++current_index;
   }
   last_element_ = 0;
diff --git a/ui/compositor/layer_animator_unittest.cc b/ui/compositor/layer_animator_unittest.cc
index 53c9aa7..1b9d4fb648 100644
--- a/ui/compositor/layer_animator_unittest.cc
+++ b/ui/compositor/layer_animator_unittest.cc
@@ -3825,4 +3825,55 @@
   }
 }
 
+// Regression test for heap-use-after-free in LayerAnimationSequence::Progress.
+class PreemptingDelegate : public TestLayerAnimationDelegate {
+ public:
+  PreemptingDelegate() = default;
+  ~PreemptingDelegate() override = default;
+
+  void set_animator(LayerAnimator* animator) { animator_ = animator; }
+  void Arm() { armed_ = true; }
+
+  void SetBrightnessFromAnimation(float brightness,
+                                  PropertyChangeReason reason) override {
+    TestLayerAnimationDelegate::SetBrightnessFromAnimation(brightness, reason);
+    // Re-enter the animator to stop the sequence and trigger deletion.
+    if (!armed_ || reentered_ ||
+        reason != PropertyChangeReason::FROM_ANIMATION) {
+      return;
+    }
+    reentered_ = true;
+    animator_->StopAnimatingProperty(LayerAnimationElement::BRIGHTNESS);
+  }
+
+ private:
+  raw_ptr<LayerAnimator> animator_ = nullptr;
+  bool armed_ = false;
+  bool reentered_ = false;
+};
+
+TEST(LayerAnimatorTest, ProgressToEndInWhileLoopFreesSequenceUAF) {
+  scoped_refptr<LayerAnimator> animator = CreateDefaultTestAnimator(nullptr);
+  PreemptingDelegate delegate;
+  animator->SetDelegate(&delegate);
+  delegate.set_animator(animator.get());
+
+  // Create a two-element sequence where the first element is finished on
+  // Step().
+  LayerAnimationSequence* seq = new LayerAnimationSequence();
+  seq->AddElement(LayerAnimationElement::CreateBrightnessElement(
+      0.5f, base::Milliseconds(10)));
+  seq->AddElement(LayerAnimationElement::CreateBrightnessElement(
+      1.0f, base::Milliseconds(1000)));
+
+  animator->StartAnimation(seq);
+  ASSERT_TRUE(animator->is_animating());
+
+  base::TimeTicks start_time = animator->last_step_time();
+  delegate.Arm();
+
+  // Step far enough to finish the first element and trigger the UAF path.
+  animator->Step(start_time + base::Milliseconds(50));
+}
+
 }  // namespace ui

diff --git a/ui/compositor/layer_animator_unittest.cc b/ui/compositor/layer_animator_unittest.cc
index 53c9aa7..1b9d4fb648 100644
--- a/ui/compositor/layer_animator_unittest.cc
+++ b/ui/compositor/layer_animator_unittest.cc
@@ -3825,4 +3825,55 @@
   }
 }
 
+// Regression test for heap-use-after-free in LayerAnimationSequence::Progress.
+class PreemptingDelegate : public TestLayerAnimationDelegate {
+ public:
+  PreemptingDelegate() = default;
+  ~PreemptingDelegate() override = default;
+
+  void set_animator(LayerAnimator* animator) { animator_ = animator; }
+  void Arm() { armed_ = true; }
+
+  void SetBrightnessFromAnimation(float brightness,
+                                  PropertyChangeReason reason) override {
+    TestLayerAnimationDelegate::SetBrightnessFromAnimation(brightness, reason);
+    // Re-enter the animator to stop the sequence and trigger deletion.
+    if (!armed_ || reentered_ ||
+        reason != PropertyChangeReason::FROM_ANIMATION) {
+      return;
+    }
+    reentered_ = true;
+    animator_->StopAnimatingProperty(LayerAnimationElement::BRIGHTNESS);
+  }
+
+ private:
+  raw_ptr<LayerAnimator> animator_ = nullptr;
+  bool armed_ = false;
+  bool reentered_ = false;
+};
+
+TEST(LayerAnimatorTest, ProgressToEndInWhileLoopFreesSequenceUAF) {
+  scoped_refptr<LayerAnimator> animator = CreateDefaultTestAnimator(nullptr);
+  PreemptingDelegate delegate;
+  animator->SetDelegate(&delegate);
+  delegate.set_animator(animator.get());
+
+  // Create a two-element sequence where the first element is finished on
+  // Step().
+  LayerAnimationSequence* seq = new LayerAnimationSequence();
+  seq->AddElement(LayerAnimationElement::CreateBrightnessElement(
+      0.5f, base::Milliseconds(10)));
+  seq->AddElement(LayerAnimationElement::CreateBrightnessElement(
+      1.0f, base::Milliseconds(1000)));
+
+  animator->StartAnimation(seq);
+  ASSERT_TRUE(animator->is_animating());
+
+  base::TimeTicks start_time = animator->last_step_time();
+  delegate.Arm();
+
+  // Step far enough to finish the first element and trigger the UAF path.
+  animator->Step(start_time + base::Milliseconds(50));
+}
+
 }  // namespace ui