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 browser-process use-after-free vulnerability exists in DigitalIdentityMultiStepDialog::TryShow. When displaying a web-modal dialog, the hosting WebContents can be synchronously destroyed during a nested message loop spun by exiting fullscreen mode. This synchronously destroys the dialog object, leaving the method to perform unsafe write and read operations on the freed instance upon return.

Affected files:

• chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog.cc
• chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog.h

Estimated timestamp from git blame: 2024-07-23

Root Cause Analysis

A potential heap use-after-free (UAF) vulnerability exists in DigitalIdentityMultiStepDialog::TryShow due to a re-entrant destruction path during dialog creation. When displaying a web modal, the hosting WebContents can be synchronously destroyed, leading to the deletion of the dialog object. Upon return from ShowWebModalDialogViews, the method performs write and read operations on the freed this instance.

The vulnerable code is located in chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog.cc:

if (new_dialog_delegate) {
  // views::Widget takes ownership of `new_dialog_delegate`.
  dialog_ = constrained_window::ShowWebModalDialogViews(
                new_dialog_delegate.release(), web_contents_.get())
                ->GetWeakPtr();
  extensions::SecurityDialogTracker::GetInstance()->AddSecurityDialog(
      dialog_.get());
}

There is no self-liveness guard after ShowWebModalDialogViews returns. The class has no base::WeakPtrFactory<DigitalIdentityMultiStepDialog>, so a self-liveness check is currently impossible without a code change. The written member dialog_ is a base::WeakPtr<views::Widget>, which is not protected by MiraclePtr.

Synchronous Destruction Path

1. ShowWebModalDialogViews calls ShowModalDialog, which delegates to WebContentsModalDialogManager::ShowDialogWithManager.
2. This invokes BlockWebContentsInteraction(true), which calls BrowserWindowModalDialogDelegate::SetWebContentsBlocked(web_contents, true).
3. If the tab is currently in HTML fullscreen, SetWebContentsBlocked calls web_contents->ExitFullscreen(true) to drop fullscreen.
4. WebContentsImpl::ExitFullscreenMode invokes delegate_->ExitFullscreenModeForTab(this).
5. As documented in the WebContentsImpl source code, this delegate call may spin a nested message loop (or wait for window server events) while adjusting the platform window state.
6. While this nested loop is spinning, queued tasks (such as standard tab closing or navigation commits) can run, which synchronously destroys the hosting WebContents and the underlying RenderFrameHost.
7. The destruction of the RenderFrameHost synchronously deletes the associated DocumentService (DigitalIdentityRequestImpl), which deletes the DigitalIdentityProviderDesktop and consequently the DigitalIdentityMultiStepDialog (this).
8. Control then returns to TryShow, which attempts to write to this->dialog_ and read from it via dialog_.get(), resulting in a Use-After-Free.

Suggested / Potential Trigger Steps

Note: These are potential steps based on static code analysis; our tooling agent does not currently have the ability to run code or verify a live proof of concept.

1. A web page in HTML fullscreen initiates a low-risk Digital Credentials API request via navigator.credentials.get using a gesture.
2. The request bypasses the security interstitial and invokes DigitalIdentityMultiStepDialog::TryShow to display the QR code modal.
3. TryShow calls ShowWebModalDialogViews, forcing the browser to exit fullscreen.
4. Exiting fullscreen spins a nested loop during which the tab is closed or navigated, synchronously deleting the WebContents and the DigitalIdentityMultiStepDialog object.
5. When ShowWebModalDialogViews returns, the method writes the return value to this->dialog_ on the freed this allocation.

Suggested Fix

Add a base::WeakPtrFactory to the DigitalIdentityMultiStepDialog class. Capture a weak reference to this before calling ShowWebModalDialogViews, and use a self-liveness guard to return early if this has been deleted:

if (new_dialog_delegate) {
  base::WeakPtr<DigitalIdentityMultiStepDialog> weak_this = weak_ptr_factory_.GetWeakPtr();
  views::Widget* widget = constrained_window::ShowWebModalDialogViews(
      new_dialog_delegate.release(), web_contents_.get());
  if (!weak_this) {
    return;
  }
  dialog_ = widget->GetWeakPtr();
  extensions::SecurityDialogTracker::GetInstance()->AddSecurityDialog(
      dialog_.get());
}

Evaluated with Chrome root at commit: 9ebf4302210513a012c901d87a2668b3aadf8cc1

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.