diff options
| author | Matt Menke <mmenke@chromium.org> | 2021-06-10 07:05:24 +0000 |
|---|---|---|
| committer | Michael Brüning <michael.bruning@qt.io> | 2021-11-10 23:08:54 +0000 |
| commit | b249cd9b6455cafacb8f7c99d300cefc59881cc2 (patch) | |
| tree | 15a9ef33d19739037ccd7a0d6c9683599195b2b2 | |
| parent | 2910985befbf2a6fc0c56825af99d6d617cf4799 (diff) | |
| download | qtwebengine-chromium-b249cd9b6455cafacb8f7c99d300cefc59881cc2.tar.gz | |
[Backport] CVE-2021-30553: Use after free in Network service
Partial cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2949089:
Fix URLLoader cleanup on CorsURLLoaderFactory destruction.
Destroying one URLLoader can result in other URLLoaders getting errors,
due to to cache interconnectedness. CorsURLLoaderFactory's destructor
was not taking that into account.
Also fix a bonus bug: HttpCache::Transaction::response_ wasn't being
cleared in HttpCache::Transaction::DoHeadersPhaseCannotProceed(), which
could result in DCHECKs when calling GetResponseInfo() when a
transaction that was waiting on a cached response from another
transaction ended up failing.
[M86] Used older API in cors_url_loader_factory_unittest.cc
Added AddDefaultHandlers to EmbeddedTestServer
(cherry picked from commit 2f49a3c69a2184c95f43a395e4f33a3959cb8dbc)
(cherry picked from commit baf23e3c5b1394982cff718a0e055d4f239245ad)
Bug: 1209769
Change-Id: I2c18caa488767a29011aca1e1b0bace24c1ba8fc
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#887522}
Auto-Submit: Matt Menke <mmenke@chromium.org>
Cr-Original-Commit-Position: refs/branch-heads/4472@{#1433}
Cr-Original-Branched-From: 3d60439cfb36485e76a1c5bb7f513d3721b20da1-refs/heads/master@{#870763}
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Artem Sumaneev <asumaneev@google.com>
Owners-Override: Victor-Gabriel Savu <vsavu@google.com>
Commit-Queue: Victor-Gabriel Savu <vsavu@google.com>
Cr-Commit-Position: refs/branch-heads/4240@{#1662}
Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
| -rw-r--r-- | chromium/net/http/http_cache_transaction.cc | 1 | ||||
| -rw-r--r-- | chromium/services/network/cors/cors_url_loader_factory.cc | 12 |
2 files changed, 12 insertions, 1 deletions
diff --git a/chromium/net/http/http_cache_transaction.cc b/chromium/net/http/http_cache_transaction.cc index 835b976b178..9303583f974 100644 --- a/chromium/net/http/http_cache_transaction.cc +++ b/chromium/net/http/http_cache_transaction.cc @@ -2060,6 +2060,7 @@ int HttpCache::Transaction::DoHeadersPhaseCannotProceed(int result) { SetRequest(net_log_); entry_ = nullptr; + SetResponse(HttpResponseInfo()); // Bypass the cache for timeout scenario. if (result == ERR_CACHE_LOCK_TIMEOUT) diff --git a/chromium/services/network/cors/cors_url_loader_factory.cc b/chromium/services/network/cors/cors_url_loader_factory.cc index e677c8c020d..e8021ec8088 100644 --- a/chromium/services/network/cors/cors_url_loader_factory.cc +++ b/chromium/services/network/cors/cors_url_loader_factory.cc @@ -47,7 +47,17 @@ CORSURLLoaderFactory::CORSURLLoaderFactory( network_loader_factory_(std::move(network_loader_factory)), preflight_finalizer_(preflight_finalizer) {} -CORSURLLoaderFactory::~CORSURLLoaderFactory() = default; +CORSURLLoaderFactory::~CORSURLLoaderFactory() { + // Delete loaders one at a time, since deleting one loader can cause another + // loader waiting on it to fail synchronously, which could result in the other + // loader calling DestroyURLLoader(). + while (!loaders_.empty()) { + // No need to call context_->LoaderDestroyed(), since this method is only + // called from the NetworkContext's destructor, or when there are no + // remaining URLLoaders. + loaders_.erase(loaders_.begin()); + } +} void CORSURLLoaderFactory::OnLoaderCreated( std::unique_ptr<mojom::URLLoader> loader) { |