CVE-2026-17670
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifui/views/view.cc |
modified | |
TEST_Fui/views/view_unittest.cc |
modified | |
ParentLayerDestroyerui/views/view_unittest.cc |
modified |
Files Changed
ui/views/view.ccui/views/view_unittest.cc
Patch
From 6b0e66c1b6dafa6967898773ff9ccbc3dc2d0445 Mon Sep 17 00:00:00 2001
From: Keren Zhu <kerenzhu@chromium.org>
Date: Wed, 10 Jun 2026 12:21:06 -0700
Subject: [PATCH] views: protect against destroyed parent layer in View::OrphanLayers()
View::OrphanLayers() iterates through layers and removes them from their
parent. Layer::Remove() can synchronously trigger LayerAnimationObserver
callbacks, which may cause the parent layer to be deleted. This change
adds a WeakPtr check to ensure the parent layer is still valid before
attempting to remove subsequent layers.
There is no evidence that any existing observers does this but it is
good to be protected against.
Fixed: 513228974
Change-Id: I6c1272d225f43213d78cbd20057d717bb0aac69f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7914212
Commit-Queue: Keren Zhu <kerenzhu@chromium.org>
Reviewed-by: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1644805}
---
diff --git a/ui/views/view.cc b/ui/views/view.cc
index 87397d46..49dbdc2d 100644
--- a/ui/views/view.cc
+++ b/ui/views/view.cc
@@ -3766,7 +3766,14 @@
void View::OrphanLayers() {
if (layer()) {
if (ui::Layer* parent = layer()->parent()) {
+ base::WeakPtr<ui::Layer> weak_parent = layer()->parent()->AsWeakPtr();
for (ui::Layer* layer : GetLayersInOrder()) {
+ // Layer::Remove() will stop any layer animation on the parent, notify
+ // LayerAnimationObserver::OnLayerAnimationAborted(). If the observer
+ // deletes the layer, the weak_parent will become null.
+ if (!weak_parent) {
+ break;
+ }
parent->Remove(layer);
}
}
diff --git a/ui/views/view_unittest.cc b/ui/views/view_unittest.cc
index c953d5b..242b5334 100644
--- a/ui/views/view_unittest.cc
+++ b/ui/views/view_unittest.cc
@@ -44,6 +44,9 @@
#include "ui/compositor/compositor.h"
#include "ui/compositor/compositor_switches.h"
#include "ui/compositor/layer.h"
+#include "ui/compositor/layer_animation_element.h"
+#include "ui/compositor/layer_animation_observer.h"
+#include "ui/compositor/layer_animation_sequence.h"
#include "ui/compositor/layer_animator.h"
#include "ui/compositor/paint_context.h"
#include "ui/compositor/test/draw_waiter_for_test.h"
@@ -56,6 +59,7 @@
#include "ui/events/types/event_type.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/geometry/transform.h"
+#include "ui/gfx/scoped_animation_duration_scale_mode.h"
#include "ui/native_theme/native_theme.h"
#include "ui/strings/grit/ui_strings.h"
#include "ui/views/background.h"
@@ -6333,6 +6337,81 @@
delete view;
}
+// View::OrphanLayers() captures a bare ui::Layer* `parent` local and loops
+// over GetLayersInOrder() calling parent->Remove(layer) on each iteration.
+// Layer::Remove() synchronously calls StopAnimatingProperty(BOUNDS), which
+// fires LayerAnimationObserver callbacks that may free `parent`.
+// Tests that this does not cause a crash.
+TEST_F(ViewLayerTest, DestroyParentLayerOnLayerAnimationAborted) {
+ // Ensure the BOUNDS animation has non-zero duration so it is still running
+ // when Layer::Remove() calls StopAnimatingProperty().
+ gfx::ScopedAnimationDurationScaleMode duration_mode(
+ gfx::ScopedAnimationDurationScaleMode::NORMAL_DURATION);
+
+ // The container View only exists so we can call RemoveChildView() to reach
+ // the (private) OrphanLayers(). It has no layer of its own.
+ auto container = std::make_unique<View>();
+
+ // The View whose OrphanLayers() will be invoked.
+ View* target = container->AddChildView(std::make_unique<View>());
+ target->SetPaintToLayer();
+
+ // The parent layer is heap-owned by the test (NOT by any View) so the
+ // observer can free it without a surviving raw_ptr<> quarantining the slot.
+ auto parent_layer = std::make_unique<ui::Layer>();
+ parent_layer->SetName("parent_layer");
+ parent_layer->Add(target->layer());
+
+ // A region layer below `target`'s layer makes GetLayersInOrder() return two
+ // entries so OrphanLayers() loops more than once. This mirrors production
+ // ink-drop region layers (InkDropHost::AddInkDropLayer).
+ auto region_layer = std::make_unique<ui::Layer>();
+ region_layer->SetName("region_layer");
+ target->AddLayerToRegion(region_layer.get(), LayerRegion::kBelow);
+ ASSERT_EQ(region_layer->parent(), parent_layer.get());
+ ASSERT_EQ(target->layer()->parent(), parent_layer.get());
+ ASSERT_EQ(target->GetLayersInOrder().size(), 2u);
+
+ // Observer that frees `parent_layer` when the BOUNDS animation is aborted.
+ class ParentLayerDestroyer : public ui::LayerAnimationObserver {
+ public:
+ explicit ParentLayerDestroyer(std::unique_ptr<ui::Layer>* parent_layer)
+ : parent_layer_(parent_layer) {}
+ void OnLayerAnimationEnded(ui::LayerAnimationSequence*) override {
+ abortion_observed_ = true;
+ parent_layer_->reset();
+ }
+ void OnLayerAnimationAborted(ui::LayerAnimationSequence*) override {
+ abortion_observed_ = true;
+ parent_layer_->reset();
+ }
+ void OnLayerAnimationScheduled(ui::LayerAnimationSequence*) override {}
+
+ bool abortion_observed() const { return abortion_observed_; }
+
+ private:
+ bool abortion_observed_ = false;
+ raw_ptr<std::unique_ptr<ui::Layer>> parent_layer_;
+ };
+ ParentLayerDestroyer observer(&parent_layer);
+
+ // Start a BOUNDS animation on `region_layer` carrying the destructive
+ // observer.
+ ui::LayerAnimator* animator = region_layer->GetAnimator();
+ animator->set_disable_timer_for_test(true);
+ auto sequence = std::make_unique<ui::LayerAnimationSequence>(
+ ui::LayerAnimationElement::CreateBoundsElement(gfx::Rect(0, 0, 50, 50),
+ base::Seconds(1)));
+ sequence->AddObserver(&observer);
+ animator->StartAnimation(sequence.release());
+ ASSERT_TRUE(animator->is_animating());
+
+ std::unique_ptr<View> owned_target = container->RemoveChildViewT(target);
+ // Not reached under ASAN if View::OrphanLayers() is not guarded against
+ // destroyed parent layer during layer-removal loop.
+ EXPECT_TRUE(observer.abortion_observed());
+}
+
TEST_F(ViewLayerTest, LayerBeneathVisibilityUpdated) {
View root;
root.SetPaintToLayer();
Regression Test / PoC
diff --git a/ui/views/view_unittest.cc b/ui/views/view_unittest.cc
index c953d5b..242b5334 100644
--- a/ui/views/view_unittest.cc
+++ b/ui/views/view_unittest.cc
@@ -44,6 +44,9 @@
#include "ui/compositor/compositor.h"
#include "ui/compositor/compositor_switches.h"
#include "ui/compositor/layer.h"
+#include "ui/compositor/layer_animation_element.h"
+#include "ui/compositor/layer_animation_observer.h"
+#include "ui/compositor/layer_animation_sequence.h"
#include "ui/compositor/layer_animator.h"
#include "ui/compositor/paint_context.h"
#include "ui/compositor/test/draw_waiter_for_test.h"
@@ -56,6 +59,7 @@
#include "ui/events/types/event_type.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/geometry/transform.h"
+#include "ui/gfx/scoped_animation_duration_scale_mode.h"
#include "ui/native_theme/native_theme.h"
#include "ui/strings/grit/ui_strings.h"
#include "ui/views/background.h"
@@ -6333,6 +6337,81 @@
delete view;
}
+// View::OrphanLayers() captures a bare ui::Layer* `parent` local and loops
+// over GetLayersInOrder() calling parent->Remove(layer) on each iteration.
+// Layer::Remove() synchronously calls StopAnimatingProperty(BOUNDS), which
+// fires LayerAnimationObserver callbacks that may free `parent`.
+// Tests that this does not cause a crash.
+TEST_F(ViewLayerTest, DestroyParentLayerOnLayerAnimationAborted) {
+ // Ensure the BOUNDS animation has non-zero duration so it is still running
+ // when Layer::Remove() calls StopAnimatingProperty().
+ gfx::ScopedAnimationDurationScaleMode duration_mode(
+ gfx::ScopedAnimationDurationScaleMode::NORMAL_DURATION);
+
+ // The container View only exists so we can call RemoveChildView() to reach
+ // the (private) OrphanLayers(). It has no layer of its own.
+ auto container = std::make_unique<View>();
+
+ // The View whose OrphanLayers() will be invoked.
+ View* target = container->AddChildView(std::make_unique<View>());
+ target->SetPaintToLayer();
+
+ // The parent layer is heap-owned by the test (NOT by any View) so the
+ // observer can free it without a surviving raw_ptr<> quarantining the slot.
+ auto parent_layer = std::make_unique<ui::Layer>();
+ parent_layer->SetName("parent_layer");
+ parent_layer->Add(target->layer());
+
+ // A region layer below `target`'s layer makes GetLayersInOrder() return two
+ // entries so OrphanLayers() loops more than once. This mirrors production
+ // ink-drop region layers (InkDropHost::AddInkDropLayer).
+ auto region_layer = std::make_unique<ui::Layer>();
+ region_layer->SetName("region_layer");
+ target->AddLayerToRegion(region_layer.get(), LayerRegion::kBelow);
+ ASSERT_EQ(region_layer->parent(), parent_layer.get());
+ ASSERT_EQ(target->layer()->parent(), parent_layer.get());
+ ASSERT_EQ(target->GetLayersInOrder().size(), 2u);
+
+ // Observer that frees `parent_layer` when the BOUNDS animation is aborted.
+ class ParentLayerDestroyer : public ui::LayerAnimationObserver {
+ public:
+ explicit ParentLayerDestroyer(std::unique_ptr<ui::Layer>* parent_layer)
+ : parent_layer_(parent_layer) {}
+ void OnLayerAnimationEnded(ui::LayerAnimationSequence*) override {
+ abortion_observed_ = true;
+ parent_layer_->reset();
+ }
+ void OnLayerAnimationAborted(ui::LayerAnimationSequence*) override {
+ abortion_observed_ = true;
+ parent_layer_->reset();
+ }
+ void OnLayerAnimationScheduled(ui::LayerAnimationSequence*) override {}
+
+ bool abortion_observed() const { return abortion_observed_; }
+
+ private:
+ bool abortion_observed_ = false;
+ raw_ptr<std::unique_ptr<ui::Layer>> parent_layer_;
+ };
+ ParentLayerDestroyer observer(&parent_layer);
+
+ // Start a BOUNDS animation on `region_layer` carrying the destructive
+ // observer.
+ ui::LayerAnimator* animator = region_layer->GetAnimator();
+ animator->set_disable_timer_for_test(true);
+ auto sequence = std::make_unique<ui::LayerAnimationSequence>(
+ ui::LayerAnimationElement::CreateBoundsElement(gfx::Rect(0, 0, 50, 50),
+ base::Seconds(1)));
+ sequence->AddObserver(&observer);
+ animator->StartAnimation(sequence.release());
+ ASSERT_TRUE(animator->is_animating());
+
+ std::unique_ptr<View> owned_target = container->RemoveChildViewT(target);
+ // Not reached under ASAN if View::OrphanLayers() is not guarded against
+ // destroyed parent layer during layer-removal loop.
+ EXPECT_TRUE(observer.abortion_observed());
+}
+
TEST_F(ViewLayerTest, LayerBeneathVisibilityUpdated) {
View root;
root.SetPaintToLayer();
Original Bug Report
Potential Use-after-free in View::OrphanLayers due to re-entrant layer destruction
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports without the Chrome Security team. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A potential Use-After-Free (UAF) vulnerability exists in View::OrphanLayers because it maintains a raw pointer to a parent layer across a loop that can trigger synchronous destruction of that parent via animation observers.
Affected files:
ui/views/view.ccui/compositor/layer.cc
Estimated timestamp from git blame: Unknown (Google3 checkout)
Summary
A potential Use-After-Free (UAF) vulnerability has been identified in View::OrphanLayers within ui/views/view.cc. The function iterates through the layers associated with a View and removes them from their parent ui::Layer. However, the removal process can trigger synchronous callbacks that destroy the parent layer, causing the raw parent pointer to dangle during subsequent iterations of the loop.
Technical Analysis
In ui/views/view.cc, View::OrphanLayers captures the parent layer as a raw pointer and iterates over child layers:
// ui/views/view.cc:3763
if (ui::Layer* parent = layer()->parent()) {
for (ui::Layer* layer : GetLayersInOrder()) {
// ... crash key instrumentation accesses parent->name() ...
parent->Remove(layer);
}
}
When parent->Remove(layer) is called (located in ui/compositor/layer.cc), it attempts to stop any ongoing animations on the child layer to ensure consistent state:
// ui/compositor/layer.cc:432
child_animator->StopAnimatingProperty(LayerAnimationElement::BOUNDS);
Stopping an animation synchronously triggers LayerAnimationObserver callbacks, such as OnLayerAnimationAborted. An observer can execute arbitrary logic, including destroying the Widget or View that owns the parent layer. If the parent layer is destroyed during the first iteration of the loop, the parent pointer in View::OrphanLayers becomes dangling.
Subsequent iterations will attempt to access this freed memory, first during crash key instrumentation (e.g., calling parent->name()) and then in the next parent->Remove(layer) call.
This re-entrancy risk is a known pattern; a similar issue was addressed within Layer::Remove itself (see crbug.com/1088432) by adding a base::WeakPtr guard. However, the outer loop in View::OrphanLayers lacks such protection. Production crash data (tracked in b/319941708) and the presence of investigative crash keys in the current source code confirm that this re-entrant state is being reached in the field.
While the ui::Layer class is protected by ADVANCED_MEMORY_SAFETY_CHECKS(), which can enable Scheduler-Loop Quarantine to prevent immediate memory reuse, the vulnerability still allows for a Use-After-Free dereference in the unsandboxed browser process.
Potential Attack Steps
An attacker might attempt to trigger this vulnerability through the following suggested steps:
- Identify a
Viewthat manages multiple layers (e.g., a View usingAddLayerToRegionfor shadows or background effects). - Initiate a bounds animation on one of the auxiliary layers.
- Attach a malicious animation observer that destroys the parent
Viewor itsWidgetwhen the animation is aborted. - Trigger a UI action from a compromised renderer that causes the child
Viewto be removed from its parent (e.g., closing a tab or a specific UI component). - During
OrphanLayers, the first layer removal aborts the animation, the observer destroys the parent layer, and the subsequent iteration dereferences the dangling parent pointer.
Suggested Fix
The loop in View::OrphanLayers should be made re-entrancy safe. This can be achieved by using a base::WeakPtr<ui::Layer> to track the parent layer’s lifetime:
base::WeakPtr<ui::Layer> weak_parent = layer()->parent()->AsWeakPtr();
for (ui::Layer* layer : GetLayersInOrder()) {
if (!weak_parent) break;
weak_parent->Remove(layer);
}
Alternatively, since OrphanLayers is typically called during View destruction or removal, checking the View’s own validity or the parent layer’s existence between iterations would mitigate the risk.
Evaluated with Chrome root at commit: b3153093eb3c78c3e88ccf562bcbc20437a04b0e
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.