Flapjack, 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) exists in TransientWindowManager::OnWindowHierarchyChanged due to iterator invalidation. Synchronous observer callbacks fired during a range-based for loop over transient_children_ can mutate the vector, invalidating iterators and potentially leading to a browser process UAF.

Affected files:

• ui/wm/core/transient_window_manager.cc

Estimated timestamp from git blame: 2023-12-20

Summary

A potential Use-After-Free vulnerability was identified in TransientWindowManager::OnWindowHierarchyChanged (ui/wm/core/transient_window_manager.cc). The method iterates over the transient_children_ vector while performing operations that trigger synchronous window observer callbacks. If an observer mutates the window hierarchy, it can modify the transient_children_ vector, invalidating the loop’s iterators and causing a UAF when the loop advances.

Technical Details

In TransientWindowManager::OnWindowHierarchyChanged, the code uses a range-based for loop to reparent transient children:

// ui/wm/core/transient_window_manager.cc:201
for (aura::Window* transient_child : transient_children_) {
  if (transient_child->parent() == old_parent) {
    new_parent->AddChild(transient_child);
    should_restack = true;
  }
}

The call to new_parent->AddChild(transient_child) inside the loop is highly complex and synchronously dispatches several observer notifications, such as OnWindowHierarchyChanging and OnWindowAdded, to observers across the UI.

If any registered observer reacts to these notifications by altering the transient children of this window (e.g., by creating a new transient UI element which calls wm::AddTransientChild, or destroying an existing transient child which calls wm::RemoveTransientChild), the transient_children_ vector is mutated.

Specifically, if push_back is called and the vector’s capacity is exceeded, its backing store is reallocated and the old buffer is freed. Because the range-based for loop caches the begin() and end() iterators (which are raw pointers to the vector’s backing store), the reallocation invalidates them.

When the loop attempts to advance, it dereferences a dangling iterator, resulting in a Use-After-Free on the freed vector backing store.

Potential Attack Scenario

While our tooling agent does not have the ability to run code to confirm exploitability, an attacker might trigger this by:

1. Using Web or Extension APIs to create multiple transient windows (e.g., PiP windows or popups) such that the transient_children_ vector is exactly at its allocated capacity.
2. Triggering a window reparenting action (e.g., entering fullscreen) that invokes OnWindowHierarchyChanged.
3. Relying on an existing UI observer to react to the reparenting by synchronously adding a new transient child.
4. The vector reallocation frees the backing store, which the attacker reclaims using memory manipulation primitives (e.g., IPC sprays).
5. The attacker places forged raw_ptr objects in the reclaimed backing store. When the loop dereferences these forged pointers, it accesses attacker-controlled fake aura::Window objects, potentially leading to virtual method calls and arbitrary code execution in the Browser process (Sandbox Escape).

Recommended Fix

To safely iterate over transient_children_ while permitting synchronous hierarchy modifications, aura::WindowTracker should be used. This pattern handles windows being added or removed during iteration and is already used safely elsewhere in the same class (e.g., OnWindowVisibilityChanged and OnWindowDestroying).

// Reparenting multiple sibling transient children will call back onto us...
base::WeakAutoReset reset(
    weak_factory_.GetWeakPtr(),
    &TransientWindowManager::pause_transient_descendants_restacking_, true);

aura::WindowTracker tracker(transient_children_);
while (!tracker.windows().empty()) {
  aura::Window* transient_child = tracker.Pop();
  if (transient_child->parent() == old_parent) {
    new_parent->AddChild(transient_child);
    should_restack = true;
  }
}

Evaluated with Chrome root at commit: c0eb5541aebfa4ea08806eaf6e94bcc69f87ab2f

Results so far have been promising, but there can be wrong deductions. If this proves to be a false positive, please close as WAI; 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.