summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYoshisato Yanagisawa <yyanagisawa@chromium.org>2022-07-20 07:35:03 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2022-08-03 10:07:06 +0000
commit45abf85387307e2937b25445a7c6cbdb920e1ecc (patch)
treedee6a003e36429344ec2d9907e7dc48539fa3c8c
parent51f8868a8cf034df7dc8f2d72924b7197418b0c2 (diff)
downloadqtwebengine-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.cc25
-rw-r--r--chromium/content/browser/service_worker/service_worker_version.h2
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);