[Backport] CVE-2021-21204: Use after free in Blink.

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2799973:
Fix Mac crash due to use after free of BlinkScrollbarPartAnimation

What is happening is that the BlinkScrollbarPartAnimation instance
passed to BlinkScrollbarPartAnimationTimer is released while
the BlinkScrollbarPartAnimationTimer::TimerFired method runs as
part of BlinkScrollbarPartAnimation::setCurrentProgress call,
during the execution of ScrollbarPainter::setKnobAlpha which ends
up calling BlinkScrollbarPainterDelegate::setUpAlphaAnimation
through a chain of observers.
BlinkScrollbarPainterDelegate::setUpAlphaAnimation releases the
BlinkScrollbarPartAnimation instance which gets deallocated.
BlinkScrollbarPartAnimation::setCurrentProgress continues execution
after ScrollbarPainter::setKnobAlpha returns, but the _scrollbar
pointer is overwritten with garbage and when SetNeedsPaintInvalidation
is called the crash happens.

We retain self in BlinkScrollbarPartAnimation::setCurrentProgress
while it runs and release self before exit. By retaining self
Objective C runtime won't free BlinkScrollbarPartAnimation
while BlinkScrollbarPartAnimationTimer is running and the crash
should be avoided.

(cherry picked from commit 19207bea6bd8472aa4203db328fc7f51826956d4)

Bug: 1183276, 1189926, 1193025
Change-Id: Ibd5092a1dbae53bc21940c43883536624d1b03f3
Commit-Queue: Robert Flack <flackr@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#867587}
Commit-Queue: Liviu Tinta <liviutinta@chromium.org>
Cr-Commit-Position: refs/branch-heads/4430@{#979}
Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

1 files changed, 16 insertions, 1 deletions

diff --git a/chromium/third_party/blink/renderer/platform/mac/scroll_animator_mac.mm b/chromium/third_party/blink/renderer/platform/mac/scroll_animator_mac.mm
index 15990c4155f..d3f2d258575 100644
--- a/chromium/third_party/blink/renderer/platform/mac/scroll_animator_mac.mm
+++ b/chromium/third_party/blink/renderer/platform/mac/scroll_animator_mac.mm
@@ -334,6 +334,11 @@ class BlinkScrollbarPartAnimationTimer {
     double fraction = delta / duration_;
     fraction = clampTo(fraction, 0.0, 1.0);
     double progress = timing_function_->Evaluate(fraction, 0.001);
+    // In some scenarios, animation_ gets released during the call to
+    // setCurrentProgress. Because BlinkScrollbarPartAnimationTimer is a
+    // member variable of BlinkScrollbarPartAnimation animation_ the timer
+    // gets freed at the same time with animation_. In that case, it will
+    // not be safe to call any other code after animation_ setCurrentProgress.
     [animation_ setCurrentProgress:progress];
   }
 
@@ -407,6 +412,10 @@ class BlinkScrollbarPartAnimationTimer {
 
 - (void)setCurrentProgress:(NSAnimationProgress)progress {
   DCHECK(_scrollbar);
+  // In some scenarios, BlinkScrollbarPartAnimation is released in the middle
+  // of this method by _scrollbarPainter. This is why we have to retain the self
+  // pointer when we run this method.
+  [self retain];
 
   CGFloat currentValue;
   if (_startValue > _endValue)
@@ -433,7 +442,13 @@ class BlinkScrollbarPartAnimationTimer {
       break;
   }
 
-  _scrollbar->SetNeedsPaintInvalidation(invalidParts);
+  // Before BlinkScrollbarPartAnimation is released by _scrollbarPainter,
+  // invalidate is called and _scrollbar is set to nullptr. Check to see
+  // if _scrollbar is non-null before calling SetNeedsPaintInvalidation.
+  if (_scrollbar)
+    _scrollbar->SetNeedsPaintInvalidation(invalidParts);
+
+  [self release];
 }
 
 - (void)invalidate {