diff options
author | Yoshisato Yanagisawa <yyanagisawa@chromium.org> | 2022-07-20 07:35:03 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2022-08-03 10:07:06 +0000 |
commit | 45abf85387307e2937b25445a7c6cbdb920e1ecc (patch) | |
tree | dee6a003e36429344ec2d9907e7dc48539fa3c8c | |
parent | 51f8868a8cf034df7dc8f2d72924b7197418b0c2 (diff) | |
download | qtwebengine-chromium-45abf85387307e2937b25445a7c6cbdb920e1ecc.tar.gz |
[Backport] CVE-2022-2480 : Use after free in Service Worker API
Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/3766745:
[M96-LTS] Keep refptr to ServiceWorkerVersion in MaybeTimeoutRequest
The callback in ServiceWorkerVersion::MaybeTimeoutRequest may reduce the
reference to the version object, which can be the last reference to it.
In thet case, the object can be destroyed and the `inflight_requests_`
field access after the callback become invalid.
This CL keeps this to avoid the object destruction.
(cherry picked from commit 5926fa916d9ad53c77e31ee757e1979275d7466c)
Bug: 1339844
Change-Id: I6564627bad0527dea007ca73261c5636dab56755
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1023475}
Reviewed-by: Simon Hangl <simonha@google.com>
Owners-Override: Simon Hangl <simonha@google.com>
Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com>
Cr-Commit-Position: refs/branch-heads/4664@{#1663}
Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>
-rw-r--r-- | chromium/content/browser/service_worker/service_worker_version.cc | 25 | ||||
-rw-r--r-- | chromium/content/browser/service_worker/service_worker_version.h | 2 |
2 files changed, 16 insertions, 11 deletions
diff --git a/chromium/content/browser/service_worker/service_worker_version.cc b/chromium/content/browser/service_worker/service_worker_version.cc index 218fe63c7f7..93f06084a9e 100644 --- a/chromium/content/browser/service_worker/service_worker_version.cc +++ b/chromium/content/browser/service_worker/service_worker_version.cc @@ -1992,6 +1992,11 @@ void ServiceWorkerVersion::OnTimeoutTimer() { MarkIfStale(); + // Global `this` protecter. + // callbacks initiated by this function sometimes reduce refcnt to 0 + // to make this instance freed. + scoped_refptr<ServiceWorkerVersion> protect_this(this); + // Stopping the worker hasn't finished within a certain period. if (GetTickDuration(stop_time_) > kStopWorkerTimeout) { DCHECK_EQ(EmbeddedWorkerStatus::STOPPING, running_status()); @@ -2002,12 +2007,6 @@ void ServiceWorkerVersion::OnTimeoutTimer() { ReportError(blink::ServiceWorkerStatusCode::kErrorTimeout, "DETACH_STALLED_IN_STOPPING"); - // Detach the worker. Remove |this| as a listener first; otherwise - // OnStoppedInternal might try to restart before the new worker - // is created. Also, protect |this|, since swapping out the - // EmbeddedWorkerInstance could destroy our ServiceWorkerHost which could in - // turn destroy |this|. - scoped_refptr<ServiceWorkerVersion> protect_this(this); embedded_worker_->RemoveObserver(this); embedded_worker_->Detach(); embedded_worker_ = std::make_unique<EmbeddedWorkerInstance>(this); @@ -2034,7 +2033,6 @@ void ServiceWorkerVersion::OnTimeoutTimer() { DCHECK(running_status() == EmbeddedWorkerStatus::STARTING || running_status() == EmbeddedWorkerStatus::STOPPING) << static_cast<int>(running_status()); - scoped_refptr<ServiceWorkerVersion> protect(this); FinishStartWorker(blink::ServiceWorkerStatusCode::kErrorTimeout); if (running_status() == EmbeddedWorkerStatus::STARTING) embedded_worker_->Stop(); @@ -2043,17 +2041,22 @@ void ServiceWorkerVersion::OnTimeoutTimer() { // Requests have not finished before their expiration. bool stop_for_timeout = false; - auto timeout_iter = request_timeouts_.begin(); - while (timeout_iter != request_timeouts_.end()) { + std::set<InflightRequestTimeoutInfo> request_timeouts; + request_timeouts.swap(request_timeouts_); + auto timeout_iter = request_timeouts.begin(); + while (timeout_iter != request_timeouts.end()) { const InflightRequestTimeoutInfo& info = *timeout_iter; - if (!RequestExpired(info.expiration)) + if (!RequestExpired(info.expiration)) { break; + } if (MaybeTimeoutRequest(info)) { stop_for_timeout = stop_for_timeout || info.timeout_behavior == KILL_ON_TIMEOUT; } - timeout_iter = request_timeouts_.erase(timeout_iter); + timeout_iter = request_timeouts.erase(timeout_iter); } + DCHECK(request_timeouts_.empty()); + request_timeouts_.swap(request_timeouts); if (stop_for_timeout && running_status() != EmbeddedWorkerStatus::STOPPING) embedded_worker_->Stop(); diff --git a/chromium/content/browser/service_worker/service_worker_version.h b/chromium/content/browser/service_worker/service_worker_version.h index 33da2093ba8..b71e5386747 100644 --- a/chromium/content/browser/service_worker/service_worker_version.h +++ b/chromium/content/browser/service_worker/service_worker_version.h @@ -869,6 +869,8 @@ class CONTENT_EXPORT ServiceWorkerVersion bool is_browser_startup_complete, blink::ServiceWorkerStatusCode status); + // The caller of MaybeTimeoutRequest must increase reference count of |this| + // to avoid it deleted during the execution. bool MaybeTimeoutRequest(const InflightRequestTimeoutInfo& info); void SetAllRequestExpirations(const base::TimeTicks& expiration); |