High CVSS 6.5 firefox Race 🔧 Commit mapped

Overview

High
Severity
6.5
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionMemory safety bugs present in Firefox 135, Thunderbird 135, Firefox ESR 128.7, and Thunderbird 128.7. 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
Tracker1922889
Fix commit787243d05493 (firefox) +24/-9
CWECWE-787 (Out-of-bounds write)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:L/A:N
CISA KEVNot listed
CreditedJulien Wajsberg, the Mozilla Fuzzing Team
Disclosed2025-03-04

Background

`mPendingSessionPromises` / `mPendingGenericPromises`
std::unordered_map<uint32_t, MozPromiseHolder<...>> tables keyed by a promise id, holding in-flight EME operations (CreateSession, LoadSession, UpdateSession, etc.).
`mManagerThread`
The thread on which IPC ->Then resolve/reject callbacks run; distinct from the main thread that issues the EME requests.
`MOZ_GUARDED_BY` / `Mutex`
Clang thread-safety annotation and lock. The patch adds mutable Mutex mMutex and annotates both maps MOZ_GUARDED_BY(mMutex).

Root Cause Analysis

The two std::unordered_map promise tables were mutated from two different threads with no lock. Request methods (CreateSessionAndGenerateRequest, LoadSession, UpdateSession, CloseSession, RemoveSession, SetServerCertificate, GetStatusForPolicy) run on the caller’s thread and emplace new (promiseId, holder) entries. The corresponding IPC completion lambdas — dispatched via ->Then(mManagerThread, ...) and the RESOLVE_OR_REJECT macro — run on the manager thread and find/erase entries. Shutdown additionally iterates both maps, rejects every holder, and clear()s them. Concurrent emplace/find/erase/clear on a single std::unordered_map is undefined behavior: the container’s bucket array and node links can be read while being rewritten, producing corrupted iterators, torn node pointers, or operations on freed nodes. The advisory labels this “memory safety bugs … evidence of memory corruption” and CWE-787 (out-of-bounds write); the diff substantiates it concretely as a data race on STL container internals, and the “OOB write” framing maps to the container’s internal pointer/bucket corruption rather than an explicit index miscalculation — note this discrepancy.

The patch adds mutable Mutex mMutex{"MFCDMChild"} and annotates mPendingSessionPromises/mPendingGenericPromises with MOZ_GUARDED_BY(mMutex), then takes MutexAutoLock lock(mMutex) around every access: each emplace in the request methods, each find/erase in the resolve/reject callbacks (including inside the RESOLVE_OR_REJECT macro), and the reject-and-clear block in Shutdown (wrapped in a { ... } scope so both maps are cleared under one lock). Serializing all map mutations removes the concurrent-access UB.

Key insight
Two std::unordered_map promise tables were inserted into on the request thread while the manager thread (and shutdown) were looking up, erasing, and clearing them — unsynchronized concurrent mutation of an STL container, which is undefined behavior and corrupts the container’s internal node/bucket structure.

Attack Path

  1. Script DRM-enabled content drives EME: it calls into MFCDMChild to create/load/update/close/remove media key sessions, each emplaceing a promise on the request thread.
  2. IPC completion The remote CDM process replies; ->Then fires resolve/reject lambdas on mManagerThread, which find/erase the same map entries.
  3. Concurrency The content arranges many overlapping EME operations (and/or a teardown triggering Shutdown) so that an insert on one thread overlaps a lookup/erase/clear on another (inferred scheduling; the diff proves the cross-thread access).
  4. Container corruption Simultaneous rehash/insert and erase leave dangling node pointers or a half-updated bucket array, so a subsequent map operation reads or writes freed/invalid memory.
  5. Primitive Heap memory corruption via the unordered_map internals; the advisory presumes this could be driven to code execution (inferred, not shown by the diff).

Impact Assessment

The code runs in the process hosting MFCDMChild (the content/renderer side of the EME IPC), reachable by web content that can exercise Encrypted Media Extensions — gated on EME/WMF being available (Windows). The demonstrated defect is a data race on std::unordered_map, yielding heap corruption of container-managed nodes; the step from there to controlled write / RCE is the advisory’s presumption and is marked (inferred). CVSS is C:L/I:L/A:N, consistent with corruption of limited scope rather than a proven high-integrity primitive.

Changed Functions

FunctionChangeNotes
for
dom/media/ipc/MFCDMChild.cpp
modified

Files Changed

  • dom/media/ipc/MFCDMChild.cpp
  • dom/media/ipc/MFCDMChild.h

Audit Directions

  • Sibling IPC actors
    Audit other media/EME IPC children (e.g. MFCDMParent, MFMediaEngineChild, RemoteDecoderChild) for std::unordered_map/std::map promise tables inserted on one thread and drained on a manager thread without a lock.
  • `MozPromiseHolder` collections
    Review any container of promise holders keyed by id; the safe pattern is a single guarding mutex covering insert, resolve, reject, and shutdown-clear.
  • `MOZ_GUARDED_BY` gaps
    Grep for cross-thread-accessed STL containers lacking thread-safety annotations; the annotation here would have caught the missing lock at compile time.
diff --git a/dom/media/ipc/MFCDMChild.cpp b/dom/media/ipc/MFCDMChild.cpp
index 4cbc7403970..feb61c82afa 100644
--- a/dom/media/ipc/MFCDMChild.cpp
+++ b/dom/media/ipc/MFCDMChild.cpp
@@ -24,6 +24,7 @@ namespace mozilla {
         self->mManagerThread, callsite,                                      \
         [self, promiseId, callsite](                                         \
             PMFCDMChild::method##Promise::ResolveOrRejectValue&& result) {   \
+          MutexAutoLock lock(self->mMutex);                                  \
           auto iter = self->mPendingGenericPromises.find(promiseId);         \
           if (iter == self->mPendingGenericPromises.end()) {                 \
             return;                                                          \
@@ -135,15 +136,18 @@ void MFCDMChild::Shutdown() {
           mRemoteRequest.DisconnectIfExists();
           mInitRequest.DisconnectIfExists();
 
-          for (auto& promise : mPendingSessionPromises) {
-            promise.second.RejectIfExists(NS_ERROR_ABORT, __func__);
-          }
-          mPendingSessionPromises.clear();
+          {
+            MutexAutoLock lock(mMutex);
+            for (auto& promise : mPendingSessionPromises) {
+              promise.second.RejectIfExists(NS_ERROR_ABORT, __func__);
+            }
+            mPendingSessionPromises.clear();
 
-          for (auto& promise : mPendingGenericPromises) {
-            promise.second.RejectIfExists(NS_ERROR_ABORT, __func__);
+            for (auto& promise : mPendingGenericPromises) {
+              promise.second.RejectIfExists(NS_ERROR_ABORT, __func__);
+            }
+            mPendingGenericPromises.clear();
           }
-          mPendingGenericPromises.clear();
 
           mRemotePromiseHolder.RejectIfExists(NS_ERROR_ABORT, __func__);
           mCapabilitiesPromiseHolder.RejectIfExists(NS_ERROR_ABORT, __func__);
@@ -274,6 +278,7 @@ RefPtr<MFCDMChild::SessionPromise> MFCDMChild::CreateSessionAndGenerateRequest(
                                                        __func__);
   }
 
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mPendingSessionPromises.find(aPromiseId) ==
              mPendingSessionPromises.end());
   mPendingSessionPromises.emplace(aPromiseId,
@@ -287,6 +292,7 @@ RefPtr<MFCDMChild::SessionPromise> MFCDMChild::CreateSessionAndGenerateRequest(
         SendCreateSessionAndGenerateRequest(params)->Then(
             mManagerThread, __func__,
             [self, aPromiseId, this](const MFCDMSessionResult& result) {
+              MutexAutoLock lock(mMutex);
               auto iter = mPendingSessionPromises.find(aPromiseId);
               if (iter == mPendingSessionPromises.end()) {
                 return;
@@ -303,6 +309,7 @@ RefPtr<MFCDMChild::SessionPromise> MFCDMChild::CreateSessionAndGenerateRequest(
             },
             [self, aPromiseId,
              this](const mozilla::ipc::ResponseRejectReason& aReason) {
+              MutexAutoLock lock(mMutex);
               auto iter = mPendingSessionPromises.find(aPromiseId);
               if (iter == mPendingSessionPromises.end()) {
                 return;
@@ -325,6 +332,7 @@ RefPtr<GenericPromise> MFCDMChild::LoadSession(
     return GenericPromise::CreateAndReject(NS_ERROR_ABORT, __func__);
   }
 
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mPendingGenericPromises.find(aPromiseId) ==
              mPendingGenericPromises.end());
   mPendingGenericPromises.emplace(aPromiseId,
@@ -343,6 +351,7 @@ RefPtr<GenericPromise> MFCDMChild::UpdateSession(uint32_t aPromiseId,
     return GenericPromise::CreateAndReject(NS_ERROR_ABORT, __func__);
   }
 
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mPendingGenericPromises.find(aPromiseId) ==
              mPendingGenericPromises.end());
   mPendingGenericPromises.emplace(aPromiseId,
@@ -361,6 +370,7 @@ RefPtr<GenericPromise> MFCDMChild::CloseSession(uint32_t aPromiseId,
     return GenericPromise::CreateAndReject(NS_ERROR_ABORT, __func__);
   }
 
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mPendingGenericPromises.find(aPromiseId) ==
              mPendingGenericPromises.end());
   mPendingGenericPromises.emplace(aPromiseId,
@@ -378,6 +388,7 @@ RefPtr<GenericPromise> MFCDMChild::RemoveSession(uint32_t aPromiseId,
     return GenericPromise::CreateAndReject(NS_ERROR_ABORT, __func__);
   }
 
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mPendingGenericPromises.find(aPromiseId) ==
              mPendingGenericPromises.end());
   mPendingGenericPromises.emplace(aPromiseId,
@@ -395,6 +406,7 @@ RefPtr<GenericPromise> MFCDMChild::SetServerCertificate(
     return GenericPromise::CreateAndReject(NS_ERROR_ABORT, __func__);
   }
 
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mPendingGenericPromises.find(aPromiseId) ==
              mPendingGenericPromises.end());
   mPendingGenericPromises.emplace(aPromiseId,
@@ -412,6 +424,7 @@ RefPtr<GenericPromise> MFCDMChild::GetStatusForPolicy(
     return GenericPromise::CreateAndReject(NS_ERROR_ABORT, __func__);
   }
 
+  MutexAutoLock lock(mMutex);
   MOZ_ASSERT(mPendingGenericPromises.find(aPromiseId) ==
              mPendingGenericPromises.end());
   mPendingGenericPromises.emplace(aPromiseId,
diff --git a/dom/media/ipc/MFCDMChild.h b/dom/media/ipc/MFCDMChild.h
index 2f36b9df0d1..fa83dc35a26 100644
--- a/dom/media/ipc/MFCDMChild.h
+++ b/dom/media/ipc/MFCDMChild.h
@@ -141,11 +141,13 @@ class MFCDMChild final : public PMFCDMChild {
   MozPromiseHolder<GenericPromise> mRemoveSessionPromiseHolder;
   MozPromiseRequestHolder<RemoveSessionPromise> mRemoveSessionRequest;
 
+  mutable Mutex mMutex{"MFCDMChild"};
+
   std::unordered_map<uint32_t, MozPromiseHolder<SessionPromise>>
-      mPendingSessionPromises;
+      mPendingSessionPromises MOZ_GUARDED_BY(mMutex);
 
   std::unordered_map<uint32_t, MozPromiseHolder<GenericPromise>>
-      mPendingGenericPromises;
+      mPendingGenericPromises MOZ_GUARDED_BY(mMutex);
 
   RefPtr<WMFCDMProxyCallback> mProxyCallback;
 };
Loading diff…