App Service: Do not use a self-owned receiver on AppServiceInternals

App Service Internals uses a self-owned mojo receiver for communication from the WebUI page. This means that the handler can live longer than the Profile, which causes a UAF if any of the handler methods are called after the Profile is freed. Instead, this switches to use a Receiver owned by the WebUI, which matches the behavior of other WebUIs. Bug: 1323236 Change-Id: Ibd41db24f98ed428d7324379a129e8d86ba38ccb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3637239 Auto-Submit: Tim Sergeant <tsergeant@chromium.org> Reviewed-by: Christopher Lam <calamity@chromium.org> Commit-Queue: Christopher Lam <calamity@chromium.org> Cr-Commit-Position: refs/heads/main@{#1001372}
chromium · May 10, 2022 · 8524364 · 8524364
1 parent 46c13be
commit 8524364
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 7 deletions.
diff --git a/chrome/browser/ui/webui/app_service_internals/app_service_internals_page_handler_impl.cc b/chrome/browser/ui/webui/app_service_internals/app_service_internals_page_handler_impl.cc
@@ -20,8 +20,10 @@
 #include "third_party/abseil-cpp/absl/utility/utility.h"
 
 AppServiceInternalsPageHandlerImpl::AppServiceInternalsPageHandlerImpl(
-    Profile* profile)
-    : profile_(profile) {}
+    Profile* profile,
+    mojo::PendingReceiver<
+        mojom::app_service_internals::AppServiceInternalsPageHandler> receiver)
+    : profile_(profile), receiver_(this, std::move(receiver)) {}
 
 AppServiceInternalsPageHandlerImpl::~AppServiceInternalsPageHandlerImpl() =
     default;

diff --git a/chrome/browser/ui/webui/app_service_internals/app_service_internals_page_handler_impl.h b/chrome/browser/ui/webui/app_service_internals/app_service_internals_page_handler_impl.h
@@ -8,11 +8,16 @@
 #include "base/memory/raw_ptr.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/webui/app_service_internals/app_service_internals.mojom.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 
 class AppServiceInternalsPageHandlerImpl
     : public mojom::app_service_internals::AppServiceInternalsPageHandler {
  public:
-  explicit AppServiceInternalsPageHandlerImpl(Profile* profile);
+  explicit AppServiceInternalsPageHandlerImpl(
+      Profile* profile,
+      mojo::PendingReceiver<
+          mojom::app_service_internals::AppServiceInternalsPageHandler>
+          receiver);
   AppServiceInternalsPageHandlerImpl(
       const AppServiceInternalsPageHandlerImpl&) = delete;
   AppServiceInternalsPageHandlerImpl& operator=(
@@ -25,6 +30,8 @@ class AppServiceInternalsPageHandlerImpl
 
  private:
   raw_ptr<Profile> profile_;
+  mojo::Receiver<mojom::app_service_internals::AppServiceInternalsPageHandler>
+      receiver_;
 };
 
 #endif  // CHROME_BROWSER_UI_WEBUI_APP_SERVICE_INTERNALS_APP_SERVICE_INTERNALS_PAGE_HANDLER_IMPL_H_
diff --git a/chrome/browser/ui/webui/app_service_internals/app_service_internals_ui.cc b/chrome/browser/ui/webui/app_service_internals/app_service_internals_ui.cc
@@ -15,7 +15,6 @@
 #include "chrome/grit/app_service_internals_resources.h"
 #include "chrome/grit/app_service_internals_resources_map.h"
 #include "content/public/browser/web_ui_data_source.h"
-#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
 AppServiceInternalsUI::AppServiceInternalsUI(content::WebUI* web_ui)
     : ui::MojoWebUIController(web_ui), profile_(Profile::FromWebUI(web_ui)) {
@@ -34,9 +33,8 @@ void AppServiceInternalsUI::BindInterface(
     mojo::PendingReceiver<
         mojom::app_service_internals::AppServiceInternalsPageHandler>
         receiver) {
-  mojo::MakeSelfOwnedReceiver(
-      std::make_unique<AppServiceInternalsPageHandlerImpl>(profile_),
-      std::move(receiver));
+  handler_ = std::make_unique<AppServiceInternalsPageHandlerImpl>(
+      profile_, std::move(receiver));
 }
 
 WEB_UI_CONTROLLER_TYPE_IMPL(AppServiceInternalsUI)

diff --git a/chrome/browser/ui/webui/app_service_internals/app_service_internals_ui.h b/chrome/browser/ui/webui/app_service_internals/app_service_internals_ui.h
@@ -28,6 +28,8 @@ class AppServiceInternalsUI : public ui::MojoWebUIController {
   WEB_UI_CONTROLLER_TYPE_DECL();
 
   raw_ptr<Profile> profile_;
+  std::unique_ptr<mojom::app_service_internals::AppServiceInternalsPageHandler>
+      handler_;
 };
 
 #endif  // CHROME_BROWSER_UI_WEBUI_APP_SERVICE_INTERNALS_APP_SERVICE_INTERNALS_UI_H_