Medium CVSS 8.8 firefox Memory Corruption 🔧 Commit mapped

Overview

Medium
Severity
8.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionMemory safety bugs present in Firefox 131, Firefox ESR 128.3, and Thunderbird 128.3. 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.
ComponentNetworking
Bug ClassMemory Corruption
Tracker1829029
Fix commit0e2d2fc47aed (firefox) +11/-8
CWECWE-787, CWE-120, CWE-125 (Out-of-bounds write, Buffer overflow, Out-of-bounds read)
CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H
CISA KEVNot listed
CreditedAndrew McCreight, the Mozilla Fuzzing Team
Disclosed2024-10-29

Background

`CacheFileIOManager`
The networking subsystem’s singleton that owns cache file state and whose members like mHandles, mSpecialHandles, and mMetadataWritesTimer are only safe to touch on the dedicated cache I/O thread.
`SizeOfHandlesRunnable`
A Runnable sync-dispatched to the cache I/O thread so that memory-reporting code can measure I/O-thread-owned objects from the correct thread.
`mMetadataWritesTimer`
An nsCOMPtr<nsITimer> member of CacheFileIOManager that schedules deferred metadata writes and, like other I/O-thread state, must not be accessed concurrently from another thread.
`SizeOfExcludingThisInternal`
The memory-reporter entry point that tallies the manager’s retained heap size and previously read some members directly on the calling (non-I/O) thread.

Root Cause Analysis

The memory-reporting path in CacheFileIOManager::SizeOfExcludingThisInternal correctly dispatched a SizeOfHandlesRunnable to the I/O thread to measure mHandles and mSpecialHandles, but it then queried mMetadataWritesTimer via do_QueryInterface and SizeOfIncludingThis directly on the memory-reporter’s own thread. Because mMetadataWritesTimer is I/O-thread-owned state, this violated the invariant that the timer is only read or mutated on the I/O thread, creating a data race: the timer could be concurrently reset, released, or reassigned by the I/O thread while the reporter was dereferencing it, leading to a use-after-free or torn read.

The fix removes the off-thread access from SizeOfExcludingThisInternal and instead passes mMetadataWritesTimer into the SizeOfHandlesRunnable, which measures it inside Run on the I/O thread where every other timer access already happens. This works because all reads of the timer now occur under the same single-threaded discipline, eliminating the concurrent access window.

Key insight
The single mistake was measuring an I/O-thread-owned member (mMetadataWritesTimer) from the memory-reporter thread instead of inside the already-dispatched I/O-thread Runnable; the fix relocates that measurement into SizeOfHandlesRunnable::Run so the timer is only ever touched on its owning thread.

Attack Path

  1. Trigger a memory report An attacker (or automated telemetry/about:memory path they can influence) causes CacheFileIOManager::SizeOfExcludingThisInternal to run on a non-I/O thread.
  2. Race the timer Concurrent cache activity on the I/O thread reschedules or releases mMetadataWritesTimer, reassigning or freeing the underlying nsITimer object.
  3. Off-thread dereference The reporter’s do_QueryInterface(mMetadataWritesTimer) and SizeOfIncludingThis call reads the timer pointer and dereferences it while it is being mutated or destroyed on the I/O thread.
  4. Corrupt memory access The torn read or use-after-free dereferences a stale or partially-updated pointer, producing memory corruption whose contents an attacker may shape through controlled cache load.

Impact Assessment

An attacker who can drive concurrent cache I/O while a memory report runs gains a data race yielding a use-after-free or torn read on the nsITimer object inside the parent-process networking subsystem. Successful exploitation could corrupt heap memory and, per Mozilla’s assessment, potentially be leveraged toward arbitrary code execution, though it requires precise timing between the memory-reporter thread and the cache I/O thread. The precondition is that the timer is accessed off the I/O thread while the I/O thread is free to mutate it.

Changed Functions

FunctionChangeNotes
mSize
netwerk/cache2/CacheFileIOManager.cpp
modified
if
netwerk/cache2/CacheFileIOManager.cpp
modified

Files Changed

  • netwerk/cache2/CacheFileIOManager.cpp

Audit Directions

  • Off-thread member reads in memory reporters
    Review other SizeOfExcludingThis/SizeOfIncludingThis implementations for members documented as single-thread-owned that are measured directly on the reporter thread instead of via a dispatched runnable.
  • Thread-affinity annotations
    Search for members guarded by MOZ_UNANNOTATED or comments like “must be accessed only on the I/O thread” and confirm every access site, including size and diagnostic code, honors that affinity.
  • Timer and nsCOMPtr lifetime races
    Audit nsITimer and other nsCOMPtr members that can be reset or released on one thread for any read, do_QueryInterface, or dereference from a different thread.
diff --git a/netwerk/cache2/CacheFileIOManager.cpp b/netwerk/cache2/CacheFileIOManager.cpp
index 85cd017d8b9..b4f986929e3 100644
--- a/netwerk/cache2/CacheFileIOManager.cpp
+++ b/netwerk/cache2/CacheFileIOManager.cpp
@@ -4410,13 +4410,15 @@ class SizeOfHandlesRunnable : public Runnable {
  public:
   SizeOfHandlesRunnable(mozilla::MallocSizeOf mallocSizeOf,
                         CacheFileHandles const& handles,
-                        nsTArray<CacheFileHandle*> const& specialHandles)
+                        nsTArray<CacheFileHandle*> const& specialHandles,
+                        nsCOMPtr<nsITimer> const& metadataWritesTimer)
       : Runnable("net::SizeOfHandlesRunnable"),
         mMonitor("SizeOfHandlesRunnable.mMonitor"),
         mMonitorNotified(false),
         mMallocSizeOf(mallocSizeOf),
         mHandles(handles),
         mSpecialHandles(specialHandles),
+        mMetadataWritesTimer(metadataWritesTimer),
         mSize(0) {}
 
   size_t Get(CacheIOThread* thread) {
@@ -4448,6 +4450,8 @@ class SizeOfHandlesRunnable : public Runnable {
     for (uint32_t i = 0; i < mSpecialHandles.Length(); ++i) {
       mSize += mSpecialHandles[i]->SizeOfIncludingThis(mMallocSizeOf);
     }
+    nsCOMPtr<nsISizeOf> sizeOf = do_QueryInterface(mMetadataWritesTimer);
+    if (sizeOf) mSize += sizeOf->SizeOfIncludingThis(mallocSizeOf);
 
     mMonitorNotified = true;
     mon.Notify();
@@ -4455,11 +4459,12 @@ class SizeOfHandlesRunnable : public Runnable {
   }
 
  private:
-  mozilla::Monitor mMonitor MOZ_UNANNOTATED;
+  mozilla::Monitor mMonitor;
   bool mMonitorNotified;
   mozilla::MallocSizeOf mMallocSizeOf;
   CacheFileHandles const& mHandles;
   nsTArray<CacheFileHandle*> const& mSpecialHandles;
+  nsCOMPtr<nsITimer> const& mMetadataWritesTimer;
   size_t mSize;
 };
 
@@ -4473,10 +4478,11 @@ size_t CacheFileIOManager::SizeOfExcludingThisInternal(
   if (mIOThread) {
     n += mIOThread->SizeOfIncludingThis(mallocSizeOf);
 
-    // mHandles and mSpecialHandles must be accessed only on the I/O thread,
-    // must sync dispatch.
+    // mHandles, mSpecialHandles and mMetadataWritesTimer must be accessed
+    // only on the I/O thread, must sync dispatch.
     RefPtr<SizeOfHandlesRunnable> sizeOfHandlesRunnable =
-        new SizeOfHandlesRunnable(mallocSizeOf, mHandles, mSpecialHandles);
+        new SizeOfHandlesRunnable(mallocSizeOf, mHandles, mSpecialHandles,
+                                  mMetadataWritesTimer);
     n += sizeOfHandlesRunnable->Get(mIOThread);
   }
 
@@ -4485,9 +4491,6 @@ size_t CacheFileIOManager::SizeOfExcludingThisInternal(
   sizeOf = do_QueryInterface(mCacheDirectory);
   if (sizeOf) n += sizeOf->SizeOfIncludingThis(mallocSizeOf);
 
-  sizeOf = do_QueryInterface(mMetadataWritesTimer);
-  if (sizeOf) n += sizeOf->SizeOfIncludingThis(mallocSizeOf);
-
   sizeOf = do_QueryInterface(mTrashTimer);
   if (sizeOf) n += sizeOf->SizeOfIncludingThis(mallocSizeOf);
 
Loading diff…