From 2b6b5a2a8d32b20c687bbac974f70bc46cc82934 Mon Sep 17 00:00:00 2001 From: Christian Dullweber Date: Mon, 10 Jan 2022 14:56:57 +0000 Subject: [Backport] CVE-2022-0468: Use after free in Payments (1/2) Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/3376968: Prevent use-after-free if a KeyedService is accessed after shutdown KeyedServices should not get created during shutdown. This is prevented in debug builds by a NOTREACHED() but only leads to a silent crash report in release builds. Considering that this can lead to security bugs, we should have a hard CHECK(false) to prevent services from being created during shutdown. Bug: 1252716 Change-Id: I079cb6d8da8bcebb0b0e369ad4f67e2764fbc986 Reviewed-by: Sylvain Defresne Reviewed-by: Colin Blundell Commit-Queue: Christian Dullweber Cr-Commit-Position: refs/heads/main@{#957059} Reviewed-by: Allan Sandfeld Jensen Reviewed-by: Michal Klocek --- chromium/components/keyed_service/core/dependency_manager.cc | 8 ++------ chromium/components/keyed_service/core/dependency_manager.h | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/chromium/components/keyed_service/core/dependency_manager.cc b/chromium/components/keyed_service/core/dependency_manager.cc index 9f603c6ebab..3513de1b558 100644 --- a/chromium/components/keyed_service/core/dependency_manager.cc +++ b/chromium/components/keyed_service/core/dependency_manager.cc @@ -157,15 +157,11 @@ void DependencyManager::DestroyFactoriesInOrder( void DependencyManager::AssertContextWasntDestroyed(void* context) const { if (dead_context_pointers_.find(context) != dead_context_pointers_.end()) { -#if DCHECK_IS_ON() - NOTREACHED() << "Attempted to access a context that was ShutDown(). " + // We want to see all possible use-after-destroy in production environment. + CHECK(false) << "Attempted to access a context that was ShutDown(). " << "This is most likely a heap smasher in progress. After " << "KeyedService::Shutdown() completes, your service MUST " << "NOT refer to depended services again."; -#else // DCHECK_IS_ON() - // We want to see all possible use-after-destroy in production environment. - base::debug::DumpWithoutCrashing(); -#endif // DCHECK_IS_ON() } } diff --git a/chromium/components/keyed_service/core/dependency_manager.h b/chromium/components/keyed_service/core/dependency_manager.h index 0c820676206..a316569ed65 100644 --- a/chromium/components/keyed_service/core/dependency_manager.h +++ b/chromium/components/keyed_service/core/dependency_manager.h @@ -73,8 +73,8 @@ class KEYED_SERVICE_EXPORT DependencyManager { void DestroyContextServices(void* context); // Runtime assertion called as a part of GetServiceForContext() to check if - // |context| is considered stale. This will NOTREACHED() or - // base::debug::DumpWithoutCrashing() depending on the DCHECK_IS_ON() value. + // |context| is considered stale. This will CHECK(false) to avoid a potential + // use-after-free from services created after context destruction. void AssertContextWasntDestroyed(void* context) const; // Marks |context| as live (i.e., not stale). This method can be called as a -- cgit v1.2.1