Skip to content

Commit

Permalink
chore: cherry-pick 0e36d324d6ef from chromium (#29777)
Browse files Browse the repository at this point in the history
* chore: cherry-pick 0e36d324d6ef from chromium

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: Electron Bot <electron@github.com>
  • Loading branch information
3 people committed Jun 21, 2021
1 parent 8dc6710 commit 8117de4
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -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
101 changes: 101 additions & 0 deletions patches/chromium/cherry-pick-0e36d324d6ef.patch
Original file line number Diff line number Diff line change
@@ -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:

0 comments on commit 8117de4

Please sign in to comment.