From fd549e15ff90ae4d7d9d2300ad042b91369609c2 Mon Sep 17 00:00:00 2001 From: Pedro Pontes Date: Fri, 18 Jun 2021 16:01:42 +0200 Subject: [PATCH 1/2] chore: cherry-pick 0e36d324d6ef from chromium --- patches/chromium/.patches | 1 + .../chromium/cherry-pick-0e36d324d6ef.patch | 102 ++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 patches/chromium/cherry-pick-0e36d324d6ef.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index c053da770c343..2e4b6288c5c8a 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -168,3 +168,4 @@ 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 diff --git a/patches/chromium/cherry-pick-0e36d324d6ef.patch b/patches/chromium/cherry-pick-0e36d324d6ef.patch new file mode 100644 index 0000000000000..bc7ae1f7ef43f --- /dev/null +++ b/patches/chromium/cherry-pick-0e36d324d6ef.patch @@ -0,0 +1,102 @@ +From 0e36d324d6ef42ddedf67bdd82001f31a47c994e Mon Sep 17 00:00:00 2001 +From: Asami Doi +Date: Thu, 10 Jun 2021 07:07:44 +0000 +Subject: [PATCH] [M86-LTS] 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 +Reviewed-by: Matt Falkenhagen +Cr-Original-Original-Commit-Position: refs/heads/master@{#887109} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2929401 +Reviewed-by: Krishna Govind +Reviewed-by: Ben Mason +Reviewed-by: Prudhvi Kumar Bommana +Commit-Queue: Krishna Govind +Owners-Override: Krishna Govind +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 +Reviewed-by: Artem Sumaneev +Commit-Queue: Victor-Gabriel Savu +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 c727f822..39ec83e 100644 +--- a/content/browser/service_worker/service_worker_container_host.cc ++++ b/content/browser/service_worker/service_worker_container_host.cc +@@ -148,7 +148,7 @@ + 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. +@@ -1235,7 +1235,7 @@ + } + } + 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 82ff3d7..c65ef635 100644 +--- a/content/browser/service_worker/service_worker_version.cc ++++ b/content/browser/service_worker/service_worker_version.cc +@@ -881,8 +881,7 @@ + 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 631003c2..973b7b4 100644 +--- a/content/browser/service_worker/service_worker_version.h ++++ b/content/browser/service_worker/service_worker_version.h +@@ -385,9 +385,12 @@ + 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: From 29a5db2cf441fb80331bf0a15e5e82b0ecc81295 Mon Sep 17 00:00:00 2001 From: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Date: Fri, 18 Jun 2021 14:13:54 +0000 Subject: [PATCH 2/2] chore: update patches --- .../chromium/cherry-pick-0e36d324d6ef.patch | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/patches/chromium/cherry-pick-0e36d324d6ef.patch b/patches/chromium/cherry-pick-0e36d324d6ef.patch index bc7ae1f7ef43f..800e88803009a 100644 --- a/patches/chromium/cherry-pick-0e36d324d6ef.patch +++ b/patches/chromium/cherry-pick-0e36d324d6ef.patch @@ -1,7 +1,7 @@ -From 0e36d324d6ef42ddedf67bdd82001f31a47c994e Mon Sep 17 00:00:00 2001 +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Asami Doi Date: Thu, 10 Jun 2021 07:07:44 +0000 -Subject: [PATCH] [M86-LTS] BFCache: remove a controllee stored in `bfcached_controllee_map_` +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 @@ -42,13 +42,12 @@ Reviewed-by: Artem Sumaneev Commit-Queue: Victor-Gabriel Savu 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 c727f822..39ec83e 100644 +index 5cb7b2da2c9a4c857c756589ba2c9eaf0dc2b67f..b91a0ec16483a38574ed7d63fdeb9133877d17ae 100644 --- a/content/browser/service_worker/service_worker_container_host.cc +++ b/content/browser/service_worker/service_worker_container_host.cc -@@ -148,7 +148,7 @@ +@@ -149,7 +149,7 @@ ServiceWorkerContainerHost::~ServiceWorkerContainerHost() { FrameTreeNodeIdRegistry::GetInstance()->Remove(fetch_request_window_id_); if (IsContainerForClient() && controller_) @@ -57,7 +56,7 @@ index c727f822..39ec83e 100644 // Remove |this| as an observer of ServiceWorkerRegistrations. // TODO(falken): Use ScopedObserver instead of this explicit call. -@@ -1235,7 +1235,7 @@ +@@ -1236,7 +1236,7 @@ void ServiceWorkerContainerHost::UpdateController( } } if (previous_version) @@ -67,10 +66,10 @@ index c727f822..39ec83e 100644 // 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 82ff3d7..c65ef635 100644 +index d360afea47c40f8c533ff52f46417371bd3a1ad3..7d057af1b760b67f061cbcb7436c1bfb651ded68 100644 --- a/content/browser/service_worker/service_worker_version.cc +++ b/content/browser/service_worker/service_worker_version.cc -@@ -881,8 +881,7 @@ +@@ -880,8 +880,7 @@ void ServiceWorkerVersion::RemoveControlleeFromBackForwardCacheMap( bfcached_controllee_map_.erase(client_uuid); } @@ -81,10 +80,10 @@ index 82ff3d7..c65ef635 100644 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 631003c2..973b7b4 100644 +index b7c3b5cc809c42111b0bf5bddff574197c9a77a8..90431fe256dc67d51ed1c01ec8202a335af78032 100644 --- a/content/browser/service_worker/service_worker_version.h +++ b/content/browser/service_worker/service_worker_version.h -@@ -385,9 +385,12 @@ +@@ -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);