[Backport] CVE-2022-3887: Use after free in Web Workers

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4004562:
Fix PauseOrFreezeOnWorkerThread with nested Worklets.

Worklets can use the same backing thread which means we can have
nested WorkerThreads paused. If a parent WorkerThread gets deallocated
make sure we don't access anything after it is deallocated once the
nested event queue is released.

BUG=1372695

(cherry picked from commit ff5696ba4bc0f8782e3de40e04685507d9f17fd2)

Change-Id: I176b8f750da5a41d19d1b3a623944d9a2ed4a441
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3953152
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1059485}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4004562
Cr-Commit-Position: refs/branch-heads/5249@{#906}
Cr-Branched-From: 4f7bea5de862aaa52e6bde5920755a9ef9db120b-refs/heads/main@{#1036826}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/443349
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

2 files changed, 34 insertions, 14 deletions

diff --git a/chromium/third_party/blink/renderer/core/workers/worker_thread.cc b/chromium/third_party/blink/renderer/core/workers/worker_thread.cc
index f2ecee885ab..8d43c0c132d 100644
--- a/chromium/third_party/blink/renderer/core/workers/worker_thread.cc
+++ b/chromium/third_party/blink/renderer/core/workers/worker_thread.cc
@@ -603,6 +603,7 @@ void WorkerThread::InitializeOnWorkerThread(
     const base::Optional<WorkerBackingThreadStartupData>& thread_startup_data,
     std::unique_ptr<WorkerDevToolsParams> devtools_params) {
   DCHECK(IsCurrentThread());
+  backing_thread_weak_factory_.emplace(this);
   worker_reporting_proxy_.WillInitializeWorkerContext();
   {
     TRACE_EVENT0("blink.worker", "WorkerThread::InitializeWorkerContext");
@@ -738,11 +739,13 @@ void WorkerThread::PrepareForShutdownOnWorkerThread() {
     SetThreadState(ThreadState::kReadyToShutdown);
   }
 
+  backing_thread_weak_factory_ = base::nullopt;
   if (pause_or_freeze_count_ > 0) {
     DCHECK(nested_runner_);
     pause_or_freeze_count_ = 0;
     nested_runner_->QuitNow();
   }
+  pause_handle_.reset();
 
   if (WorkerThreadDebugger* debugger = WorkerThreadDebugger::From(GetIsolate()))
     debugger->WorkerThreadDestroyed(this);
@@ -876,8 +879,7 @@ void WorkerThread::PauseOrFreezeOnWorkerThread(
   if (pause_or_freeze_count_ > 1)
     return;
 
-  std::unique_ptr<scheduler::WorkerScheduler::PauseHandle> pause_handle =
-      GetScheduler()->Pause();
+  pause_handle_ = GetScheduler()->Pause();
   {
     // Since the nested message loop runner needs to be created and destroyed on
     // the same thread we allocate and destroy a new message loop runner each
@@ -885,12 +887,19 @@ void WorkerThread::PauseOrFreezeOnWorkerThread(
     // the worker thread such that the resume/terminate can quit this runner.
     std::unique_ptr<Platform::NestedMessageLoopRunner> nested_runner =
         Platform::Current()->CreateNestedMessageLoopRunner();
-    base::AutoReset<Platform::NestedMessageLoopRunner*> nested_runner_autoreset(
-        &nested_runner_, nested_runner.get());
+    auto weak_this = backing_thread_weak_factory_->GetWeakPtr();
+    nested_runner_ = nested_runner.get();
     nested_runner->Run();
+
+    // Careful `this` may be destroyed.
+    if (!weak_this) {
+      return;
+    }
+    nested_runner_ = nullptr;
   }
   GlobalScope()->SetDefersLoadingForResourceFetchers(false);
   GlobalScope()->SetLifecycleState(mojom::FrameLifecycleState::kRunning);
+  pause_handle_.reset();
 }
 
 void WorkerThread::ResumeOnWorkerThread() {
diff --git a/chromium/third_party/blink/renderer/core/workers/worker_thread.h b/chromium/third_party/blink/renderer/core/workers/worker_thread.h
index d9b73ba10dc..5bb8a45571a 100644
--- a/chromium/third_party/blink/renderer/core/workers/worker_thread.h
+++ b/chromium/third_party/blink/renderer/core/workers/worker_thread.h
@@ -30,6 +30,7 @@
 #include <memory>
 
 #include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/waitable_event.h"
@@ -74,7 +75,7 @@ struct WorkerMainScriptLoadParameters;
 // abstract class. Multiple WorkerThreads may share one WorkerBackingThread for
 // worklets.
 //
-// WorkerThread start and termination must be initiated on the main thread and
+// WorkerThread start and termination must be initiated on the parent thread and
 // an actual task is executed on the worker thread.
 //
 // When termination starts, (debugger) tasks on WorkerThread are handled as
@@ -97,7 +98,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
   ~WorkerThread() override;
 
   // Starts the underlying thread and creates the global scope. Called on the
-  // main thread.
+  // parent thread.
   // Startup data for WorkerBackingThread is base::nullopt if |this| doesn't own
   // the underlying WorkerBackingThread.
   // TODO(nhiroki): We could separate WorkerBackingThread initialization from
@@ -109,14 +110,14 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
              std::unique_ptr<WorkerDevToolsParams>);
 
   // Posts a task to evaluate a top-level classic script on the worker thread.
-  // Called on the main thread after Start().
+  // Called on the parent thread after Start().
   void EvaluateClassicScript(const KURL& script_url,
                              const String& source_code,
                              std::unique_ptr<Vector<uint8_t>> cached_meta_data,
                              const v8_inspector::V8StackTraceId& stack_id);
 
   // Posts a task to fetch and run a top-level classic script on the worker
-  // thread. Called on the main thread after Start().
+  // thread. Called on the parent thread after Start().
   void FetchAndRunClassicScript(
       const KURL& script_url,
       std::unique_ptr<WorkerMainScriptLoadParameters>
@@ -127,7 +128,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
       const v8_inspector::V8StackTraceId& stack_id);
 
   // Posts a task to fetch and run a top-level module script on the worker
-  // thread. Called on the main thread after Start().
+  // thread. Called on the parent thread after Start().
   void FetchAndRunModuleScript(
       const KURL& script_url,
       std::unique_ptr<WorkerMainScriptLoadParameters>
@@ -148,7 +149,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
   // Terminates the worker thread. Subclasses of WorkerThread can override this
   // to do cleanup. The default behavior is to call Terminate() and
   // synchronously call EnsureScriptExecutionTerminates() to ensure the thread
-  // is quickly terminated. Called on the main thread.
+  // is quickly terminated. Called on the parent thread.
   virtual void TerminateForTesting();
 
   // Called on the main thread for the leak detector. Forcibly terminates the
@@ -183,7 +184,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
   void DebuggerTaskStarted();
   void DebuggerTaskFinished();
 
-  // Callable on both the main thread and the worker thread.
+  // Callable on both the parent thread and the worker thread.
   const base::UnguessableToken& GetDevToolsWorkerToken() const {
     return devtools_worker_token_;
   }
@@ -326,7 +327,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
   // already shutting down. Does not terminate if a debugger task is running,
   // because the debugger task is guaranteed to finish and it heavily uses V8
   // API calls which would crash after forcible script termination. Called on
-  // the main thread.
+  // the parent thread.
   void EnsureScriptExecutionTerminates(ExitCode) LOCKS_EXCLUDED(mutex_);
 
   // These are called in this order during worker thread startup.
@@ -409,7 +410,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
   // A unique identifier among all WorkerThreads.
   const int worker_thread_id_;
 
-  // Set on the main thread.
+  // Set on the parent thread.
   bool requested_to_terminate_ GUARDED_BY(mutex_) = false;
 
   ThreadState thread_state_ GUARDED_BY(mutex_) = ThreadState::kNotStarted;
@@ -442,7 +443,7 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
                                     TaskTypeTraits>;
   TaskRunnerHashMap worker_task_runners_;
 
-  // This lock protects shared states between the main thread and the worker
+  // This lock protects shared states between the parent thread and the worker
   // thread. See thread-safety annotations (e.g., GUARDED_BY) in this header
   // file.
   Mutex mutex_;
@@ -451,6 +452,10 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
   // only on the worker thread.
   int pause_or_freeze_count_ = 0;
 
+  // The `PauseHandle` needs to be destroyed before the scheduler is destroyed
+  // otherwise we will hit a DCHECK.
+  std::unique_ptr<scheduler::WorkerScheduler::PauseHandle> pause_handle_;
+
   // A nested message loop for handling pausing. Pointer is not owned. Used only
   // on the worker thread.
   Platform::NestedMessageLoopRunner* nested_runner_ = nullptr;
@@ -478,6 +483,12 @@ class CORE_EXPORT WorkerThread : public Thread::TaskObserver {
   HashSet<std::unique_ptr<InterruptData>> pending_interrupts_
       GUARDED_BY(mutex_);
 
+  // Since the WorkerThread is allocated and deallocated on the parent thread,
+  // we need a WeakPtrFactory that is allocated and cleared on the backing
+  // thread.
+  base::Optional<base::WeakPtrFactory<WorkerThread>>
+      backing_thread_weak_factory_;
+
   THREAD_CHECKER(parent_thread_checker_);
 };