electron · zcbenz · Jun 21, 2021 · Jun 18, 2021 · Jun 18, 2021 · Jun 21, 2021
@@ -168,5 +168,6 @@ media_feeds_disable_media_feeds_and_related_features.patch
 remove_tabs_and_line_breaks_from_the_middle_of_app_names_when.patch
 autofill_fixed_refill_of_changed_form.patch
 x11_fix_window_enumeration_order_when_wm_doesn_t_set.patch
+cherry-pick-0e36d324d6ef.patch
 m86-lts_longtaskdetector_remove_container_mutation_during.patch
 m86-lts_reduce_memory_consumption_on.patch
@@ -0,0 +1,101 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Asami Doi <asamidoi@chromium.org>
+Date: Thu, 10 Jun 2021 07:07:44 +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 <asamidoi@chromium.org>
+Reviewed-by: Matt Falkenhagen <falken@chromium.org>
+Cr-Original-Original-Commit-Position: refs/heads/master@{#887109}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2929401
+Reviewed-by: Krishna Govind <govind@chromium.org>
+Reviewed-by: Ben Mason <benmason@chromium.org>
+Reviewed-by: Prudhvi Kumar Bommana <pbommana@google.com>
+Commit-Queue: Krishna Govind <govind@chromium.org>
+Owners-Override: Krishna Govind <govind@chromium.org>
+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/+/2948660
+Owners-Override: Victor-Gabriel Savu <vsavu@google.com>
+Reviewed-by: Artem Sumaneev <asumaneev@google.com>
+Commit-Queue: Victor-Gabriel Savu <vsavu@google.com>
+Cr-Commit-Position: refs/branch-heads/4240@{#1663}
+Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
+
+diff --git a/content/browser/service_worker/service_worker_container_host.cc b/content/browser/service_worker/service_worker_container_host.cc
+index 5cb7b2da2c9a4c857c756589ba2c9eaf0dc2b67f..b91a0ec16483a38574ed7d63fdeb9133877d17ae 100644
+--- a/content/browser/service_worker/service_worker_container_host.cc
++++ b/content/browser/service_worker/service_worker_container_host.cc
+@@ -149,7 +149,7 @@ ServiceWorkerContainerHost::~ServiceWorkerContainerHost() {
+     FrameTreeNodeIdRegistry::GetInstance()->Remove(fetch_request_window_id_);
+
+   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.
+@@ -1236,7 +1236,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 d360afea47c40f8c533ff52f46417371bd3a1ad3..7d057af1b760b67f061cbcb7436c1bfb651ded68 100644
+--- a/content/browser/service_worker/service_worker_version.cc
++++ b/content/browser/service_worker/service_worker_version.cc
+@@ -880,8 +880,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 b7c3b5cc809c42111b0bf5bddff574197c9a77a8..90431fe256dc67d51ed1c01ec8202a335af78032 100644
+--- a/content/browser/service_worker/service_worker_version.h
++++ b/content/browser/service_worker/service_worker_version.h
+@@ -390,9 +390,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: