Moderate firefox Logic Error 🔧 Commit mapped

Overview

Moderate
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionUse-after-free in the DOM: Workers component
ComponentDOM
Bug ClassLogic Error
Tracker2068407
Fix commit4e1e928adb4b (firefox) +108/-37
CISA KEVNot listed
CreditedMozilla
Disclosed2026-10-13

Changed Functions

FunctionChangeNotes
for
dom/workers/ScriptLoader.cpp
modified
if
dom/workers/ScriptLoader.cpp
modified
if
dom/workers/loader/CacheLoadHandler.cpp
modified
nsIReferrerInfo
dom/workers/loader/WorkerLoadContext.h
modified
nsIURI
dom/workers/loader/WorkerLoadContext.h
modified

Files Changed

  • dom/workers/ScriptLoader.cpp
  • dom/workers/loader/CacheLoadHandler.cpp
  • dom/workers/loader/NetworkLoadHandler.cpp
  • dom/workers/loader/WorkerLoadContext.cpp
  • dom/workers/loader/WorkerLoadContext.h
diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp
index c27c7ecf4c7..decb34da82a 100644
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -1579,7 +1579,9 @@ void ScriptLoaderRunnable::CancelMainThread(nsresult aCancelResult) {
     mCancelMainThread = Some(aCancelResult);
 
     for (ThreadSafeRequestHandle* handle : mLoadingRequests) {
-      if (handle->IsEmpty()) {
+      // Finishing an earlier request can have scheduled execution of this one,
+      // handing it over to the worker thread.
+      if (handle->ExecutionScheduled()) {
         continue;
       }
 
@@ -1623,7 +1625,7 @@ void ScriptLoaderRunnable::DispatchProcessPendingRequests() {
     // unset.
     auto firstItToExecute = std::find_if(
         begin, end, [](const RefPtr<ThreadSafeRequestHandle>& requestHandle) {
-          return !requestHandle->mExecutionScheduled;
+          return !requestHandle->ExecutionScheduled();
         });
 
     if (firstItToExecute == end) {
@@ -1636,13 +1638,13 @@ void ScriptLoaderRunnable::DispatchProcessPendingRequests() {
     const auto firstItUnexecutable =
         std::find_if(firstItToExecute, end,
                      [](RefPtr<ThreadSafeRequestHandle>& requestHandle) {
-                       MOZ_ASSERT(!requestHandle->IsEmpty());
+                       MOZ_ASSERT(!requestHandle->ExecutionScheduled());
                        if (!requestHandle->Finished()) {
                          return true;
                        }
 
                        // We can execute this one.
-                       requestHandle->mExecutionScheduled = true;
+                       requestHandle->SetExecutionScheduled();
 
                        return false;
                      });
diff --git a/dom/workers/loader/CacheLoadHandler.cpp b/dom/workers/loader/CacheLoadHandler.cpp
index 0416272e539..a2d45457f39 100644
--- a/dom/workers/loader/CacheLoadHandler.cpp
+++ b/dom/workers/loader/CacheLoadHandler.cpp
@@ -47,7 +47,7 @@ void CachePromiseHandler::ResolvedCallback(JSContext* aCx,
                                            ErrorResult& aRv) {
   AssertIsOnMainThread();
   // skip to schedule execution if it has been scheduled already.
-  if (mRequestHandle->mExecutionScheduled) {
+  if (mRequestHandle->ExecutionScheduled()) {
     return;
   }
   WorkerLoadContext* loadContext = mRequestHandle->GetContext();
@@ -71,7 +71,7 @@ void CachePromiseHandler::RejectedCallback(JSContext* aCx,
                                            ErrorResult& aRv) {
   AssertIsOnMainThread();
   // skip to schedule execution if it has been scheduled already.
-  if (mRequestHandle->mExecutionScheduled) {
+  if (mRequestHandle->ExecutionScheduled()) {
     return;
   }
   WorkerLoadContext* loadContext = mRequestHandle->GetContext();
@@ -256,19 +256,19 @@ void CacheLoadHandler::Fail(nsresult aRv) {
 
   mFailed = true;
 
+  // The request may already belong to the worker thread, in which case we can
+  // only stop the read and must leave the request alone.
+  const bool executionScheduled = mRequestHandle->ExecutionScheduled();
+
   if (mPump) {
-    MOZ_ASSERT_IF(!mRequestHandle->IsEmpty(),
+    MOZ_ASSERT_IF(!executionScheduled,
                   mRequestHandle->GetContext()->mCacheStatus ==
                       WorkerLoadContext::ReadingFromCache);
     mPump->Cancel(aRv);
     mPump = nullptr;
   }
 
-  if (mRequestHandle->mExecutionScheduled) {
-    return;
-  }
-
-  if (mRequestHandle->IsEmpty()) {
+  if (executionScheduled) {
     return;
   }
 
@@ -301,7 +301,13 @@ void CacheLoadHandler::Fail(nsresult aRv) {
 void CacheLoadHandler::Load(Cache* aCache) {
   AssertIsOnMainThread();
   MOZ_ASSERT(aCache);
-  MOZ_ASSERT(!mRequestHandle->IsEmpty());
+
+  // The load can have been cancelled, and the request handed over to the
+  // worker thread, while CacheStorage::Open() was still pending.
+  if (mRequestHandle->ExecutionScheduled()) {
+    return;
+  }
+
   WorkerLoadContext* loadContext = mRequestHandle->GetContext();
 
   nsCOMPtr<nsIURI> uri;
@@ -343,10 +349,9 @@ void CacheLoadHandler::RejectedCallback(JSContext* aCx,
                                         JS::Handle<JS::Value> aValue,
                                         ErrorResult& aRv) {
   AssertIsOnMainThread();
-  MOZ_ASSERT(!mRequestHandle->IsEmpty());
-
-  MOZ_ASSERT(mRequestHandle->GetContext()->mCacheStatus ==
-             WorkerLoadContext::Uncached);
+  MOZ_ASSERT_IF(!mRequestHandle->ExecutionScheduled(),
+                mRequestHandle->GetContext()->mCacheStatus ==
+                    WorkerLoadContext::Uncached);
   Fail(NS_ERROR_FAILURE);
 }
 
@@ -354,15 +359,16 @@ void CacheLoadHandler::ResolvedCallback(JSContext* aCx,
                                         JS::Handle<JS::Value> aValue,
                                         ErrorResult& aRv) {
   AssertIsOnMainThread();
-  MOZ_ASSERT(!mRequestHandle->IsEmpty());
-  WorkerLoadContext* loadContext = mRequestHandle->GetContext();
 
   // If we have already called 'Fail', we should not proceed. If we cancelled,
-  // we should similarily not proceed.
-  if (mFailed) {
+  // we should similarily not proceed. The request may also have been handed
+  // over to the worker thread while Cache::Match() was pending.
+  if (mFailed || mRequestHandle->ExecutionScheduled()) {
     return;
   }
 
+  WorkerLoadContext* loadContext = mRequestHandle->GetContext();
+
   MOZ_ASSERT(loadContext->mCacheStatus == WorkerLoadContext::Uncached);
 
   nsresult rv;
@@ -504,9 +510,16 @@ CacheLoadHandler::OnStreamComplete(nsIStreamLoader* aLoader,
                                    uint32_t aStringLen,
                                    const uint8_t* aString) {
   AssertIsOnMainThread();
-  if (mRequestHandle->IsEmpty()) {
+
+  // The load can be finished, and the request handed over to the worker
+  // thread, before the stream we are reading from the cache completes; see
+  // ScriptLoaderRunnable::CancelMainThread(). From then on the request belongs
+  // to the worker thread and we must not touch it.
+  if (mRequestHandle->ExecutionScheduled()) {
+    mPump = nullptr;
     return NS_OK;
   }
+
   WorkerLoadContext* loadContext = mRequestHandle->GetContext();
 
   mPump = nullptr;
@@ -539,7 +552,7 @@ nsresult CacheLoadHandler::DataReceivedFromCache(
     const nsACString& aReferrerPolicyHeaderValue,
     const nsACString& aReportingEndpointsHeaderValue) {
   AssertIsOnMainThread();
-  if (mRequestHandle->IsEmpty()) {
+  if (mRequestHandle->ExecutionScheduled()) {
     return NS_OK;
   }
   WorkerLoadContext* loadContext = mRequestHandle->GetContext();
@@ -641,7 +654,7 @@ nsresult CacheLoadHandler::DataReceivedFromCache(
 }
 
 nsresult CacheLoadHandler::DataReceived() {
-  MOZ_ASSERT(!mRequestHandle->IsEmpty());
+  MOZ_ASSERT(!mRequestHandle->ExecutionScheduled());
   WorkerLoadContext* loadContext = mRequestHandle->GetContext();
 
   if (loadContext->IsTopLevel()) {
diff --git a/dom/workers/loader/NetworkLoadHandler.cpp b/dom/workers/loader/NetworkLoadHandler.cpp
index 1944d74a93f..a388afa7308 100644
--- a/dom/workers/loader/NetworkLoadHandler.cpp
+++ b/dom/workers/loader/NetworkLoadHandler.cpp
@@ -53,9 +53,13 @@ NetworkLoadHandler::OnStreamComplete(nsIStreamLoader* aLoader,
                                      nsISupports* aContext, nsresult aStatus,
                                      uint32_t aStringLen,
                                      const uint8_t* aString) {
-  // If we have cancelled, or we have no mRequest, it means that the loader has
-  // shut down and we can exit early. If the cancel result is still NS_OK
-  if (mRequestHandle->IsEmpty()) {
+  AssertIsOnMainThread();
+
+  // If the request has been handed over to the worker thread, it means that
+  // the loader has shut down and we can exit early. This happens when
+  // cancelling the channel failed and the load was finished without us; see
+  // ScriptLoaderRunnable::CancelMainThread().
+  if (mRequestHandle->ExecutionScheduled()) {
     return NS_OK;
   }
   nsresult rv = DataReceivedFromNetwork(aLoader, aStatus, aStringLen, aString);
@@ -67,7 +71,7 @@ nsresult NetworkLoadHandler::DataReceivedFromNetwork(nsIStreamLoader* aLoader,
                                                      uint32_t aStringLen,
                                                      const uint8_t* aString) {
   AssertIsOnMainThread();
-  MOZ_ASSERT(!mRequestHandle->IsEmpty());
+  MOZ_ASSERT(!mRequestHandle->ExecutionScheduled());
 
   if (aStringLen > GetWorkerScriptMaxSizeInBytes()) {
     Document* parentDoc = mWorkerRef->Private()->GetDocument();
@@ -337,15 +341,17 @@ NetworkLoadHandler::OnStartRequest(nsIRequest* aRequest) {
 
 nsresult NetworkLoadHandler::PrepareForRequest(nsIRequest* aRequest) {
   AssertIsOnMainThread();
-  MOZ_ASSERT(!mRequestHandle->IsEmpty());
-  WorkerLoadContext* loadContext = mRequestHandle->GetContext();
 
   // If one load info cancels or hits an error, it can race with the start
-  // callback coming from another load info.
-  if (mRequestHandle->IsCancelled()) {
+  // callback coming from another load info. The request may even have been
+  // handed over to the worker thread already, in which case we must not touch
+  // it any more and let the caller cancel this channel.
+  if (mRequestHandle->ExecutionScheduled() || mRequestHandle->IsCancelled()) {
     return NS_ERROR_FAILURE;
   }
 
+  WorkerLoadContext* loadContext = mRequestHandle->GetContext();
+
   nsCOMPtr<nsIChannel> channel = do_QueryInterface(aRequest);
 
   // Checking the MIME type is only required for ServiceWorkers'
diff --git a/dom/workers/loader/WorkerLoadContext.cpp b/dom/workers/loader/WorkerLoadContext.cpp
index 6b807ff5976..d14552a6e48 100644
--- a/dom/workers/loader/WorkerLoadContext.cpp
+++ b/dom/workers/loader/WorkerLoadContext.cpp
@@ -31,9 +31,20 @@ ThreadSafeRequestHandle::ThreadSafeRequestHandle(
     : mRequest(aRequest), mOwningEventTarget(aSyncTarget) {}
 
 WorkerLoadContext* ThreadSafeRequestHandle::GetContext() {
+  AssertRequestNotHandedOff();
   return mRequest->GetWorkerLoadContext();
 }
 
+bool ThreadSafeRequestHandle::ExecutionScheduled() const {
+  AssertIsOnMainThread();
+  return mExecutionScheduled;
+}
+
+void ThreadSafeRequestHandle::SetExecutionScheduled() {
+  AssertIsOnMainThread();
+  mExecutionScheduled = true;
+}
+
 void ThreadSafeRequestHandle::SetRunnable(
     workerinternals::loader::ScriptLoaderRunnable* aRunnable) {
   MutexAutoLock lock(mMutex);
@@ -124,11 +135,20 @@ ThreadSafeRequestHandle::GetCacheCreator() {
 }
 
 ThreadSafeRequestHandle::~ThreadSafeRequestHandle() {
+  // Nothing can be racing with us any more, but ReleaseRequest() may have
+  // cleared mRequest on the worker thread, so take mMutex once to make sure we
+  // observe that write rather than a stale pointer to an already freed request.
+  bool hasRequest;
+  {
+    MutexAutoLock lock(mMutex);
+    hasRequest = !!mRequest;
+  }
+
   // Normally we only touch mStrongRef on the owning thread.  This is safe,
   // however, because when we do use mStrongRef on the owning thread we are
   // always holding a strong ref to the ThreadsafeHandle via the owning
   // runnable.  So we cannot run the ThreadsafeHandle destructor simultaneously.
-  if (!mRequest || mOwningEventTarget->IsOnCurrentThread()) {
+  if (!hasRequest || mOwningEventTarget->IsOnCurrentThread()) {
     return;
   }
 
diff --git a/dom/workers/loader/WorkerLoadContext.h b/dom/workers/loader/WorkerLoadContext.h
index ebe0a045b08..7e271911dc5 100644
--- a/dom/workers/loader/WorkerLoadContext.h
+++ b/dom/workers/loader/WorkerLoadContext.h
@@ -14,6 +14,7 @@
 #include "nsIChannel.h"
 #include "nsIInputStream.h"
 #include "nsIRequest.h"
+#include "nsThreadUtils.h"
 
 class nsIReferrerInfo;
 class nsIURI;
@@ -176,15 +177,29 @@ class ThreadSafeRequestHandle final {
   ThreadSafeRequestHandle(JS::loader::ScriptLoadRequest* aRequest,
                           nsISerialEventTarget* aSyncTarget);
 
-  JS::loader::ScriptLoadRequest* GetRequest() const { return mRequest; }
+  JS::loader::ScriptLoadRequest* GetRequest() const {
+    AssertRequestNotHandedOff();
+    return mRequest;
+  }
 
   WorkerLoadContext* GetContext();
 
-  bool IsEmpty() { return !mRequest; }
+  bool IsEmpty() const {
+    AssertRequestNotHandedOff();
+    return !mRequest;
+  }
 
   // Sets the owning runnable. Called on the main thread before loading begins.
   void SetRunnable(workerinternals::loader::ScriptLoaderRunnable* aRunnable);
 
+  // Whether this handle has been handed off to a ScriptExecutorRunnable, after
+  // which the request belongs to the worker thread. Main thread only; this is
+  // the only race-free way for main-thread code to test the handoff, see the
+  // comment on mRequest.
+  bool ExecutionScheduled() const;
+
+  void SetExecutionScheduled();
+
   // Runnable controls
   nsresult OnStreamComplete(nsresult aStatus);
 
@@ -204,11 +219,15 @@ class ThreadSafeRequestHandle final {
 
   already_AddRefed<workerinternals::loader::CacheCreator> GetCacheCreator();
 
-  bool mExecutionScheduled = false;
-
  private:
   ~ThreadSafeRequestHandle();
 
+  // Main-thread code must stop looking at the request, its WorkerLoadContext
+  // and anything else it owns once the handle has been handed off.
+  void AssertRequestNotHandedOff() const {
+    MOZ_ASSERT_IF(NS_IsMainThread(), !mExecutionScheduled);
+  }
+
   // Protects mRunnable, which is read on the main thread by the accessors above
   // but cleared on the worker thread by ReleaseRequest(). Without this lock the
   // unsynchronized read/write races and the ScriptLoaderRunnable can be freed
@@ -217,6 +236,17 @@ class ThreadSafeRequestHandle final {
   RefPtr<workerinternals::loader::ScriptLoaderRunnable> mRunnable
       MOZ_GUARDED_BY(mMutex);
 
+  // Only ever set on the main thread, and set before the handle is handed to
+  // the ScriptExecutorRunnable that releases the request, which is what makes
+  // it safe to read on the main thread without synchronization.
+  bool mExecutionScheduled = false;
+
+  // The request is released on the worker thread by ReleaseRequest(), which
+  // only runs once the handle has been handed off to a ScriptExecutorRunnable,
+  // and the worker may drop the last reference to it immediately afterwards.
+  // Main-thread code therefore cannot test mRequest to decide whether the
+  // request is still around: that read races with the worker thread and can
+  // hand back an already freed request. Gate on ExecutionScheduled() instead.
   RefPtr<JS::loader::ScriptLoadRequest> mRequest;
   nsCOMPtr<nsISerialEventTarget> mOwningEventTarget;
 };
Loading diff…