From 3a3a5b7344f90754184eb9dc36e9c18c6d4bf35b Mon Sep 17 00:00:00 2001
From: Brian Johnson <34129+bridiver@users.noreply.github.com>
Date: Wed, 13 May 2026 01:56:34 -0700
Subject: [PATCH] Remove re-entrant call to lazy initialize adblock service
 (#36368)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1. ad_block_service() lazy initializes
2. Inside AdBlockService ctor -> AdBlockSubscriptionServiceManager ctor -> AdBlockSubscriptionDownloadManagerGetter -> MaybeGetDownloadManager-> builds the background download service -> DeferredClientWrapper::InflateClient → CreateAdBlockSubscriptionDownloadClient override
3. That override calls g_brave_browser_process->ad_block_service() again. Since ad_block_service_ is still null (assignment happens after make_unique returns), it constructs a second AdBlockService,
assigns it to ad_block_service_, and the client captures a raw_ptr to that inner instance's subscription_service_manager().
4. Outer make_unique finishes; assignment ad_block_service_ = ... overwrites the unique_ptr -> destroys inner instance.
5. Later, DeferredClientWrapper::DoRunDeferredClosures runs AdBlockSubscriptionDownloadClient::OnServiceInitialized -> dereferences dangling subscription_service_manager_ -> UAF.
---
 .../download/background_download_service_factory.cc    | 10 ++++++++--
 .../browser/ad_block_subscription_download_client.cc   |  8 +++++---
 .../browser/ad_block_subscription_download_client.h    | 10 ++++++----
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/chromium_src/chrome/browser/download/background_download_service_factory.cc b/chromium_src/chrome/browser/download/background_download_service_factory.cc
index d625d6d6d3c..d8f54020e96 100644
--- a/chromium_src/chrome/browser/download/background_download_service_factory.cc
+++ b/chromium_src/chrome/browser/download/background_download_service_factory.cc
@@ -5,6 +5,7 @@
 
 #include "chrome/browser/download/background_download_service_factory.h"
 
+#include "base/functional/bind.h"
 #include "brave/browser/brave_browser_process.h"
 #include "brave/components/brave_shields/content/browser/ad_block_service.h"
 #include "brave/components/brave_shields/content/browser/ad_block_subscription_download_client.h"
@@ -16,11 +17,16 @@
 
 namespace {
 
+brave_shields::AdBlockSubscriptionServiceManager*
+GetAdBlockSubscriptionServiceManager() {
+  return g_brave_browser_process->ad_block_service()
+      ->subscription_service_manager();
+}
+
 std::unique_ptr<download::Client> CreateAdBlockSubscriptionDownloadClient(
     Profile* profile) {
   return std::make_unique<brave_shields::AdBlockSubscriptionDownloadClient>(
-      g_brave_browser_process->ad_block_service()
-          ->subscription_service_manager());
+      base::BindRepeating(&GetAdBlockSubscriptionServiceManager));
 }
 
 }  // namespace
diff --git a/components/brave_shields/content/browser/ad_block_subscription_download_client.cc b/components/brave_shields/content/browser/ad_block_subscription_download_client.cc
index e56063a3cd0..1c3a3f50a48 100644
--- a/components/brave_shields/content/browser/ad_block_subscription_download_client.cc
+++ b/components/brave_shields/content/browser/ad_block_subscription_download_client.cc
@@ -22,15 +22,17 @@
 namespace brave_shields {
 
 AdBlockSubscriptionDownloadClient::AdBlockSubscriptionDownloadClient(
-    AdBlockSubscriptionServiceManager* subscription_manager)
-    : subscription_manager_(subscription_manager) {}
+    SubscriptionServiceManagerGetter subscription_manager_getter)
+    : subscription_manager_getter_(std::move(subscription_manager_getter)) {}
 
 AdBlockSubscriptionDownloadClient::~AdBlockSubscriptionDownloadClient() =
     default;
 
 AdBlockSubscriptionDownloadManager*
 AdBlockSubscriptionDownloadClient::GetAdBlockSubscriptionDownloadManager() {
-  return subscription_manager_->download_manager();
+  auto* subscription_manager = subscription_manager_getter_.Run();
+  return subscription_manager ? subscription_manager->download_manager()
+                              : nullptr;
 }
 
 void AdBlockSubscriptionDownloadClient::OnServiceInitialized(
diff --git a/components/brave_shields/content/browser/ad_block_subscription_download_client.h b/components/brave_shields/content/browser/ad_block_subscription_download_client.h
index a58b2254977..27ba0dbcc2c 100644
--- a/components/brave_shields/content/browser/ad_block_subscription_download_client.h
+++ b/components/brave_shields/content/browser/ad_block_subscription_download_client.h
@@ -9,7 +9,7 @@
 #include <string>
 #include <vector>
 
-#include "base/memory/raw_ptr.h"
+#include "base/functional/callback.h"
 #include "components/download/public/background_service/client.h"
 #include "components/download/public/background_service/download_metadata.h"
 
@@ -21,8 +21,11 @@ class AdBlockSubscriptionDownloadManager;
 
 class AdBlockSubscriptionDownloadClient : public download::Client {
  public:
+  using SubscriptionServiceManagerGetter =
+      base::RepeatingCallback<AdBlockSubscriptionServiceManager*()>;
+
   explicit AdBlockSubscriptionDownloadClient(
-      AdBlockSubscriptionServiceManager* subscription_manager);
+      SubscriptionServiceManagerGetter subscription_manager_getter);
   ~AdBlockSubscriptionDownloadClient() override;
   AdBlockSubscriptionDownloadClient(const AdBlockSubscriptionDownloadClient&) =
       delete;
@@ -49,8 +52,7 @@ class AdBlockSubscriptionDownloadClient : public download::Client {
   // Returns the AdBlockSubscriptionDownloadManager for the profile.
   AdBlockSubscriptionDownloadManager* GetAdBlockSubscriptionDownloadManager();
 
-  raw_ptr<AdBlockSubscriptionServiceManager> subscription_manager_ =
-      nullptr;  // NOT OWNED
+  SubscriptionServiceManagerGetter subscription_manager_getter_;
 };
 
 }  // namespace brave_shields