PatchGap
Firefox · DOM

CVE-2025-1943

Race in DOM
High CVSS 8.2 firefox Race 🔧 Commit mapped

Overview

High
Severity
8.2
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionMemory safety bugs present in Firefox 135 and Thunderbird 135. Some of these bugs showed evidence of memory corruption and we presume that with enough effort some of these could have been exploited to run arbitrary code.
ComponentDOM
Bug ClassRace
Tracker1869650
Fix commit3fedfa61ee4e (firefox) +59/-40
CWECWE-122 (Heap buffer overflow)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:L/A:H
CISA KEVNot listed
Creditedthe Mozilla Fuzzing Team, Andrew McCreight, Sebastian Hengst, Randell Jesup
Disclosed2025-03-04

Background

`IsResolved()`
Prior to the patch, a public MozPromise method returning mValue.IsResolve(). Callers used it to decide whether to resolve/short-circuit. The patch removes it and marks IsPending()/Value() protected, documenting that inspecting a promise’s state from outside its code is not thread-safe.
`FetchParent` / `FetchEventOpProxyChild`
IPC actors coordinating worker and service-worker fetch, including keepalive and navigation-preload responses. Resolving the “parent” promise tears down actors (mActorDying).

Root Cause Analysis

Across the Fetch/ServiceWorker stack, code inspected promise state synchronously with MozPromise::IsResolved() from threads other than the one that owns and resolves the promise. Examples in the diff: FetchParent::RecvFetchOp tested GetResponseEndPromise()->IsResolved(); FetchService::FetchInstance::Cancel asserted and branched on mFetchParentPromise->IsResolved(); OnResponseEnd guarded ResolveResponseAvailablePromise/ResolveResponseTimingPromise with IsResolved() checks; OnReportPerformanceTiming and FetchService::Fetch likewise. IsResolved() reads mValue without the promise’s lifecycle guarantees, so between the check and the dependent action the promise could be resolved on another thread. The dangerous consequence is in the actor-teardown path: Cancel used the “not yet resolved” check to decide to set mActorDying and resolve mFetchParentPromise, whose resolution “leads to deleting of actors.” A racy/stale IsResolved() answer could drive a second resolution or an access to FetchParent after the actor was already being torn down — a use-after-free of IPC actor state. The MozPromise header comment added by the patch states plainly that IsPending()/Value() are unsafe outside the promise code because of its “somewhat complex locking and thread safety properties.”

The fix removes MozPromise::IsResolved() entirely and instead tracks resolution with explicit, caller-owned booleans set at the moment the owner calls Resolve: FetchServicePromises gains mAvailablePromiseResolved/mTimingPromiseResolved/mEndPromiseResolved (set inside ResolveResponse*Promise) exposed via IsResponse*PromiseResolved(); FetchEventOpProxyChild gains mPreloadResponseAvailablePromiseResolved/mPreloadResponseEndPromiseResolved set at each resolve site (Initialize, RecvPreloadResponse, RecvPreloadResponseEnd, ActorDestroy). FetchInstance::Cancel drops the IsResolved() assert/guard and unconditionally sets mActorDying then resolves (double-resolve is now a no-op by MozPromise contract), and OnResponseEnd resolves the available/timing promises unconditionally with comments noting a second resolve “has no effect.” IdentityCredential and nsUserCharacteristics timer callbacks likewise drop pre-resolve IsResolved() checks. The advisory’s CWE-122 “heap buffer overflow” label is not substantiated by the diff; the code shows a thread-safety/lifetime defect — a data race on promise state that could cause use-after-free of actor objects — so the heap-overflow framing is noted as a mismatch.

Key insight
Fetch code queried a MozPromise’s resolved state synchronously and cross-thread via IsResolved(), a check with no locking guarantees, then acted on the stale answer — most dangerously to gate an actor-destroying resolution, opening a race that could double-resolve or touch an actor during teardown (use-after-free).

Attack Path

  1. Script Web content / a service worker initiates a fetch (including keepalive or navigation-preload paths), creating the FetchServicePromises and associated IPC actors.
  2. Concurrent completion + cancel The fetch completes on its owning thread while the content cancels/aborts (or navigates away), so Cancel/OnResponseEnd run concurrently with the resolution of the response promises (inferred timing; the diff proves the cross-thread state read).
  3. Stale `IsResolved()` A teardown-path IsResolved() check reads promise state racily and returns a value that no longer matches reality.
  4. Bad resolution Acting on the stale answer, code resolves mFetchParentPromise (setting mActorDying) a second time or accesses FetchParent while the actor is being deleted.
  5. Primitive Use-after-free / invalid access on IPC actor state driven from the parent-process fetch service; the advisory presumes exploitability for code execution (inferred, not shown by the diff).

Impact Assessment

The affected code runs in the parent process’s FetchService and in content/worker IPC actors; it is reachable by web content and service workers that issue fetches with no special privilege. The substantiated defect is a thread-safety/lifetime bug: cross-thread IsResolved() reads racing with resolution, which the actor-teardown path turns into a potential use-after-free. No out-of-bounds index computation is present, so the CWE-122 heap-overflow label is not supported; “memory corruption → RCE” is the advisory’s presumption and is marked (inferred). CVSS C:N/I:L/A:H is consistent with a corruption/availability bug rather than a direct disclosure.

Changed Functions

FunctionChangeNotes
if
dom/fetch/FetchParent.cpp
 modified
if
dom/fetch/FetchService.cpp
 modified
if
dom/serviceworkers/FetchEventOpProxyChild.cpp
 modified

Files Changed

  • dom/credentialmanagement/identity/IdentityCredential.cpp
  • dom/fetch/FetchParent.cpp
  • dom/fetch/FetchService.cpp
  • dom/fetch/FetchService.h
  • dom/serviceworkers/FetchEventOpProxyChild.cpp
  • dom/serviceworkers/FetchEventOpProxyChild.h
  • toolkit/components/contentanalysis/ContentAnalysis.cpp
  • toolkit/components/resistfingerprinting/nsUserCharacteristics.cpp
  • xpcom/threads/MozPromise.h

Audit Directions

  • Remaining `IsResolved()`/`IsPending()` callers
    With the method removed, grep for any residual external promise-state inspection in the tree; each is a candidate race and should move to a tracked flag or ->Then.
  • Actor-teardown gating
    Audit other IPC paths that gate actor deletion (mActorDying-style flags) on a promise-state check; ensure the resolve is idempotent and not conditioned on a racy read.
  • MozPromise idempotency assumptions
    Review code that assumed Resolve must be called at most once (asserts, guards); confirm reliance on MozPromise’s “second resolve is a no-op” contract is correct for each promise type and thread.

Patch

view commit
diff --git a/dom/credentialmanagement/identity/IdentityCredential.cpp b/dom/credentialmanagement/identity/IdentityCredential.cpp
index 323b020db68..d06c9078678 100644
--- a/dom/credentialmanagement/identity/IdentityCredential.cpp
+++ b/dom/credentialmanagement/identity/IdentityCredential.cpp
@@ -995,9 +995,7 @@ IdentityCredential::DiscoverFromExternalSourceInMainProcess(
     nsresult rv = NS_NewTimerWithCallback(
         getter_AddRefs(timeout),
         [=](auto) {
-          if (!result->IsResolved()) {
-            result->Reject(NS_ERROR_DOM_NETWORK_ERR, __func__);
-          }
+          result->Reject(NS_ERROR_DOM_NETWORK_ERR, __func__);
           IdentityCredential::CloseUserInterface(browsingContext);
         },
         StaticPrefs::
diff --git a/dom/fetch/FetchParent.cpp b/dom/fetch/FetchParent.cpp
index 8b82381abea..6f969246aab 100644
--- a/dom/fetch/FetchParent.cpp
+++ b/dom/fetch/FetchParent.cpp
@@ -199,8 +199,7 @@ IPCResult FetchParent::RecvFetchOp(FetchOpArgs&& aArgs) {
           })));
     }
 
-    bool isResolved =
-        self->mResponsePromises->GetResponseEndPromise()->IsResolved();
+    bool isResolved = self->mResponsePromises->IsResponseEndPromiseResolved();
     if (!isResolved && self->mIsWorkerFetch) {
       // track only unresolved promises for worker fetch requests
       // this is needed for clean-up of keepalive requests
diff --git a/dom/fetch/FetchService.cpp b/dom/fetch/FetchService.cpp
index b854d1e9882..b5e60bbd27f 100644
--- a/dom/fetch/FetchService.cpp
+++ b/dom/fetch/FetchService.cpp
@@ -68,6 +68,7 @@ FetchServicePromises::GetResponseEndPromise() {
 void FetchServicePromises::ResolveResponseAvailablePromise(
     FetchServiceResponse&& aResponse, StaticString aMethodName) {
   if (mAvailablePromise) {
+    mAvailablePromiseResolved = true;
     mAvailablePromise->Resolve(std::move(aResponse), aMethodName);
   }
 }
@@ -82,6 +83,7 @@ void FetchServicePromises::RejectResponseAvailablePromise(
 void FetchServicePromises::ResolveResponseTimingPromise(
     ResponseTiming&& aTiming, StaticString aMethodName) {
   if (mTimingPromise) {
+    mTimingPromiseResolved = true;
     mTimingPromise->Resolve(std::move(aTiming), aMethodName);
   }
 }
@@ -96,6 +98,7 @@ void FetchServicePromises::RejectResponseTimingPromise(
 void FetchServicePromises::ResolveResponseEndPromise(ResponseEndArgs&& aArgs,
                                                      StaticString aMethodName) {
   if (mEndPromise) {
+    mEndPromiseResolved = true;
     mEndPromise->Resolve(std::move(aArgs), aMethodName);
   }
 }
@@ -345,15 +348,11 @@ void FetchService::FetchInstance::Cancel(bool aForceAbort) {
 
           mArgs.as<WorkerFetchArgs>().mResponseEndPromiseHolder.Disconnect();
 
-          MOZ_ASSERT(
-              !mArgs.as<WorkerFetchArgs>().mFetchParentPromise->IsResolved());
-          if (!mArgs.as<WorkerFetchArgs>().mFetchParentPromise->IsResolved()) {
-            // the parent promise resolution leads to deleting of actors
-            // mActorDying prevents further access to FetchParent
-            mActorDying = true;
-            mArgs.as<WorkerFetchArgs>().mFetchParentPromise->Resolve(true,
-                                                                     __func__);
-          }
+          // the parent promise resolution leads to deleting of actors
+          // mActorDying prevents further access to FetchParent
+          mActorDying = true;
+          mArgs.as<WorkerFetchArgs>().mFetchParentPromise->Resolve(true,
+                                                                   __func__);
         }
       }
       return;
@@ -412,24 +411,21 @@ void FetchService::FetchInstance::OnResponseEnd(
 
   if (aReason == eAborted) {
     // If ResponseAvailablePromise has not resolved yet, resolved with
-    // NS_ERROR_DOM_ABORT_ERR response.
-    if (!mPromises->GetResponseAvailablePromise()->IsResolved()) {
-      mPromises->ResolveResponseAvailablePromise(
-          InternalResponse::NetworkError(NS_ERROR_DOM_ABORT_ERR), __func__);
-    }
+    // NS_ERROR_DOM_ABORT_ERR response. If the promise is already resolved,
+    // this will have no effect.
+    mPromises->ResolveResponseAvailablePromise(
+        InternalResponse::NetworkError(NS_ERROR_DOM_ABORT_ERR), __func__);
 
     // If ResponseTimingPromise has not resolved yet, resolved with empty
-    // ResponseTiming.
-    if (!mPromises->GetResponseTimingPromise()->IsResolved()) {
-      mPromises->ResolveResponseTimingPromise(ResponseTiming(), __func__);
-    }
+    // ResponseTiming. If the promise is already resolved, this has no effect.
+    mPromises->ResolveResponseTimingPromise(ResponseTiming(), __func__);
     // Resolve the ResponseEndPromise
     mPromises->ResolveResponseEndPromise(ResponseEndArgs(aReason), __func__);
     return;
   }
 
-  MOZ_ASSERT(mPromises->GetResponseAvailablePromise()->IsResolved() &&
-             mPromises->GetResponseTimingPromise()->IsResolved());
+  MOZ_ASSERT(mPromises->IsResponseAvailablePromiseResolved() &&
+             mPromises->IsResponseTimingPromiseResolved());
 
   // Resolve the ResponseEndPromise
   mPromises->ResolveResponseEndPromise(ResponseEndArgs(aReason), __func__);
@@ -542,7 +538,7 @@ void FetchService::FetchInstance::OnReportPerformanceTiming() {
   MOZ_ASSERT(mFetchDriver);
   MOZ_ASSERT(mPromises);
 
-  if (mPromises->GetResponseTimingPromise()->IsResolved()) {
+  if (mPromises->IsResponseTimingPromiseResolved()) {
     return;
   }
 
@@ -872,7 +868,7 @@ RefPtr<FetchServicePromises> FetchService::Fetch(FetchArgs&& aArgs) {
   RefPtr<FetchServicePromises> promises = fetch->Fetch();
   MOZ_ASSERT(promises);
 
-  if (!promises->GetResponseAvailablePromise()->IsResolved()) {
+  if (!promises->IsResponseAvailablePromiseResolved()) {
     // Insert the created FetchInstance into FetchInstanceTable.
     if (!mFetchInstanceTable.WithEntryHandle(promises, [&](auto&& entry) {
           if (entry.HasEntry()) {
diff --git a/dom/fetch/FetchService.h b/dom/fetch/FetchService.h
index 5af939bea49..e98aaa4d278 100644
--- a/dom/fetch/FetchService.h
+++ b/dom/fetch/FetchService.h
@@ -47,6 +47,12 @@ class FetchServicePromises final {
   RefPtr<FetchServiceResponseTimingPromise> GetResponseTimingPromise();
   RefPtr<FetchServiceResponseEndPromise> GetResponseEndPromise();
 
+  bool IsResponseAvailablePromiseResolved() {
+    return mAvailablePromiseResolved;
+  }
+  bool IsResponseTimingPromiseResolved() { return mTimingPromiseResolved; }
+  bool IsResponseEndPromiseResolved() { return mEndPromiseResolved; }
+
   void ResolveResponseAvailablePromise(FetchServiceResponse&& aResponse,
                                        StaticString aMethodName);
   void RejectResponseAvailablePromise(const CopyableErrorResult&& aError,
@@ -66,6 +72,13 @@ class FetchServicePromises final {
   RefPtr<FetchServiceResponseAvailablePromise::Private> mAvailablePromise;
   RefPtr<FetchServiceResponseTimingPromise::Private> mTimingPromise;
   RefPtr<FetchServiceResponseEndPromise::Private> mEndPromise;
+
+  // The MozPromise interface intentionally does not expose synchronous access
+  // to the internal resolved/rejected state. Instead, we track whether or not
+  // we've called Resolve on the FetchServicePromises.
+  bool mAvailablePromiseResolved = false;
+  bool mTimingPromiseResolved = false;
+  bool mEndPromiseResolved = false;
 };
 
 /**
diff --git a/dom/serviceworkers/FetchEventOpProxyChild.cpp b/dom/serviceworkers/FetchEventOpProxyChild.cpp
index 8e437356ec5..3fd531373c0 100644
--- a/dom/serviceworkers/FetchEventOpProxyChild.cpp
+++ b/dom/serviceworkers/FetchEventOpProxyChild.cpp
@@ -75,6 +75,7 @@ void FetchEventOpProxyChild::Initialize(
             __func__);
     mPreloadResponseAvailablePromise->UseSynchronousTaskDispatch(__func__);
     if (aArgs.preloadResponse().isSome()) {
+      mPreloadResponseAvailablePromiseResolved = true;
       mPreloadResponseAvailablePromise->Resolve(
           InternalResponse::FromIPC(aArgs.preloadResponse().ref()), __func__);
     }
@@ -91,6 +92,7 @@ void FetchEventOpProxyChild::Initialize(
         MakeRefPtr<FetchEventPreloadResponseEndPromise::Private>(__func__);
     mPreloadResponseEndPromise->UseSynchronousTaskDispatch(__func__);
     if (aArgs.preloadResponseEndArgs().isSome()) {
+      mPreloadResponseEndPromiseResolved = true;
       mPreloadResponseEndPromise->Resolve(aArgs.preloadResponseEndArgs().ref(),
                                           __func__);
     }
@@ -107,8 +109,8 @@ void FetchEventOpProxyChild::Initialize(
     // result until RecvPreloadResponseEnd is called, such that the preload
     // response could be completed.
     if (self->mPreloadResponseEndPromise &&
-        !self->mPreloadResponseEndPromise->IsResolved() &&
-        self->mPreloadResponseAvailablePromise->IsResolved()) {
+        !self->mPreloadResponseEndPromiseResolved &&
+        self->mPreloadResponseAvailablePromiseResolved) {
       self->mCachedOpResult = Some(aResult);
       return;
     }
@@ -203,6 +205,7 @@ mozilla::ipc::IPCResult FetchEventOpProxyChild::RecvPreloadResponse(
   // Initialize() should have created this promise.
   MOZ_ASSERT(mPreloadResponseAvailablePromise);
 
+  mPreloadResponseAvailablePromiseResolved = true;
   mPreloadResponseAvailablePromise->Resolve(
       InternalResponse::FromIPC(aResponse), __func__);
 
@@ -225,6 +228,7 @@ mozilla::ipc::IPCResult FetchEventOpProxyChild::RecvPreloadResponseEnd(
   // Initialize() should have created this promise.
   MOZ_ASSERT(mPreloadResponseEndPromise);
 
+  mPreloadResponseEndPromiseResolved = true;
   mPreloadResponseEndPromise->Resolve(std::move(aArgs), __func__);
   // If mCachedOpResult is not nothing, it means FetchEventOp had already done
   // and the operation result is cached. Continue closing IPC here.
@@ -259,6 +263,7 @@ void FetchEventOpProxyChild::ActorDestroy(ActorDestroyReason) {
   // FetchEvent. Resolve the preload response promise with
   // NS_ERROR_DOM_ABORT_ERR.
   if (mPreloadResponseAvailablePromise) {
+    mPreloadResponseAvailablePromiseResolved = true;
     mPreloadResponseAvailablePromise->Resolve(
         InternalResponse::NetworkError(NS_ERROR_DOM_ABORT_ERR), __func__);
   }
@@ -268,6 +273,7 @@ void FetchEventOpProxyChild::ActorDestroy(ActorDestroyReason) {
   }
 
   if (mPreloadResponseEndPromise) {
+    mPreloadResponseEndPromiseResolved = true;
     ResponseEndArgs args(FetchDriverObserver::eAborted);
     mPreloadResponseEndPromise->Resolve(args, __func__);
   }
diff --git a/dom/serviceworkers/FetchEventOpProxyChild.h b/dom/serviceworkers/FetchEventOpProxyChild.h
index e5bd205602f..1908463b68e 100644
--- a/dom/serviceworkers/FetchEventOpProxyChild.h
+++ b/dom/serviceworkers/FetchEventOpProxyChild.h
@@ -70,6 +70,12 @@ class FetchEventOpProxyChild final : public PFetchEventOpProxyChild {
   RefPtr<FetchEventPreloadResponseEndPromise::Private>
       mPreloadResponseEndPromise;
 
+  // MozPromise intentionally does not expose synchronous access to the
+  // resolved/rejected state of a promise, so track whether or not we've
+  // resolved these promises manually.
+  bool mPreloadResponseAvailablePromiseResolved = false;
+  bool mPreloadResponseEndPromiseResolved = false;
+
   Maybe<ServiceWorkerOpResult> mCachedOpResult;
 };
 
diff --git a/toolkit/components/contentanalysis/ContentAnalysis.cpp b/toolkit/components/contentanalysis/ContentAnalysis.cpp
index f6d3c74a6ce..2cfea564d2c 100644
--- a/toolkit/components/contentanalysis/ContentAnalysis.cpp
+++ b/toolkit/components/contentanalysis/ContentAnalysis.cpp
@@ -334,8 +334,6 @@ nsresult ContentAnalysis::CreateContentAnalysisClient(
     nsCString&& aPipePathName, nsString&& aClientSignatureSetting,
     bool aIsPerUser) {
   MOZ_ASSERT(!NS_IsMainThread());
-  // This method should only be called once
-  MOZ_ASSERT(!mCaClientPromise->IsResolved());
 
   std::shared_ptr<content_analysis::sdk::Client> client(
       content_analysis::sdk::Client::Create({aPipePathName.Data(), aIsPerUser})
diff --git a/toolkit/components/resistfingerprinting/nsUserCharacteristics.cpp b/toolkit/components/resistfingerprinting/nsUserCharacteristics.cpp
index 18c6652e6d3..253171bed4d 100644
--- a/toolkit/components/resistfingerprinting/nsUserCharacteristics.cpp
+++ b/toolkit/components/resistfingerprinting/nsUserCharacteristics.cpp
@@ -657,10 +657,8 @@ const RefPtr<PopulatePromise>& TimoutPromise(
   nsresult rv = NS_NewTimerWithCallback(
       getter_AddRefs(timeout),
       [=](auto) {
-        if (!promise->IsResolved()) {
-          promise->Reject(std::pair(funcName, "TIMEOUT"_ns.AsString()),
-                          __func__);
-        }
+        // NOTE: has no effect if `promise` has already been resolved.
+        promise->Reject(std::pair(funcName, "TIMEOUT"_ns.AsString()), __func__);
       },
       delay, nsITimer::TYPE_ONE_SHOT, "UserCharacteristicsPromiseTimeout");
   if (NS_FAILED(rv)) {
diff --git a/xpcom/threads/MozPromise.h b/xpcom/threads/MozPromise.h
index 7a54d9d8ead..573555d1209 100644
--- a/xpcom/threads/MozPromise.h
+++ b/xpcom/threads/MozPromise.h
@@ -1112,21 +1112,21 @@ class MozPromise : public MozPromiseBase {
 
     template <typename... Ts>
     auto Then(Ts&&... aArgs) -> decltype(std::declval<PromiseType>().Then(
-                                 std::forward<Ts>(aArgs)...)) {
+        std::forward<Ts>(aArgs)...)) {
       return static_cast<RefPtr<PromiseType>>(*this)->Then(
           std::forward<Ts>(aArgs)...);
     }
 
     template <typename... Ts>
     auto Map(Ts&&... aArgs) -> decltype(std::declval<PromiseType>().Map(
-                                std::forward<Ts>(aArgs)...)) {
+        std::forward<Ts>(aArgs)...)) {
       return static_cast<RefPtr<PromiseType>>(*this)->Map(
           std::forward<Ts>(aArgs)...);
     }
 
     template <typename... Ts>
     auto MapErr(Ts&&... aArgs) -> decltype(std::declval<PromiseType>().MapErr(
-                                   std::forward<Ts>(aArgs)...)) {
+        std::forward<Ts>(aArgs)...)) {
       return static_cast<RefPtr<PromiseType>>(*this)->MapErr(
           std::forward<Ts>(aArgs)...);
     }
@@ -1257,9 +1257,14 @@ class MozPromise : public MozPromiseBase {
     }
   }
 
-  bool IsResolved() const { return mValue.IsResolve(); }
-
  protected:
+  // NOTE: Methods like `IsPending()` and `Value()` are intentionally marked as
+  // `protected`, as they are not safe to call from outside of the MozPromise
+  // code.  The only way to inspect the state of a MozPromise is intentionally
+  // to call `->Then` on the promise, and receive an async callback.
+  //
+  // MozPromise has somewhat complex locking and thread safety properties which
+  // can't be extended outside of the code in this file.
   bool IsPending() const { return mValue.IsNothing(); }
 
   ResolveOrRejectValue& Value() {
Loading diff…
On This Page