Skip to content

Commit

Permalink
[M90-LTS] Manually post task to bind FileUtilitiesHost.
Browse files Browse the repository at this point in the history
The FileUtilitiesHost binder is posted to a separate sequence, and the
ServiceWorkerHost may be destroyed by the time the it runs, causing a
UAF.
This CL changes it so that, when we try to bind a new receiver, the
host's worker_process_id() is obtained first (on the service worker's
core thread) and then a task is posted to do the actual binding on a
USER_VISIBLE task runner.

Credit: This issue was first reported (with analysis) by
soulchen8650@gmail.com.

(cherry picked from commit e2123a8)

Bug: 1229298
Change-Id: I6d5c05a830ba30f6cb98bf2df70a3df3333f3dd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3041006
Commit-Queue: Tal Pressman <talp@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#903832}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3071365
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Zakhar Voit <voit@google.com>
Owners-Override: Achuith Bhandarkar <achuith@chromium.org>
Cr-Commit-Position: refs/branch-heads/4430@{#1564}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}
  • Loading branch information
talpr authored and Chromium LUCI CQ committed Aug 10, 2021
1 parent a9eab50 commit 995064c
Showing 1 changed file with 15 additions and 5 deletions.
20 changes: 15 additions & 5 deletions content/browser/browser_interface_binders.cc
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,22 @@ void BindTextSuggestionHostForFrame(
}
#endif

// Get the service worker's worker process ID and post a task to bind the
// receiver on a USER_VISIBLE task runner.
// This is necessary because:
// - Binding the host itself and checking the ID on the task's thread may cause
// a UAF if the host has been deleted in the meantime.
// - The process ID is not yet populated at the time `PopulateInterfaceBinders`
// is called.
void BindFileUtilitiesHost(
const ServiceWorkerHost* host,
ServiceWorkerHost* host,
mojo::PendingReceiver<blink::mojom::FileUtilitiesHost> receiver) {
FileUtilitiesHostImpl::Create(host->worker_process_id(), std::move(receiver));
auto task_runner = base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE});
task_runner->PostTask(
FROM_HERE,
base::BindOnce(&FileUtilitiesHostImpl::Create, host->worker_process_id(),
std::move(receiver)));
}

template <typename WorkerHost, typename Interface>
Expand Down Expand Up @@ -1151,9 +1163,7 @@ void PopulateServiceWorkerBinders(ServiceWorkerHost* host,

// static binders
map->Add<blink::mojom::FileUtilitiesHost>(
base::BindRepeating(&BindFileUtilitiesHost, host),
base::ThreadPool::CreateSequencedTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE}));
base::BindRepeating(&BindFileUtilitiesHost, host));
map->Add<shape_detection::mojom::BarcodeDetectionProvider>(
base::BindRepeating(&BindBarcodeDetectionProvider));
map->Add<shape_detection::mojom::FaceDetectionProvider>(
Expand Down

0 comments on commit 995064c

Please sign in to comment.