chrome: Fix potential UAF of ChromeBrowserContext and Profile (see issue #2969)

libcef/browser/browser_context.cc

@@ -229,7 +229,9 @@ void CefBrowserContext::RemoveCefRequestContext(
   // Delete ourselves when the reference count reaches zero.
   if (request_context_set_.empty()) {
     Shutdown();
-    delete this;
+
+    // Allow the current call stack to unwind before deleting |this|.
+    content::BrowserThread::DeleteSoon(CEF_UIT, FROM_HERE, this);
   }
 }
 

libcef/browser/browser_context.h

@@ -16,6 +16,7 @@
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/memory/weak_ptr.h"
+#include "base/sequenced_task_runner_helpers.h"
 #include "chrome/common/plugin.mojom.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -236,6 +237,9 @@ class CefBrowserContext {
   base::FilePath cache_path_;
 
  private:
+  // For DeleteSoon().
+  friend class base::DeleteHelper<CefBrowserContext>;
+
   scoped_refptr<CefIOThreadState> iothread_state_;
   CookieableSchemes cookieable_schemes_;
   std::unique_ptr<CefMediaRouterManager> media_router_manager_;

libcef/browser/chrome/chrome_browser_context.cc

@@ -8,6 +8,8 @@
 
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/off_the_record_profile_impl.h"
+#include "chrome/browser/profiles/profile_keep_alive_types.h"
+#include "chrome/browser/profiles/scoped_profile_keep_alive.h"
 
 ChromeBrowserContext::ChromeBrowserContext(
     const CefRequestContextSettings& settings)
@@ -16,15 +18,18 @@ ChromeBrowserContext::ChromeBrowserContext(
 ChromeBrowserContext::~ChromeBrowserContext() = default;
 
 content::BrowserContext* ChromeBrowserContext::AsBrowserContext() {
+  CHECK(!destroyed_);
   return profile_;
 }
 
 Profile* ChromeBrowserContext::AsProfile() {
+  CHECK(!destroyed_);
   return profile_;
 }
 
 bool ChromeBrowserContext::IsInitialized() const {
   CEF_REQUIRE_UIT();
+  CHECK(!destroyed_);
   return !!profile_;
 }
 
@@ -73,13 +78,21 @@ void ChromeBrowserContext::InitializeAsync(base::OnceClosure initialized_cb) {
 
 void ChromeBrowserContext::Shutdown() {
   CefBrowserContext::Shutdown();
+
+  // Allow potential deletion of the Profile at some future point (controlled
+  // by ProfileManager).
+  profile_keep_alive_.reset();
+
   // |g_browser_process| may be nullptr during shutdown.
-  if (should_destroy_ && g_browser_process) {
-    g_browser_process->profile_manager()
-        ->GetPrimaryUserProfile()
-        ->DestroyOffTheRecordProfile(profile_);
+  if (g_browser_process) {
+    if (should_destroy_) {
+      g_browser_process->profile_manager()
+          ->GetPrimaryUserProfile()
+          ->DestroyOffTheRecordProfile(profile_);
+    } else if (profile_) {
+      OnProfileWillBeDestroyed(profile_);
+    }
   }
-  profile_ = nullptr;
 }
 
 void ChromeBrowserContext::ProfileCreated(Profile* profile,
@@ -107,6 +120,9 @@ void ChromeBrowserContext::ProfileCreated(Profile* profile,
     // *CREATED isn't always sent for a disk-based profile that already
     // exists.
     profile_ = profile;
+    profile_->AddObserver(this);
+    profile_keep_alive_.reset(new ScopedProfileKeepAlive(
+        profile_, ProfileKeepAliveOrigin::kAppWindow));
   }
 
   if (status == Profile::CreateStatus::CREATE_STATUS_INITIALIZED) {
@@ -128,3 +144,10 @@ void ChromeBrowserContext::ProfileCreated(Profile* profile,
     }
   }
 }
+
+void ChromeBrowserContext::OnProfileWillBeDestroyed(Profile* profile) {
+  CHECK_EQ(profile_, profile);
+  profile_->RemoveObserver(this);
+  profile_ = nullptr;
+  destroyed_ = true;
+}

libcef/browser/chrome/chrome_browser_context.h

@@ -10,10 +10,13 @@
 
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile_observer.h"
+
+class ScopedProfileKeepAlive;
 
 // See CefBrowserContext documentation for usage. Only accessed on the UI thread
 // unless otherwise indicated.
-class ChromeBrowserContext : public CefBrowserContext {
+class ChromeBrowserContext : public CefBrowserContext, public ProfileObserver {
  public:
   explicit ChromeBrowserContext(const CefRequestContextSettings& settings);
 
@@ -26,6 +29,9 @@ class ChromeBrowserContext : public CefBrowserContext {
   void StoreOrTriggerInitCallback(base::OnceClosure callback) override;
   void Shutdown() override;
 
+  // ProfileObserver overrides.
+  void OnProfileWillBeDestroyed(Profile* profile) override;
+
  private:
   ~ChromeBrowserContext() override;
 
@@ -35,6 +41,9 @@ class ChromeBrowserContext : public CefBrowserContext {
   Profile* profile_ = nullptr;
   bool should_destroy_ = false;
 
+  bool destroyed_ = false;
+  std::unique_ptr<ScopedProfileKeepAlive> profile_keep_alive_;
+
   std::vector<base::OnceClosure> init_callbacks_;
 
   base::WeakPtrFactory<ChromeBrowserContext> weak_ptr_factory_;