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: A potential Use-After-Free (UAF) vulnerability exists in the Windows browser process when display topology or work area updates synchronously close a widget. During DesktopWindowTreeHostWin::HandleMove(), a synchronous display notification can trigger the immediate closure and deletion of the active widget, leaving the stack ’this’ pointer dangling during subsequent member accesses.

Affected files:

• ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc
• ui/views/win/hwnd_message_handler.cc

Estimated timestamp from git blame: 2024-01-02

Root Cause Analysis

In ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc, the function DesktopWindowTreeHostWin::HandleMove() invokes display::win::GetScreenWin()->UpdateDisplayInfosIfNeeded() without any subsequent liveness or weak pointer checks on this:

// ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc:1178-1185
void DesktopWindowTreeHostWin::HandleMove() {
  // Adding/removing a monitor, or changing the primary monitor can cause a
  // WM_MOVE message before `OnDisplayChanged()`. Without this call, we would
  // DCHECK due to stale `DisplayInfo`s. See https:://crbug.com/1413940.
  display::win::GetScreenWin()->UpdateDisplayInfosIfNeeded();   // <-- Can synchronously destroy 'this'
  CheckForMonitorChange();                                      // <-- UAF read/write
  OnHostMovedInPixels();                                        // <-- UAF virtual call
}

When a display configuration change occurs (e.g., changing the primary monitor or work area boundaries), the OS dispatches a synchronous WM_MOVE message to all top-level windows (including notification popups). This message is received by HWNDMessageHandler::OnMove and delegated to DesktopWindowTreeHostWin::HandleMove().

Inside HandleMove(), the call to UpdateDisplayInfosIfNeeded() triggers a synchronous display observer update loop (change_notifier_.NotifyDisplaysChanged()).

On Windows, DesktopMessagePopupCollection (defined in ui/message_center/views/desktop_message_popup_collection.cc) is registered as an active DisplayObserver. When the display layout changes, DesktopMessagePopupCollection::OnDisplayMetricsChanged is synchronously invoked, which calls ResetBounds() -> ClosePopupsOutsideWorkArea().

If the notification popup currently executing the HandleMove() method is now outside the visible work area, it is synchronously closed via GetWidget()->CloseNow(). This completely deallocates and deletes the underlying DesktopWindowTreeHostWin and its HWNDMessageHandler on the heap.

Upon returning from UpdateDisplayInfosIfNeeded(), control resumes in HandleMove(), but the stack this pointer is now dangling. This results in:

1. A Use-After-Free read/write in CheckForMonitorChange() when querying the window and updating last_nearest_display_.
2. A Use-After-Free virtual call in OnHostMovedInPixels() when iterating over the observers_ list.
3. A Use-After-Free write in HWNDMessageHandler::OnMove() when setting SetMsgHandled(FALSE).

Note: These are potential steps and findings derived from static analysis of the Chromium source tree; our automated tooling does not currently have the capability to execute and validate live exploits.

Sibling Guard Proof of Hazard

The sibling handler HWNDMessageHandler::OnDpiChanged() already implements a weak pointer guard pattern to defend against this exact synchronous destruction scenario:

// ui/views/win/hwnd_message_handler.cc:2017-2021
auto ref = msg_handler_weak_factory_.GetWeakPtr();
display::win::GetScreenWin()->UpdateDisplayInfos();
if (!ref) {
  return 0;
}

No such liveness check is present in DesktopWindowTreeHostWin::HandleMove() or HWNDMessageHandler::OnMove().

Potential Steps to Trigger the Issue

1. Grant a website permission to display desktop notifications.
2. Direct the website to trigger a notification popup (which translates to a top-level WS_POPUP window on Windows).
3. Alter the system’s screen topology (e.g., unplug a monitor or change the primary monitor’s resolution/identity).
4. Windows sends a synchronous WM_MOVE message to the popup, calling HandleMove().
5. The display metrics update loop synchronously runs. DesktopMessagePopupCollection identifies that the notification popup is now outside the newly calculated work area boundaries and synchronously calls CloseNow().
6. The host and its message handler are destroyed.
7. Execution returns to the call stack in HandleMove() and OnMove(), triggering UAF access on the freed objects.

Suggested Fix

Introduce a base::WeakPtr liveness check inside both HWNDMessageHandler::OnMove() and DesktopWindowTreeHostWin::HandleMove() to safely abort execution if the widget is synchronously destroyed during the display information update.

Specifically, we can add a weak factory or leverage the message handler’s existing weak factory to track liveness. For example, in HWNDMessageHandler::OnMove():

void HWNDMessageHandler::OnMove(const gfx::Point& point) {
  auto ref = msg_handler_weak_factory_.GetWeakPtr();
  delegate_->HandleMove();
  if (!ref) {
    return;
  }
  SetMsgHandled(FALSE);
}

And in DesktopWindowTreeHostWin::HandleMove():

void DesktopWindowTreeHostWin::HandleMove() {
  base::WeakPtr<DesktopWindowTreeHostWin> ref = weak_factory_.GetWeakPtr();
  display::win::GetScreenWin()->UpdateDisplayInfosIfNeeded();
  if (!ref) {
    return;
  }
  CheckForMonitorChange();
  OnHostMovedInPixels();
}

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.