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: AuthenticatorCommonImpl::Cleanup() synchronously resets the request state, which triggers WebAuthn modal dialog widget destruction. Under specific conditions, such as the page being hosted in an extension popup, widget closure synchronously destroys the hosting WebContents and the AuthenticatorCommonImpl instance itself. This leads to a potential Use-After-Free (UAF) write and read when Cleanup() subsequently increments and checks next_request_key_.

Affected files:

• content/browser/webauth/authenticator_common_impl.cc
• content/browser/webauth/authenticator_common_impl.h

Estimated timestamp from git blame: 2024-08-06

Location

• content/browser/webauth/authenticator_common_impl.cc:3223-3229
• content/browser/webauth/authenticator_common_impl.h

Root Cause Analysis

AuthenticatorCommonImpl::Cleanup() performs req_state_.reset() to tear down request-specific state. However, resetting this state can synchronously close UI dialogs/widgets. Under certain environments (like an extension popup), closing the UI shifts focus/activation, triggering a chain that synchronously destroys the hosting WebContents, the Mojo DocumentService AuthenticatorImpl, and AuthenticatorCommonImpl itself.

Since Cleanup() accesses this immediately after the reset without any liveness guards, this leads to a potential browser-process Use-After-Free (UAF) write (next_request_key_++) and read (CHECK(next_request_key_)):

// content/browser/webauth/authenticator_common_impl.cc:3223-3229
void AuthenticatorCommonImpl::Cleanup() {
  CHECK(!req_state_ || req_state_->request_key.value() == next_request_key_);
  req_state_.reset();          // <--- Synchronous widget teardown and self-destruction
  next_request_key_++;         // <--- UAF Write to next_request_key_ on freed heap
  CHECK(next_request_key_);    // <--- UAF Read from freed heap
}

next_request_key_ is a plain uint64_t member (defined in authenticator_common_impl.h:384), meaning it is not protected by MiraclePtr.

Potential Destruction Sequence

1. req_state_.reset() deletes the RequestState structure.
2. Deleting RequestState destroys its member std::unique_ptr<AuthenticatorRequestClientDelegate> request_delegate.
3. In Chrome, this invokes ~ChromeAuthenticatorRequestDelegate(), which calls dialog_model_->OnRequestComplete() (defined in chrome/browser/webauthn/chrome_authenticator_request_delegate.cc:290).
4. Observers are notified, and AuthenticatorRequestDialogController::OnRequestComplete() transitions the current step to Step::kClosed.
5. AuthenticatorRequestDialogModel::SetStep() sees that kClosed has no dialog UI type, and executes view_controller_.reset() (defined in authenticator_request_dialog_model.cc:167).
6. The view controller’s default destructor destroys std::unique_ptr<views::Widget> widget_, invoking views::Widget::~Widget() (defined in ui/views/widget/widget.cc:295).
7. Since the widget has CLIENT_OWNS_WIDGET ownership, the destructor removes the dialog from the WebContentsModalDialogManager via WillClose() and calls native_widget_->Close().
8. The native widget closure shifts native window activation back to the browser window.
9. This activation change is observed by the parent tree, triggering ExtensionPopup::OnWidgetTreeActivated() on the extension popup (defined in chrome/browser/ui/views/extensions/extension_popup.cc:152).
10. Since the WebAuthn dialog was already removed from the modal manager, the popup checks web_modal::WebContentsHasActiveWebModal() which returns false, leading to the popup closing itself via CloseDeferredIfNecessary().
11. Popup closure synchronously destroys its WebContents and the frame-bound Mojo DocumentService AuthenticatorImpl containing the AuthenticatorCommonImpl instance, freeing its heap memory.
12. The call stack unwinds back to Cleanup(), which attempts to execute next_request_key_++ on the freed this object.

Suggested / Potential Reproduction Steps

(Note: These are potential steps based on code analysis; our tooling agent does not yet have the capability to execute code or verify the exact platform-specific activation timing.)

1. Load an extension popup page that initiates a WebAuthn registration or assertion request (via navigator.credentials.create / navigator.credentials.get).
2. While the WebAuthn modal dialog is active, complete or abort the request (e.g., via user cancellation or programmatic abort).
3. Observe if the destruction of the widget shifts focus and synchronously tears down the WebContents container and the AuthenticatorCommonImpl instance before the Cleanup() call stack unwinds.

Suggested Fix

Guard the post-reset instructions inside AuthenticatorCommonImpl::Cleanup() with a WeakPtr liveness check, similarly to how identical patterns were resolved in the sibling Digital Credentials implementation:

void AuthenticatorCommonImpl::Cleanup() {
  CHECK(!req_state_ || req_state_->request_key.value() == next_request_key_);
  base::WeakPtr<AuthenticatorCommonImpl> weak_this = weak_factory_.GetWeakPtr();
  req_state_.reset();
  if (!weak_this) {
    return;
  }
  next_request_key_++;
  CHECK(next_request_key_);
}

Evaluated with Chrome root at commit: 3947e01999a53d4e2382e39736cb79d79c7dffcf

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.