From 0333ecde91425e518cd898614c7b018209a18511 Mon Sep 17 00:00:00 2001
From: Alvin Ji <alvinji@chromium.org>
Date: Tue, 08 Apr 2025 10:46:18 -0700
Subject: [PATCH] usb: Use GlobalRenderFrameHostId in UsbChooserController

The UsbChooserController currently holds a raw pointer to the requesting
RenderFrameHost. This can lead to use-after-free issues if the
RenderFrameHost is destroyed before the chooser controller. This CL
replaces the raw pointer with a `GlobalRenderFrameHostId`. This ID can
be used to retrieve the RenderFrameHost when needed, and checks are
added to ensure the RenderFrameHost is still valid before accessing it.

Bug: 405292639
Change-Id: Ifedaf80f6700d57ea28691abfaf4d2ff9cdbb448
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6440254
Commit-Queue: Alvin Ji <alvinji@chromium.org>
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1444224}
---

diff --git a/chrome/browser/usb/usb_chooser_controller.cc b/chrome/browser/usb/usb_chooser_controller.cc
index 2654ad4f..4c64fc2d 100644
--- a/chrome/browser/usb/usb_chooser_controller.cc
+++ b/chrome/browser/usb/usb_chooser_controller.cc
@@ -100,8 +100,8 @@
           CreateChooserTitle(render_frame_host, IDS_USB_DEVICE_CHOOSER_PROMPT)),
       options_(std::move(options)),
       callback_(std::move(callback)),
-      requesting_frame_(render_frame_host) {
-  RenderFrameHost* main_frame = requesting_frame_->GetMainFrame();
+      render_frame_host_id_(render_frame_host->GetGlobalId()) {
+  RenderFrameHost* main_frame = render_frame_host->GetMainFrame();
   origin_ = main_frame->GetLastCommittedOrigin();
   Profile* profile =
       Profile::FromBrowserContext(main_frame->GetBrowserContext());
@@ -203,7 +203,15 @@
 void UsbChooserController::Close() {}
 
 void UsbChooserController::OpenHelpCenterUrl() const {
-  WebContents::FromRenderFrameHost(requesting_frame_)
+  content::RenderFrameHost* render_frame_host =
+      content::RenderFrameHost::FromID(render_frame_host_id_);
+  if (!render_frame_host) {
+    // When |render_frame_host| is not valid anymore we don't want to open help
+    // center url.
+    return;
+  }
+
+  WebContents::FromRenderFrameHost(render_frame_host)
       ->OpenURL(content::OpenURLParams(
                     GURL(chrome::kChooserUsbOverviewURL), content::Referrer(),
                     WindowOpenDisposition::NEW_FOREGROUND_TAB,
@@ -267,6 +275,14 @@
 
 bool UsbChooserController::DisplayDevice(
     const device::mojom::UsbDeviceInfo& device_info) const {
+  content::RenderFrameHost* render_frame_host =
+      content::RenderFrameHost::FromID(render_frame_host_id_);
+  if (!render_frame_host) {
+    // When |render_frame_host| is not valid anymore we don't want to display
+    // any device information.
+    return false;
+  }
+
   if (!device::UsbDeviceFilterMatchesAny(options_->filters, device_info)) {
     return false;
   }
@@ -281,10 +297,9 @@
   bool is_usb_unrestricted = false;
   if (base::FeatureList::IsEnabled(blink::features::kUnrestrictedUsb)) {
     is_usb_unrestricted =
-        requesting_frame_ &&
-        requesting_frame_->IsFeatureEnabled(
+        render_frame_host->IsFeatureEnabled(
             network::mojom::PermissionsPolicyFeature::kUsbUnrestricted) &&
-        content::HasIsolatedContextCapability(requesting_frame_);
+        content::HasIsolatedContextCapability(render_frame_host);
   }
   // Isolated context with permission to access the policy-controlled feature
   // "usb-unrestricted" can bypass the USB blocklist.
diff --git a/chrome/browser/usb/usb_chooser_controller.h b/chrome/browser/usb/usb_chooser_controller.h
index de5d7fb..8e1bb06 100644
--- a/chrome/browser/usb/usb_chooser_controller.h
+++ b/chrome/browser/usb/usb_chooser_controller.h
@@ -15,6 +15,7 @@
 #include "base/scoped_observation.h"
 #include "chrome/browser/usb/usb_chooser_context.h"
 #include "components/permissions/chooser_controller.h"
+#include "content/public/browser/global_routing_id.h"
 #include "services/device/public/mojom/usb_device.mojom.h"
 #include "third_party/blink/public/mojom/usb/web_usb_service.mojom.h"
 #include "url/origin.h"
@@ -64,8 +65,9 @@
   blink::mojom::WebUsbService::GetPermissionCallback callback_;
   url::Origin origin_;
 
-  const raw_ptr<content::RenderFrameHost, AcrossTasksDanglingUntriaged>
-      requesting_frame_;
+  // Hold the GlobalRenderFrameHostId for requesting frame so we can always
+  // check whether the frame host is still valid before we access it.
+  const content::GlobalRenderFrameHostId render_frame_host_id_;
   base::WeakPtr<UsbChooserContext> chooser_context_;
   base::ScopedObservation<UsbChooserContext, UsbChooserContext::DeviceObserver>
       observation_{this};