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 write vulnerability exists in ConsentDialogCoordinator::Show on macOS. Calling chrome::ShowBrowserModal synchronously runs a nested run loop that can process incoming tasks or IPCs, potentially destroying the ConsentDialogCoordinator object before the function returns. This could allow a heap write-after-free when assigning the resulting widget pointer to a member of the freed coordinator.

Affected files:

• chrome/browser/ui/views/device_signals_consent/consent_dialog_coordinator.cc
• chrome/browser/ui/views/device_signals_consent/consent_dialog_coordinator.h
• chrome/browser/enterprise/connectors/device_trust/navigation_throttle.cc
• chrome/browser/enterprise/connectors/device_trust/navigation_throttle.h

Estimated timestamp from git blame: 2023-05-25

Description

A potential use-after-free (UAF) write vulnerability has been identified in ConsentDialogCoordinator::Show() on macOS. The root cause is a synchronous nested run loop spawned during the modal sheet presentation, which permits arbitrary task processing (including frame destruction) while ConsentDialogCoordinator::Show() is suspended on the call stack.

Root Cause Analysis

In chrome/browser/ui/views/device_signals_consent/consent_dialog_coordinator.cc, the Show() method is defined as follows:

void ConsentDialogCoordinator::Show() {
  if (dialog_widget_ && !dialog_widget_->IsClosed()) {
    ...
    return;
  }
  base::RecordAction(base::UserMetricsAction("DeviceSignalsConsent_Shown"));
  dialog_widget_ = chrome::ShowBrowserModal(
      browser_, CreateDeviceSignalsConsentDialogModel());
}

When chrome::ShowBrowserModal is called:

1. It delegates to BrowserModalHelper::Show, which creates a views::BubbleDialogModelHost with ModalType::kWindow and calls widget->Show() (components/constrained_window/constrained_window_views.cc:215).
2. On macOS, this triggers the in-process Cocoa window bridge (NativeWidgetNSWindowBridge::ShowAsModalSheet), which synchronously invokes -[NSWindow beginSheet:completionHandler:] on the UI thread’s stack (components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm:2232).
3. beginSheet: enters a synchronous nested run loop to perform the sheet’s slide-down presentation animation.

During this nested run loop, the UI thread continues to pump events, allowing processing of arbitrary Mojo IPCs or timers. If a renderer-initiated navigation or window-close event occurs for the deferred frame:

1. The pending NavigationRequest is aborted and synchronously destroyed.
2. The owned DeviceTrustNavigationThrottle is deleted (chrome/browser/enterprise/connectors/device_trust/navigation_throttle.cc:129).
3. The throttle deletes its owned consent_requester_ (std::unique_ptr<ConsentRequester>), invoking ~ConsentDialogCoordinator().
4. Since dialog_widget_ is still nullptr (as the assignment has not yet executed), the destructor’s safety checks do not perform any widget cleanup, and the ConsentDialogCoordinator heap allocation is freed.

Once the AppKit nested loop yields and chrome::ShowBrowserModal returns the Widget* pointer, execution resumes at the call site. The code then attempts to write the pointer to this->dialog_widget_. Since this has already been deleted, this results in a write-after-free (and a potential reference count decrement inside raw_ptr::operator= if the slot has been reallocated to attacker-influenced data).

Potential Trigger Steps

Note: These are suggested potential steps, as our testing tools do not currently have the capability to execute code.

1. Establish an enterprise-managed macOS environment where Device Trust consent collection is enabled (ShouldCollectConsent() returns true).
2. Navigate a main frame to an attacker-controlled page, which spawns a popup (w = window.open(...)).
3. Have the user perform a gesture in the popup to navigate to a monitored Device Trust URL, triggering the deferred DeviceTrustNavigationThrottle and posting the task to show the consent dialog.
4. Within the popup origin, use a timed script (e.g., setTimeout) to close the popup or navigate it to about:blank within the ~200ms slide-down animation window.
5. The browser processes the navigation cancel/abort IPC, destroying the deferred NavigationRequest, the throttle, and the coordinator object.
6. The nested loop returns, triggering the UAF write to this->dialog_widget_ on the freed coordinator’s address.

Suggested Fix

To prevent this issue, we should use a base::WeakPtr to guard the assignment to dialog_widget_ after chrome::ShowBrowserModal returns, ensuring the coordinator object is still alive:

void ConsentDialogCoordinator::Show() {
  if (dialog_widget_ && !dialog_widget_->IsClosed()) {
    if (!dialog_widget_->IsVisible()) {
      dialog_widget_->Show();
    }
    return;
  }
  base::RecordAction(base::UserMetricsAction("DeviceSignalsConsent_Shown"));
  
  auto weak_this = weak_ptr_factory_.GetWeakPtr();
  views::Widget* widget = chrome::ShowBrowserModal(
      browser_, CreateDeviceSignalsConsentDialogModel());
  if (weak_this) {
    dialog_widget_ = widget;
  }
}

Evaluated with Chrome root at commit: b5b015ea5f690560237d1f0cff1405844cd12b8d

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.