From 57b6bdd81cd758d9a11914b9c28ae51d8fbee529 Mon Sep 17 00:00:00 2001
From: Mohamed Amir Yosef <mamir@chromium.org>
Date: Mon, 08 Jun 2026 03:11:49 -0700
Subject: [PATCH] [DC] Fix UAF on dialog close

DigitalIdentityProviderDesktop::EndRequestWithError() resets the multi-
step dialog, which synchronously destroys the associated Views widget.
If the dialog is hosted in a transient container (like an extension
popup) that closes on deactivation, closing the widget can synchronously
destroy the hosting WebContents.

Since the Mojo interface implementation DigitalIdentityRequestImpl is a
frame-bound DocumentService, the WebContents destruction synchronously
deletes it, which in turn deletes the DigitalIdentityProviderDesktop
instance on the stack.

When control returns to EndRequestWithError(), the method attempts to
access and run callback_, which is a member of the now-deleted provider
instance, resulting in a browser-process heap use-after-free (UAF).

This CL fixes the UAF by moving the callback to a local stack variable
before resetting the dialog. If the provider is synchronously destroyed,
the callback resides safely on the stack.

This CL also adds a regression browser test that reproduces the UAF
under a simulated WebContents teardown.

Fixed: 516942828
Test: DigitalIdentityMultiStepDialogBrowserTest.EndRequestWithErrorOwnerDestroyedDuringDialogClose
Change-Id: Ia916e6551a7cda6a0a9e1bfb3cfdbd93482da039
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7904663
Reviewed-by: Andrii Natiahlyi <natiahlyi@google.com>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1643069}
---

diff --git a/chrome/browser/digital_credentials/digital_identity_provider_desktop.cc b/chrome/browser/digital_credentials/digital_identity_provider_desktop.cc
index 6332881..867cd5e60 100644
--- a/chrome/browser/digital_credentials/digital_identity_provider_desktop.cc
+++ b/chrome/browser/digital_credentials/digital_identity_provider_desktop.cc
@@ -399,8 +399,19 @@
     return;
   }
 
+  // `dialog_.reset()` can synchronously close the UI which (via activation
+  // observers) may destroy the hosting WebContents, resulting in the
+  // synchronous destruction of the frame-bound Mojo DocumentService
+  // `DigitalIdentityRequestImpl` and therefore `this`.
+  //
+  // To avoid a Use-After-Free, move the callback to a local variable on the
+  // stack before resetting the dialog or the manual bluetooth controller (both
+  // of which could trigger synchronous teardown via UI events).
+  auto local_callback = std::move(callback_);
+
   bluetooth_manual_dialog_controller_.reset();
   dialog_.reset();
+  // `this` may be deleted at this point.
 
-  std::move(callback_).Run(base::unexpected(status));
+  std::move(local_callback).Run(base::unexpected(status));
 }
diff --git a/chrome/browser/digital_credentials/digital_identity_provider_desktop.h b/chrome/browser/digital_credentials/digital_identity_provider_desktop.h
index 1387e29bd..2be6166 100644
--- a/chrome/browser/digital_credentials/digital_identity_provider_desktop.h
+++ b/chrome/browser/digital_credentials/digital_identity_provider_desktop.h
@@ -74,6 +74,13 @@
     callback_ = std::move(callback);
   }
 
+  // Ensures `dialog_` is initialized and returns it.
+  DigitalIdentityMultiStepDialog* EnsureDialogCreated();
+
+  // Called to end the request with an error.
+  void EndRequestWithError(
+      content::DigitalIdentityProvider::RequestStatusForMetrics);
+
  private:
   // Called whenever some significant event occurs during the transaction.
   void OnEvent(const std::string& qr_url,
@@ -90,8 +97,6 @@
       base::expected<content::digital_credentials::cross_device::Response,
                      content::digital_credentials::cross_device::Error>);
 
-  // Ensures `dialog_` is initialized and returns it.
-  DigitalIdentityMultiStepDialog* EnsureDialogCreated();
 
   // Shows dialog which prompts user to manually turn on bluetooth.
   void ShowBluetoothManualTurnOnDialog();
@@ -115,9 +120,6 @@
   // canceling the dialog.
   void OnCanceled();
 
-  // Called to end the request with an error.
-  void EndRequestWithError(
-      content::DigitalIdentityProvider::RequestStatusForMetrics);
 
   // The web contents to which the dialog is modal to.
   base::WeakPtr<content::WebContents> web_contents_;
diff --git a/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc b/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc
index 30059c9..40ad72c 100644
--- a/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc
+++ b/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc
@@ -6,6 +6,7 @@
 
 #include "base/scoped_observation.h"
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/digital_credentials/digital_identity_provider_desktop.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -233,3 +234,106 @@
         ui::mojom::DialogButton::kOk));
   }
 }
+
+namespace {
+
+// Subclass to expose protected methods for testing.
+class TestDigitalIdentityProviderDesktop
+    : public DigitalIdentityProviderDesktop {
+ public:
+  using DigitalIdentityProviderDesktop::EndRequestWithError;
+  using DigitalIdentityProviderDesktop::EnsureDialogCreated;
+  using DigitalIdentityProviderDesktop::set_callback_for_testing;
+  using DigitalIdentityProviderDesktop::set_rp_origin_for_testing;
+  using DigitalIdentityProviderDesktop::set_web_contents_for_testing;
+
+  // Calls the protected ShowQrCodeDialog.
+  void SetUpAndShowQrDialog(content::WebContents* web_contents,
+                            base::OnceClosure callback) {
+    set_web_contents_for_testing(web_contents->GetWeakPtr());
+    set_rp_origin_for_testing(url::Origin::Create(GURL("https://rp.example")));
+    set_callback_for_testing(base::BindOnce(
+        [](base::OnceClosure callback,
+           base::expected<
+               TestDigitalIdentityProviderDesktop::DigitalCredential,
+               content::DigitalIdentityProvider::RequestStatusForMetrics>
+               result) { std::move(callback).Run(); },
+        std::move(callback)));
+    ShowQrCodeDialog("FIDO:/0123456789", RequestInfo::RequestType::kGet);
+  }
+
+  DigitalIdentityMultiStepDialog* GetDialog() { return EnsureDialogCreated(); }
+};
+
+class ProviderDestroyerOnWidgetClosingObserver : public views::WidgetObserver {
+ public:
+  explicit ProviderDestroyerOnWidgetClosingObserver(
+      base::OnceClosure destruction_callback)
+      : destruction_callback_(std::move(destruction_callback)) {}
+
+  void OnWidgetClosing(views::Widget* widget) override {
+    widget->RemoveObserver(this);
+    if (destruction_callback_) {
+      std::move(destruction_callback_).Run();
+    }
+  }
+
+ private:
+  base::OnceClosure destruction_callback_;
+};
+
+}  // namespace
+
+// Regression test for UAF in
+// DigitalIdentityProviderDesktop::EndRequestWithError when the owner is
+// synchronously destroyed during dialog close.
+IN_PROC_BROWSER_TEST_F(DigitalIdentityMultiStepDialogBrowserTest,
+                       EndRequestWithErrorOwnerDestroyedDuringDialogClose) {
+  auto provider = std::make_unique<TestDigitalIdentityProviderDesktop>();
+
+  base::RunLoop run_loop;
+  // Show the dialog via the real ShowQrCodeDialog flow.
+  provider->SetUpAndShowQrDialog(GetActiveWebContents(),
+                                 run_loop.QuitClosure());
+
+  // Retrieve the widget robustly using TestApi and EnsureDialogCreated.
+  // Wrap `TestApi` in a nested scope so it is destroyed before the message
+  // loop runs. Otherwise, when the loop runs and triggers the UAF teardown,
+  // the dialog is deleted, leaving `TestApi` holding a dangling raw_ptr.
+  views::Widget* widget = nullptr;
+  {
+    DigitalIdentityMultiStepDialog* dialog = provider->GetDialog();
+    DigitalIdentityMultiStepDialog::TestApi dialog_test_api(dialog);
+    widget = dialog_test_api.GetWidget();
+  }
+  ASSERT_TRUE(widget);
+  base::WeakPtr<views::Widget> weak_widget = widget->GetWeakPtr();
+
+  // Set up the observer to synchronously destroy the provider when the widget
+  // closes.
+  ProviderDestroyerOnWidgetClosingObserver observer(base::BindOnce(
+      [](std::unique_ptr<TestDigitalIdentityProviderDesktop>* provider) {
+        provider->reset();
+      },
+      base::Unretained(&provider)));
+  widget->AddObserver(&observer);
+
+  // Trigger cancellation. OnDialogCanceled() will PostTask a call to
+  // OnCanceled() -> EndRequestWithError().

diff --git a/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc b/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc
index 30059c9..40ad72c 100644
--- a/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc
+++ b/chrome/browser/ui/views/digital_credentials/digital_identity_multi_step_dialog_browsertest.cc
@@ -6,6 +6,7 @@
 
 #include "base/scoped_observation.h"
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/digital_credentials/digital_identity_provider_desktop.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -233,3 +234,106 @@
         ui::mojom::DialogButton::kOk));
   }
 }
+
+namespace {
+
+// Subclass to expose protected methods for testing.
+class TestDigitalIdentityProviderDesktop
+    : public DigitalIdentityProviderDesktop {
+ public:
+  using DigitalIdentityProviderDesktop::EndRequestWithError;
+  using DigitalIdentityProviderDesktop::EnsureDialogCreated;
+  using DigitalIdentityProviderDesktop::set_callback_for_testing;
+  using DigitalIdentityProviderDesktop::set_rp_origin_for_testing;
+  using DigitalIdentityProviderDesktop::set_web_contents_for_testing;
+
+  // Calls the protected ShowQrCodeDialog.
+  void SetUpAndShowQrDialog(content::WebContents* web_contents,
+                            base::OnceClosure callback) {
+    set_web_contents_for_testing(web_contents->GetWeakPtr());
+    set_rp_origin_for_testing(url::Origin::Create(GURL("https://rp.example")));
+    set_callback_for_testing(base::BindOnce(
+        [](base::OnceClosure callback,
+           base::expected<
+               TestDigitalIdentityProviderDesktop::DigitalCredential,
+               content::DigitalIdentityProvider::RequestStatusForMetrics>
+               result) { std::move(callback).Run(); },
+        std::move(callback)));
+    ShowQrCodeDialog("FIDO:/0123456789", RequestInfo::RequestType::kGet);
+  }
+
+  DigitalIdentityMultiStepDialog* GetDialog() { return EnsureDialogCreated(); }
+};
+
+class ProviderDestroyerOnWidgetClosingObserver : public views::WidgetObserver {
+ public:
+  explicit ProviderDestroyerOnWidgetClosingObserver(
+      base::OnceClosure destruction_callback)
+      : destruction_callback_(std::move(destruction_callback)) {}
+
+  void OnWidgetClosing(views::Widget* widget) override {
+    widget->RemoveObserver(this);
+    if (destruction_callback_) {
+      std::move(destruction_callback_).Run();
+    }
+  }
+
+ private:
+  base::OnceClosure destruction_callback_;
+};
+
+}  // namespace
+
+// Regression test for UAF in
+// DigitalIdentityProviderDesktop::EndRequestWithError when the owner is
+// synchronously destroyed during dialog close.
+IN_PROC_BROWSER_TEST_F(DigitalIdentityMultiStepDialogBrowserTest,
+                       EndRequestWithErrorOwnerDestroyedDuringDialogClose) {
+  auto provider = std::make_unique<TestDigitalIdentityProviderDesktop>();
+
+  base::RunLoop run_loop;
+  // Show the dialog via the real ShowQrCodeDialog flow.
+  provider->SetUpAndShowQrDialog(GetActiveWebContents(),
+                                 run_loop.QuitClosure());
+
+  // Retrieve the widget robustly using TestApi and EnsureDialogCreated.
+  // Wrap `TestApi` in a nested scope so it is destroyed before the message
+  // loop runs. Otherwise, when the loop runs and triggers the UAF teardown,
+  // the dialog is deleted, leaving `TestApi` holding a dangling raw_ptr.
+  views::Widget* widget = nullptr;
+  {
+    DigitalIdentityMultiStepDialog* dialog = provider->GetDialog();
+    DigitalIdentityMultiStepDialog::TestApi dialog_test_api(dialog);
+    widget = dialog_test_api.GetWidget();
+  }
+  ASSERT_TRUE(widget);
+  base::WeakPtr<views::Widget> weak_widget = widget->GetWeakPtr();
+
+  // Set up the observer to synchronously destroy the provider when the widget
+  // closes.
+  ProviderDestroyerOnWidgetClosingObserver observer(base::BindOnce(
+      [](std::unique_ptr<TestDigitalIdentityProviderDesktop>* provider) {
+        provider->reset();
+      },
+      base::Unretained(&provider)));
+  widget->AddObserver(&observer);
+
+  // Trigger cancellation. OnDialogCanceled() will PostTask a call to
+  // OnCanceled() -> EndRequestWithError().
+  static_cast<views::DialogDelegate*>(widget->widget_delegate())
+      ->CancelDialog();
+  ASSERT_FALSE(widget->IsClosed());
+
+  // Run the message loop to execute the posted tasks.
+  // This will run EndRequestWithError(), triggering the UAF if the bug exists.
+  run_loop.Run();
+
+  // Verify that the provider was safely destroyed (pointer is null).
+  EXPECT_FALSE(provider);
+
+  // Clean up if the widget is still alive.
+  if (weak_widget) {
+    weak_widget->RemoveObserver(&observer);
+    views::test::WidgetDestroyedWaiter(weak_widget.get()).Wait();
+  }
+}