CVE-2025-1016

High CVSS 9.8 firefox Memory Corruption 🔧 Commit mapped

Overview

High

Severity

9.8

CVSS

Exploited ITW

Fixed

Fix Status

Impacthigh

DescriptionMemory safety bugs present in Firefox 134, Thunderbird 134, Firefox ESR 115.19, Firefox ESR 128.6, Thunderbird 115.19, and Thunderbird 128.6. 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.

ComponentGraphics

Bug ClassMemory Corruption

Tracker1936601

Fix commit761554b9ed77 (firefox) +13/-19

CWECWE-787 (Out-of-bounds write)

CVSS vectorCVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H

CISA KEVNot listed

CreditedAndrew McCreight, Randell Jesup, Andrew Osmond, Akmat Suleimanov and the Mozilla Fuzzing Team

Disclosed2025-02-04

Background

`SourceSurfaceSharedDataWrapper`: the GPU/compositor-process wrapper around a shared-memory-backed image surface that other processes hand off for rendering.
`mConsumers`: a reference count on SourceSurfaceSharedDataWrapper recording how many external-image references currently hold the surface alive.
Expiration tracking: SharedSurfacesParent keeps unmapped-but-live surfaces in an nsExpirationTracker (via AddTracking/RemoveTracking) so idle surfaces can be aged out and freed.
`mMapCount`: a per-wrapper count of active Map calls that keeps the shared memory mapped while any consumer is reading it.

Root Cause Analysis

Both SharedSurfacesParent::Add and SharedSurfacesParent::AddSameProcess inserted a surface into mSurfaces and then called surface->AddConsumer(), but the surface object was constructed with mConsumers initialized to 0, creating a window in which a wrapper could legitimately observe a consumer count of 0. The mapping paths did not respect that count: SourceSurfaceSharedDataWrapper::Map unconditionally called SharedSurfacesParent::RemoveTracking(this) and SourceSurfaceSharedDataWrapper::Unmap unconditionally called SharedSurfacesParent::AddTracking(this) when mMapCount reached 0, so a surface with zero consumers could be inserted into or removed from the expiration tracker. Because the expiration tracker frees surfaces it owns, mutating tracking membership for a surface that no consumer is holding lets the expiration machinery act on an object whose lifetime is no longer guaranteed, producing a use-after-free / memory-corruption condition.

The fix makes the invariant that a tracked surface always has at least one consumer explicit: mConsumers now defaults to 1 (so the wrapper begins life owned), the redundant AddConsumer() calls in Add/AddSameProcess are removed, and Map/Unmap gate the RemoveTracking/AddTracking calls on mConsumers > 0. Assertions in AddTrackingLocked and ExpireMap enforce GetConsumers() > 0 so any future violation is caught.

Key insight

The core mistake was decoupling expiration-tracker membership from the consumer reference count, allowing a surface with zero consumers to be (un)tracked and then freed out from under a live reference; the fix re-couples them by initializing mConsumers to 1 and only touching tracking while mConsumers > 0.

Attack Path

Register a shared surface A content/compositor client causes SharedSurfacesParent::Add to wrap externally supplied shared memory into a SourceSurfaceSharedDataWrapper and register it as a WebRender external image.
Drive the consumer count to zero By racing or repeating add/remove and map operations, the attacker reaches a state where mConsumers is 0 while the surface is still reachable through mSurfaces.
Toggle expiration tracking Calls into SourceSurfaceSharedDataWrapper::Map and Unmap invoke RemoveTracking/AddTracking on the zero-consumer surface, inserting it into the nsExpirationTracker without a live owner.
Trigger the free The expiration generation ages out and frees the tracked surface via AgeAndExpireOneGeneration/ExpireMap even though a reference is still in use.
Reuse the dangling surface A subsequent Map or render access dereferences the freed SourceSurfaceSharedDataWrapper/backing buffer, giving a use-after-free that can be groomed for arbitrary code execution.

Impact Assessment

An attacker gains a use-after-free on a shared-memory surface object in the GPU/compositor process, which the advisory rates as high-impact memory corruption potentially leading to arbitrary code execution. Reaching it requires the ability to register and manipulate shared surfaces through the graphics IPC layer and to drive the consumer count and expiration tracking into an inconsistent state, typically via crafted or timed compositor traffic.

Changed Functions

Function	Change	Notes
`if` gfx/layers/SourceSurfaceSharedData.cpp	modified
`mCreatorRef` gfx/layers/SourceSurfaceSharedData.h	modified
`for` gfx/layers/ipc/SharedSurfacesParent.cpp	modified

Files Changed

gfx/layers/SourceSurfaceSharedData.cpp
gfx/layers/SourceSurfaceSharedData.h
gfx/layers/ipc/SharedSurfacesParent.cpp

Audit Directions

Refcount/lifetime coupling
Audit every place a lifetime-tracking structure (expiration trackers, caches, LRU lists) is mutated to confirm membership is gated on the object’s own reference count rather than on unrelated state like mMapCount.
Deferred initialization windows
Look for objects whose reference counts are set to 0 at construction and incremented later, since any access between insertion and the increment can observe an unowned object; prefer initializing owned objects to 1.
Assertion-only invariants
Search for MOZ_ASSERT-guarded invariants (like the old GetConsumers assert) that vanish in release builds, and verify the invariant is also enforced by real runtime guards on the code paths that free memory.

Patch

view commit

diff --git a/gfx/layers/SourceSurfaceSharedData.cpp b/gfx/layers/SourceSurfaceSharedData.cpp
index b8d1456fba0..793b6bfa9c7 100644
--- a/gfx/layers/SourceSurfaceSharedData.cpp
+++ b/gfx/layers/SourceSurfaceSharedData.cpp
@@ -105,7 +105,9 @@ bool SourceSurfaceSharedDataWrapper::Map(MapType aMapType,
     MutexAutoLock lock(*mHandleLock);
     dataPtr = GetData();
     if (mMapCount == 0) {
-      SharedSurfacesParent::RemoveTracking(this);
+      if (mConsumers > 0) {
+        SharedSurfacesParent::RemoveTracking(this);
+      }
       if (!dataPtr) {
         size_t len = GetAlignedDataLength();
         if (!EnsureMapped(len)) {
@@ -129,7 +131,7 @@ bool SourceSurfaceSharedDataWrapper::Map(MapType aMapType,
 void SourceSurfaceSharedDataWrapper::Unmap() {
   if (mHandleLock) {
     MutexAutoLock lock(*mHandleLock);
-    if (--mMapCount == 0) {
+    if (--mMapCount == 0 && mConsumers > 0) {
       SharedSurfacesParent::AddTracking(this);
     }
   } else {
diff --git a/gfx/layers/SourceSurfaceSharedData.h b/gfx/layers/SourceSurfaceSharedData.h
index 1f92dbabd72..b17d6c4fed6 100644
--- a/gfx/layers/SourceSurfaceSharedData.h
+++ b/gfx/layers/SourceSurfaceSharedData.h
@@ -40,12 +40,7 @@ class SourceSurfaceSharedDataWrapper final : public DataSourceSurface {
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(SourceSurfaceSharedDataWrapper,
                                           override)
 
-  SourceSurfaceSharedDataWrapper()
-      : mStride(0),
-        mConsumers(0),
-        mFormat(SurfaceFormat::UNKNOWN),
-        mCreatorPid(0),
-        mCreatorRef(true) {}
+  SourceSurfaceSharedDataWrapper() = default;
 
   void Init(const IntSize& aSize, int32_t aStride, SurfaceFormat aFormat,
             SharedMemory::Handle aHandle, base::ProcessId aCreatorPid);
@@ -86,10 +81,7 @@ class SourceSurfaceSharedDataWrapper final : public DataSourceSurface {
     return --mConsumers == 0;
   }
 
-  uint32_t GetConsumers() const {
-    MOZ_ASSERT(mConsumers > 0);
-    return mConsumers;
-  }
+  uint32_t GetConsumers() const { return mConsumers; }
 
   bool HasCreatorRef() const { return mCreatorRef; }
 
@@ -113,13 +105,13 @@ class SourceSurfaceSharedDataWrapper final : public DataSourceSurface {
   // Protects mapping and unmapping of mBuf.
   Maybe<Mutex> mHandleLock;
   nsExpirationState mExpirationState;
-  int32_t mStride;
-  uint32_t mConsumers;
+  int32_t mStride = 0;
+  uint32_t mConsumers = 1;
   IntSize mSize;
   RefPtr<SharedMemory> mBuf;
-  SurfaceFormat mFormat;
-  base::ProcessId mCreatorPid;
-  bool mCreatorRef;
+  SurfaceFormat mFormat = SurfaceFormat::UNKNOWN;
+  base::ProcessId mCreatorPid = 0;
+  bool mCreatorRef = true;
 };
 
 /**
diff --git a/gfx/layers/ipc/SharedSurfacesParent.cpp b/gfx/layers/ipc/SharedSurfacesParent.cpp
index b2c3946d065..0a6d851f4a8 100644
--- a/gfx/layers/ipc/SharedSurfacesParent.cpp
+++ b/gfx/layers/ipc/SharedSurfacesParent.cpp
@@ -200,7 +200,6 @@ void SharedSurfacesParent::AddSameProcess(const wr::ExternalImageId& aId,
   auto texture = MakeRefPtr<wr::RenderSharedSurfaceTextureHost>(surface);
   wr::RenderThread::Get()->RegisterExternalImage(aId, texture.forget());
 
-  surface->AddConsumer();
   sInstance->mSurfaces.InsertOrUpdate(id, std::move(surface));
 }
 
@@ -269,7 +268,6 @@ void SharedSurfacesParent::Add(const wr::ExternalImageId& aId,
   auto texture = MakeRefPtr<wr::RenderSharedSurfaceTextureHost>(surface);
   wr::RenderThread::Get()->RegisterExternalImage(aId, texture.forget());
 
-  surface->AddConsumer();
   sInstance->mSurfaces.InsertOrUpdate(id, std::move(surface));
 }
 
@@ -284,6 +282,7 @@ void SharedSurfacesParent::AddTrackingLocked(
     SourceSurfaceSharedDataWrapper* aSurface,
     const StaticMutexAutoLock& aAutoLock) {
   MOZ_ASSERT(!aSurface->GetExpirationState()->IsTracked());
+  MOZ_ASSERT(aSurface->GetConsumers() > 0);
   sInstance->mTracker.AddObjectLocked(aSurface, aAutoLock);
 }
 
@@ -356,6 +355,7 @@ bool SharedSurfacesParent::AgeAndExpireOneGeneration() {
 void SharedSurfacesParent::ExpireMap(
     nsTArray<RefPtr<SourceSurfaceSharedDataWrapper>>& aExpired) {
   for (auto& surface : aExpired) {
+    MOZ_ASSERT(surface->GetConsumers() > 0);
     surface->ExpireMap();
   }
 }

Loading diff…

References

Vendor advisory Fix commit Bug tracker NVD

On This Page