[Backport] CVE-2020-6454 2/2

Fix couple of ProcessManager UaF issues

Worker can legitimately fail to start, this CL clears worker's
PendingTasks when that happens.

In addition to this, this CL makes ServiceWorkerTaskQueue
factory dependent on ProcessManager factory as pending tasks
can call out to ProcessManager
(courtesy of https://crbug.com/1019161#c16) upon
ServiceWorkerTaskQueue's destruction.

This CL adds a test for this ensuring a worker's pending_tasks_
is cleared when start worker failure is seen. The test rejects a
service worker's install event to trigger the failure.

Bug: 1019161
Change-Id: I78e663eef7bc2b590bc44de0d57a54dc0158aec6
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

3 files changed, 28 insertions, 3 deletions

diff --git a/chromium/extensions/browser/service_worker_task_queue.cc b/chromium/extensions/browser/service_worker_task_queue.cc
index 087cdb0d519..d5492b22bf0 100644
--- a/chromium/extensions/browser/service_worker_task_queue.cc
+++ b/chromium/extensions/browser/service_worker_task_queue.cc
@@ -224,8 +224,17 @@ void ServiceWorkerTaskQueue::DidStartWorkerFail(
     return;
   }
 
-  // TODO(lazyboy): Handle failure cases.
-  DCHECK(false) << "DidStartWorkerFail: " << context_id.first.extension_id();
+  WorkerState* worker_state = GetWorkerState(context_id);
+  DCHECK(worker_state);
+  if (g_test_observer) {
+    g_test_observer->DidStartWorkerFail(context_id.first.extension_id(),
+                                        worker_state->pending_tasks_.size());
+  }
+  worker_state->pending_tasks_.clear();
+  // TODO(https://crbug/1062936): Needs more thought: extension would be in
+  // perma-broken state after this as the registration wouldn't be stored if
+  // this happens.
+  LOG(ERROR) << "DidStartWorkerFail " << context_id.first.extension_id();
 }
 
 void ServiceWorkerTaskQueue::DidInitializeServiceWorkerContext(
@@ -586,6 +595,16 @@ ServiceWorkerTaskQueue::GetCurrentSequence(
   return iter->second;
 }
 
+size_t ServiceWorkerTaskQueue::GetNumPendingTasksForTest(
+    const LazyContextId& lazy_context_id) {
+  auto current_sequence = GetCurrentSequence(lazy_context_id.extension_id());
+  if (!current_sequence)
+    return 0u;
+  const SequencedContextId context_id(lazy_context_id, *current_sequence);
+  WorkerState* worker_state = GetWorkerState(context_id);
+  return worker_state ? worker_state->pending_tasks_.size() : 0u;
+}
+
 ServiceWorkerTaskQueue::WorkerState* ServiceWorkerTaskQueue::GetWorkerState(
     const SequencedContextId& context_id) {
   auto worker_iter = worker_state_map_.find(context_id);
diff --git a/chromium/extensions/browser/service_worker_task_queue.h b/chromium/extensions/browser/service_worker_task_queue.h
index 76b22858cc5..c1a9dd17845 100644
--- a/chromium/extensions/browser/service_worker_task_queue.h
+++ b/chromium/extensions/browser/service_worker_task_queue.h
@@ -135,7 +135,9 @@ class ServiceWorkerTaskQueue : public KeyedService,
     // |will_register_service_worker| is true if a Service Worker will be
     // registered.
     virtual void OnActivateExtension(const ExtensionId& extension_id,
-                                     bool will_register_service_worker) = 0;
+                                     bool will_register_service_worker) {}
+    virtual void DidStartWorkerFail(const ExtensionId& extension_id,
+                                    size_t num_pending_tasks) {}
 
    private:
     DISALLOW_COPY_AND_ASSIGN(TestObserver);
@@ -143,6 +145,8 @@ class ServiceWorkerTaskQueue : public KeyedService,
 
   static void SetObserverForTest(TestObserver* observer);
 
+  size_t GetNumPendingTasksForTest(const LazyContextId& lazy_context_id);
+
  private:
   using SequencedContextId = std::pair<LazyContextId, ActivationSequence>;
 
diff --git a/chromium/extensions/browser/service_worker_task_queue_factory.cc b/chromium/extensions/browser/service_worker_task_queue_factory.cc
index b62d2c1c1e3..29ff874ef47 100644
--- a/chromium/extensions/browser/service_worker_task_queue_factory.cc
+++ b/chromium/extensions/browser/service_worker_task_queue_factory.cc
@@ -6,6 +6,7 @@
 
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "extensions/browser/extension_registry_factory.h"
+#include "extensions/browser/process_manager_factory.h"
 #include "extensions/browser/service_worker_task_queue.h"
 
 using content::BrowserContext;
@@ -28,6 +29,7 @@ ServiceWorkerTaskQueueFactory::ServiceWorkerTaskQueueFactory()
           "ServiceWorkerTaskQueue",
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(ExtensionRegistryFactory::GetInstance());
+  DependsOn(ProcessManagerFactory::GetInstance());
 }
 
 ServiceWorkerTaskQueueFactory::~ServiceWorkerTaskQueueFactory() {}