[Backport] CVE-2023-4351: Use after free in Network

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4733351:
NetworkContext: Don't access url_loader_factories_ during destruction

Move the contents of `url_loader_factories_` to a temporary variable in
the destructor of network::NetworkContext so that re-entrant calls to
DestroyURLLoaderFactory() don't happen after it has started being
destroyed.

BUG=1465833

Change-Id: I476f0865256bdcba4ec934688597e69991968f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4733351
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1177648}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/503194
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

2 files changed, 17 insertions, 7 deletions

diff --git a/chromium/services/network/network_context.cc b/chromium/services/network/network_context.cc
index 795b2d5d7d3..035ee94c9b4 100644
--- a/chromium/services/network/network_context.cc
+++ b/chromium/services/network/network_context.cc
@@ -477,6 +477,8 @@ NetworkContext::NetworkContext(
 }
 
 NetworkContext::~NetworkContext() {
+  is_destructing_ = true;
+
   // May be nullptr in tests.
   if (network_service_)
     network_service_->DeregisterNetworkContext(this);
@@ -518,6 +520,12 @@ NetworkContext::~NetworkContext() {
     }
 #endif  // BUILDFLAG(IS_CT_SUPPORTED)
   }
+
+  // Clear `url_loader_factories_` before deleting the contents, as it can
+  // result in re-entrant calls to DestroyURLLoaderFactory().
+  std::set<std::unique_ptr<cors::CorsURLLoaderFactory>,
+           base::UniquePtrComparator>
+      url_loader_factories = std::move(url_loader_factories_);
 }
 
 // static
@@ -641,6 +649,9 @@ void NetworkContext::DisableQuic() {
 
 void NetworkContext::DestroyURLLoaderFactory(
     cors::CorsURLLoaderFactory* url_loader_factory) {
+   if (is_destructing_) {
+    return;
+  }
   const int32_t process_id = url_loader_factory->process_id();
 
   auto it = url_loader_factories_.find(url_loader_factory);
diff --git a/chromium/services/network/network_context.h b/chromium/services/network/network_context.h
index a9c3c62457d..7d006d48646 100644
--- a/chromium/services/network/network_context.h
+++ b/chromium/services/network/network_context.h
@@ -726,15 +726,14 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
   // HttpAuthHandle via |NetworkContext::CreateHttpAuthHandlerFactory|.
   net::HttpAuthPreferences http_auth_merged_preferences_;
 
+  // True once the destructor has been called. Used to guard against re-entrant
+  // calls to DestroyURLLoaderFactory().
+  bool is_destructing_ = false;
+
   // CorsURLLoaderFactory assumes that fields owned by the NetworkContext always
   // live longer than the factory.  Therefore we want the factories to be
-  // destroyed before other fields above.  In particular:
-  // - This must be below |url_request_context_| so that the URLRequestContext
-  //   outlives all the URLLoaderFactories and URLLoaders that depend on it;
-  //   for the same reason, it must also be below |network_context_|.
-  // - This must be below |loader_count_per_process_| that is touched by
-  //   CorsURLLoaderFactory::DestroyURLLoader (see also
-  //   https://crbug.com/1174943).
+  // destroyed before other fields above. This is accomplished by explicitly
+  // clearing `url_loader_factories_` in the destructor.
   std::set<std::unique_ptr<cors::CorsURLLoaderFactory>,
            base::UniquePtrComparator>
       url_loader_factories_;