|
|
@@ -0,0 +1,101 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Asami Doi <[email protected]>
|
|
|
+Date: Thu, 10 Jun 2021 07:03:17 +0000
|
|
|
+Subject: BFCache: remove a controllee stored in `bfcached_controllee_map_`
|
|
|
+
|
|
|
+This CL fixes the UAF that happens with the following case:
|
|
|
+Let's assume we have 2 service workers (sw1.js and sw2.js) are
|
|
|
+registered in the same page. When the second service worker (sw2.js) is
|
|
|
+registered, ServiceWorkerContainerHost::UpdateController() is called
|
|
|
+and the previous SWVersion (sw1.js) removes a controllee from
|
|
|
+`controllee_map_`. If BackForwardCache is enabled, a controllee is
|
|
|
+stored in `bfcached_controllee_map_` instead and the controllee will
|
|
|
+not be removed in ServiceWorkerContainerHost::UpdateController().
|
|
|
+When ServiceWorkerContainerHost::UpdateController() is called and
|
|
|
+keep a controllee in `bfcached_controllee_map_`, and a page navigates to
|
|
|
+a different page (evicts BFCache), use-after-free (UAF) happens.
|
|
|
+
|
|
|
+This CL updates ServiceWorkerContainerHost::UpdateController()
|
|
|
+to remove a controllee from `bfcached_controllee_map_` if it exists.
|
|
|
+
|
|
|
+(cherry picked from commit a2414a05a486ca0ad18ba4caf78e883a668a0555)
|
|
|
+
|
|
|
+(cherry picked from commit 7cd7f6741fc4491c2f7ef21052a370ee23887e37)
|
|
|
+
|
|
|
+Bug: 1212618
|
|
|
+Change-Id: I13e023e6d273268a08ea9276a056f7f5acba39cd
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2919020
|
|
|
+Commit-Queue: Asami Doi <[email protected]>
|
|
|
+Reviewed-by: Matt Falkenhagen <[email protected]>
|
|
|
+Cr-Original-Original-Commit-Position: refs/heads/master@{#887109}
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2929401
|
|
|
+Reviewed-by: Krishna Govind <[email protected]>
|
|
|
+Reviewed-by: Ben Mason <[email protected]>
|
|
|
+Reviewed-by: Prudhvi Kumar Bommana <[email protected]>
|
|
|
+Commit-Queue: Krishna Govind <[email protected]>
|
|
|
+Owners-Override: Krishna Govind <[email protected]>
|
|
|
+Cr-Original-Commit-Position: refs/branch-heads/4472@{#1375}
|
|
|
+Cr-Original-Branched-From: 3d60439cfb36485e76a1c5bb7f513d3721b20da1-refs/heads/master@{#870763}
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2944946
|
|
|
+Owners-Override: Victor-Gabriel Savu <[email protected]>
|
|
|
+Reviewed-by: Achuith Bhandarkar <[email protected]>
|
|
|
+Commit-Queue: Victor-Gabriel Savu <[email protected]>
|
|
|
+Cr-Commit-Position: refs/branch-heads/4430@{#1512}
|
|
|
+Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950}
|
|
|
+
|
|
|
+diff --git a/content/browser/service_worker/service_worker_container_host.cc b/content/browser/service_worker/service_worker_container_host.cc
|
|
|
+index 0b1981efd9eb008caea1c94da138a2ed7c386bc5..9a2193ac5a4e8a738bef8e391145deecef9b9ac2 100644
|
|
|
+--- a/content/browser/service_worker/service_worker_container_host.cc
|
|
|
++++ b/content/browser/service_worker/service_worker_container_host.cc
|
|
|
+@@ -138,7 +138,7 @@ ServiceWorkerContainerHost::~ServiceWorkerContainerHost() {
|
|
|
+ }
|
|
|
+
|
|
|
+ if (IsContainerForClient() && controller_)
|
|
|
+- controller_->OnControlleeDestroyed(client_uuid());
|
|
|
++ controller_->Uncontrol(client_uuid());
|
|
|
+
|
|
|
+ // Remove |this| as an observer of ServiceWorkerRegistrations.
|
|
|
+ // TODO(falken): Use ScopedObserver instead of this explicit call.
|
|
|
+@@ -1244,7 +1244,7 @@ void ServiceWorkerContainerHost::UpdateController(
|
|
|
+ }
|
|
|
+ }
|
|
|
+ if (previous_version)
|
|
|
+- previous_version->RemoveControllee(client_uuid());
|
|
|
++ previous_version->Uncontrol(client_uuid());
|
|
|
+
|
|
|
+ // SetController message should be sent only for clients.
|
|
|
+ DCHECK(IsContainerForClient());
|
|
|
+diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
|
|
|
+index 798d0bd350976cf2724e4d4b726c24d63e0a5ec3..5e268a5331e80ff7c6c93c425171bbd31ddb342a 100644
|
|
|
+--- a/content/browser/service_worker/service_worker_version.cc
|
|
|
++++ b/content/browser/service_worker/service_worker_version.cc
|
|
|
+@@ -886,8 +886,7 @@ void ServiceWorkerVersion::RemoveControlleeFromBackForwardCacheMap(
|
|
|
+ bfcached_controllee_map_.erase(client_uuid);
|
|
|
+ }
|
|
|
+
|
|
|
+-void ServiceWorkerVersion::OnControlleeDestroyed(
|
|
|
+- const std::string& client_uuid) {
|
|
|
++void ServiceWorkerVersion::Uncontrol(const std::string& client_uuid) {
|
|
|
+ if (!IsBackForwardCacheEnabled()) {
|
|
|
+ RemoveControllee(client_uuid);
|
|
|
+ } else {
|
|
|
+diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h
|
|
|
+index cb5afe636f45b70fce2ff313641834bf7bddff30..a96d2c4575fa3879cf6133fbe656cb20745bd66a 100644
|
|
|
+--- a/content/browser/service_worker/service_worker_version.h
|
|
|
++++ b/content/browser/service_worker/service_worker_version.h
|
|
|
+@@ -394,9 +394,12 @@ class CONTENT_EXPORT ServiceWorkerVersion
|
|
|
+ void RestoreControlleeFromBackForwardCacheMap(const std::string& client_uuid);
|
|
|
+ // Called when a back-forward cached controllee is evicted or destroyed.
|
|
|
+ void RemoveControlleeFromBackForwardCacheMap(const std::string& client_uuid);
|
|
|
+- // Called when a controllee is destroyed. Remove controllee from whichever
|
|
|
+- // map it belongs to, or do nothing when it is already removed.
|
|
|
+- void OnControlleeDestroyed(const std::string& client_uuid);
|
|
|
++ // Called when this version should no longer be the controller of this client.
|
|
|
++ // Called when the controllee is destroyed or it changes controller. Removes
|
|
|
++ // controllee from whichever map it belongs to, or do nothing when it is
|
|
|
++ // already removed. This function is different from RemoveController(), which
|
|
|
++ // can only be called if the controllee is not in the back-forward cache map.
|
|
|
++ void Uncontrol(const std::string& client_uuid);
|
|
|
+
|
|
|
+ // Returns true if this version has a controllee.
|
|
|
+ // Note regarding BackForwardCache:
|
Pedro Pontes