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. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.

Overview: An analysis of WindowModalityController::OnWindowPropertyChanged identified a potential re-entrancy path where ActivateWindow can trigger window destruction before CancelTouchesOnTransientWindowTree is called. However, investigation reveals that synchronous destruction under this path is intercepted by ScopedDeleteBlocker, resulting in a safe CHECK crash rather than a Use-After-Free. This report documents the re-entrancy flow and recommends a robust safety improvement.

Affected files:

• ui/wm/core/window_modality_controller.cc

Estimated timestamp from git blame: 2018-11-14

Description

In ui/wm/core/window_modality_controller.cc:163-175, when the kModalKey property is modified on an already-visible window, WindowModalityController::OnWindowPropertyChanged is triggered:

void WindowModalityController::OnWindowPropertyChanged(aura::Window* window,
                                                       const void* key,
                                                       intptr_t old) {
  if (key == aura::client::kModalKey &&
      window->GetProperty(aura::client::kModalKey) !=
          ui::mojom::ModalType::kNone &&
      window->IsVisible()) {
    ActivateWindow(window);
    CancelTouchesOnTransientWindowTree(window);
  }
}

Calling ActivateWindow(window) synchronously fires activation and deactivation observers via the focus/activation controller. If a deactivation or focus observer attempts to synchronously destroy the window, CancelTouchesOnTransientWindowTree(window) would immediately execute and dereference the potentially freed window pointer.

Root Cause & Existing Safety Mechanisms

An in-depth analysis of the Aura window lifetime management reveals that a Use-After-Free (UAF) is successfully prevented by the following safety barriers:

1. ScopedDeleteBlocker Protection: When a window property is changed, aura::Window::AfterPropertyChange is executed. This function places a ScopedDeleteBlocker on the stack, incrementing the window’s delete_block_count_ before notifying observers:

   void Window::AfterPropertyChange(const void* key, int64_t old_value) {
     ScopedDeleteBlocker blocker(this);
     for (WindowObserver& observer : observers_)
       observer.OnWindowPropertyChanged(this, key, old_value);
   }
   

   If any observer attempts to synchronously delete window (or its transient parent/child which cascades to window), the destructor Window::~Window() executes the safety check CHECK_EQ(delete_block_count_, 0u). Since delete_block_count_ is non-zero, the browser process safely and immediately terminates with a crash, preventing any UAF condition.

2. Asynchronous Exo Widget Destruction: For Wayland clients (Exo), widget destruction sequences (e.g., capture-loss or deactivation-triggered closes) use Widget::Close(), which posts an asynchronous task to delete the widget. Thus, synchronous destruction of the native window cannot be initiated by Wayland client actions during activation observer dispatch.

3. Transient Tree Integrity: If a transient parent window is destroyed under normal conditions, standard destruction observers cleanly update the active window’s parent pointers to nullptr, ensuring no dangling references are left in the tree.

Potential Steps to Trigger the Safe Crash

An attacker/client could theoretically try to trigger a crash (Denial of Service) via the following suggested steps:

1. From a Wayland client, create and commit an xdg_toplevel window that is mapped and visible.
2. Configure focus such that activating this window deactivates another window containing an observer that initiates synchronous closure of the transient hierarchy.
3. Request system modal state on the visible window via zaura_toplevel.set_system_modal or zcr_remote_surface_v2.set_system_modal.
4. During ActivateWindow, the synchronous deletion path is executed, immediately triggering a safe browser-process crash due to the delete_block_count_ assertion.

Suggested Fix

To make the code robust against re-entrancy hazards and avoid potential denial of service crashes, it is recommended to use aura::WindowTracker to check if the window is still alive after ActivateWindow completes:

void WindowModalityController::OnWindowPropertyChanged(aura::Window* window,
                                                       const void* key,
                                                       intptr_t old) {
  if (key == aura::client::kModalKey &&
      window->GetProperty(aura::client::kModalKey) !=
          ui::mojom::ModalType::kNone &&
      window->IsVisible()) {
    aura::WindowTracker tracker({window});
    ActivateWindow(window);
    if (tracker.Contains(window)) {
      CancelTouchesOnTransientWindowTree(window);
    }
  }
}

Evaluated with Chrome root at commit: d8b226a3be7c9c1ac9240c09e14698866c82e4ac

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.