High firefox Memory Corruption 🔧 Commit mapped

Overview

High
Severity
CVSS
No
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
CISA KEVNot listed
CreditedAndrew McCreight, Randell Jesup, Andrew Osmond, Akmat Suleimanov and the Mozilla Fuzzing Team
Disclosed2025-02-04

Changed Functions

FunctionChangeNotes
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
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…