[Backport] CVE-2020-6543: Use after free in task scheduling

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2308550:
Worker: Fix a race condition on task runner handling

WebSharedWorkerImpl accesses WorkerScheduler from the main thread to
take a task runner, and then dispatches a connect event to
SharedWorkerGlobalScope using the task runner.

This causes a race condition if close() is called on the global scope
on the worker thread while the task runner is being taken on the main
thread: close() call disposes of WorkerScheduler, and accessing the
scheduler after that is not allowed. See the issue for details.

To fix this, this CL makes WebSharedWorkerImpl capture the task runner
between starting a worker thread (initializing WorkerScheduler) and
posting a task to evaluate worker scripts that may call close(). This
ensures that WebSharedWorkerImpl accesses WorkerScheduler before the
scheduler is disposed of.

Bug: 1104046
Change-Id: I145cd39f706019c33220fcb01ed81f76963ffff0
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790284}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

2 files changed, 16 insertions, 4 deletions

diff --git a/chromium/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc b/chromium/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc
index 473ee5d3b6e..db54f019570 100644
--- a/chromium/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc
+++ b/chromium/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc
@@ -129,11 +129,9 @@ void WebSharedWorkerImpl::Connect(MessagePortChannel web_channel) {
   DCHECK(IsMainThread());
   if (asked_to_terminate_)
     return;
-  // The HTML spec requires to queue a connect event using the DOM manipulation
-  // task source.
-  // https://html.spec.whatwg.org/C/#shared-workers-and-the-sharedworker-interface
+
   PostCrossThreadTask(
-      *GetWorkerThread()->GetTaskRunner(TaskType::kDOMManipulation), FROM_HERE,
+      *task_runner_for_connect_event_, FROM_HERE,
       CrossThreadBindOnce(&WebSharedWorkerImpl::ConnectTaskOnWorkerThread,
                           WTF::CrossThreadUnretained(this),
                           WTF::Passed(std::move(web_channel))));
@@ -247,6 +245,18 @@ void WebSharedWorkerImpl::StartWorkerContext(
 
   GetWorkerThread()->Start(std::move(creation_params), thread_startup_data,
                            std::move(devtools_params));
+
+  // Capture the task runner for dispatching connect events. This is necessary
+  // for avoiding race condition with WorkerScheduler termination induced by
+  // close() call on SharedWorkerGlobalScope. See https://crbug.com/1104046 for
+  // details.
+  //
+  // The HTML spec requires to queue a connect event using the DOM manipulation
+  // task source.
+  // https://html.spec.whatwg.org/C/#shared-workers-and-the-sharedworker-interface
+  task_runner_for_connect_event_ =
+      GetWorkerThread()->GetTaskRunner(TaskType::kDOMManipulation);
+
   GetWorkerThread()->FetchAndRunClassicScript(
       script_request_url, outside_settings_object->CopyData(),
       nullptr /* outside_resource_timing_notifier */,
diff --git a/chromium/third_party/blink/renderer/core/exported/web_shared_worker_impl.h b/chromium/third_party/blink/renderer/core/exported/web_shared_worker_impl.h
index 3249c8529ef..9d603d19c43 100644
--- a/chromium/third_party/blink/renderer/core/exported/web_shared_worker_impl.h
+++ b/chromium/third_party/blink/renderer/core/exported/web_shared_worker_impl.h
@@ -104,6 +104,8 @@ class CORE_EXPORT WebSharedWorkerImpl final : public WebSharedWorker {
   // |client_| owns |this|.
   WebSharedWorkerClient* client_;
 
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_for_connect_event_;
+
   bool asked_to_terminate_ = false;
 
   base::WeakPtrFactory<WebSharedWorkerImpl> weak_ptr_factory_{this};