High CVSS 9.8 firefox OOB 🔧 Commit mapped

Overview

High
Severity
9.8
CVSS
No
Exploited ITW
Fixed
Fix Status
Impacthigh
DescriptionCertain canvas operations could have lead to memory corruption.
ComponentDOM
Bug ClassOOB
Tracker1966083
Fix commit2d53f4e1dec2 (firefox) +156/-17
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
CreditedYannis Juglaret and Steven Michaud
Disclosed2025-06-10

Background

Remote/recorded canvas
Firefox records canvas drawing commands in the content process and replays them in a separate process via CanvasTranslator; surfaces are referenced across the IPC boundary by opaque IDs rather than pointers.

Root Cause Analysis

Two related defects caused the canvas memory corruption (CWE-787 out-of-bounds write). First, on the upload side, TexUnpackSurface::TexOrSubImage trusted that the SourceSurface resolved from a SurfaceDescriptorCanvasSurface was at least as large as the requested upload size. If the surface returned was smaller than size.x/size.y, the texture-upload code would read/write past the surface’s backing buffer — an out-of-bounds access driven by the mismatch between the requested dimensions and the actual surface size. The patch adds an explicit guard: if surf->GetSize().width < size.x or surf->GetSize().height < size.y, it allocates a correctly sized Skia DrawTarget, CopySurfaces the undersized surface into it, snapshots, and uses that adjusted data surface — guaranteeing the source is at least size before upload.

Second, surface identity across the recording IPC was keyed by the raw this pointer of SourceSurfaceCanvasRecording (uintptr_t(ReferencePtr(this))), and the translator used HasSourceSurface/LookupSourceSurface keyed on that pointer. A pointer value can be reused after a surface is freed, so a stale/ambiguous key could resolve to the wrong surface. The patch replaces this with an explicit, monotonically increasing export ID: GetSurfaceDescriptor lazily allocates mExportID = ReferencePtr(++sNextExportID) and records a RecordedAddExportSurface(mExportID, this) event; the descriptor now carries mExportID instead of this. The translator keeps a dedicated mExportSurfaces (nsRefPtrHashtable) populated by AddExportSurface/RemoveExportSurface and looked up via LookupExportSurface; WaitForSurface now resolves via LookupExportSurface(idRef) rather than HasSourceSurface/LookupSourceSurface. RecordedRemoveExportSurface is emitted on destruction (threaded through ReleaseOnMainThread) so the mapping is cleaned up deterministically. The now-unused InlineTranslator::HasSourceSurface is removed, and SourceSurface::GetSurfaceDescriptor loses its const qualifier so the ID can be assigned lazily. The advisory’s “canvas operations → memory corruption / out-of-bounds write” is accurate; the diff shows the specific sink is the undersized-surface texture upload, with the export-ID change hardening surface identity against stale-pointer confusion.

Key insight
The WebGL/canvas texture-upload path assumed the resolved canvas SourceSurface was at least the requested upload size and keyed cross-process surface identity by a reusable raw this pointer; an undersized surface (and/or a stale pointer resolving to the wrong surface) let the upload read/write out of bounds. The fix validates/resizes the surface to the requested dimensions and replaces pointer-keyed identity with explicit allocated export IDs.

Attack Path

  1. Attacker content script drives canvas/WebGL operations that produce a recorded canvas surface and request a texture upload (e.g. texImage2D from a canvas source).
  2. Size mismatch the operations arrange for the resolved SourceSurface to be smaller than the requested upload size (dimensions the attacker influences).
  3. Unchecked upload pre-fix, TexUnpackSurface::TexOrSubImage uploads size.x×size.y pixels from the undersized surface buffer.
  4. OOB access the upload reads/writes beyond the surface’s allocation; combined with stale-pointer surface resolution, it can also operate on an unintended surface.
  5. Primitive out-of-bounds write (and OOB read) in the canvas/GPU translation process; the advisory treats this as exploitable toward memory corruption (full RCE is inferred, not proven by the diff).

Impact Assessment

This is a high-severity out-of-bounds write (CVSS 9.8, C/I/A:H) reachable from unprivileged web content via canvas/WebGL, in the process that performs canvas recording translation/texture upload (GPU/compositor-side). The attacker gains a heap out-of-bounds write primitive bounded by the dimension mismatch they can induce; the undersized-surface resize and deterministic export-ID lifetime both close the corruption. Escalation from the OOB write to code execution is the advisory’s presumption and is marked inferred. The lifetime fix is an ID-indirection hardening rather than a RefPtr/rooting change, so stale-pointer reuse (not a classic UAF of a ref-counted object) is the identity hazard addressed.

Changed Functions

FunctionChangeNotes
if
dom/canvas/TexUnpackBlob.cpp
modified
RecordedCanvasBeginTransaction
gfx/layers/RecordedCanvasEventImpl.h
modified
RecordedAddExportSurface
gfx/layers/RecordedCanvasEventImpl.h
modified
mActualSurface
gfx/layers/RecordedCanvasEventImpl.h
modified
if
gfx/layers/RecordedCanvasEventImpl.h
modified
RecordedRemoveExportSurface
gfx/layers/RecordedCanvasEventImpl.h
modified
if
gfx/layers/ipc/CanvasChild.cpp
modified
CanvasDataShmemHolder
gfx/layers/ipc/CanvasChild.cpp
modified
if
gfx/layers/ipc/CanvasTranslator.cpp
modified

Files Changed

  • dom/canvas/TexUnpackBlob.cpp
  • gfx/2d/2D.h
  • gfx/2d/InlineTranslator.h
  • gfx/layers/RecordedCanvasEventImpl.h
  • gfx/layers/ipc/CanvasChild.cpp
  • gfx/layers/ipc/CanvasTranslator.cpp
  • gfx/layers/ipc/CanvasTranslator.h

Audit Directions

  • Other texture/surface upload sinks
    review WebGL/WebGPU and TexUnpackBlob/TexUnpackImage paths for other spots that upload size pixels from a surface without validating surf->GetSize() against the requested dimensions.
  • Pointer-keyed cross-process identity
    audit remaining uses of uintptr_t(ReferencePtr(this)) / raw-pointer surface IDs in the canvas-recording IPC for the same stale-pointer-reuse confusion now replaced by export IDs.
  • Export-surface lifetime
    verify RecordedRemoveExportSurface is emitted on every destruction path (main-thread and deferred-deletion) so mExportSurfaces cannot grow unbounded or resolve to freed data.
  • Surface descriptor consumers
    check all readers of SurfaceDescriptorCanvasSurface (besides WaitForSurface) to ensure they migrated to the export-ID table rather than the old source-surface lookup.
diff --git a/dom/canvas/TexUnpackBlob.cpp b/dom/canvas/TexUnpackBlob.cpp
index f0c8e11f8d6..815ca11c461 100644
--- a/dom/canvas/TexUnpackBlob.cpp
+++ b/dom/canvas/TexUnpackBlob.cpp
@@ -1040,6 +1040,30 @@ bool TexUnpackSurface::TexOrSubImage(bool isSubImage, bool needsRespec,
         gfxCriticalNote << "TexUnpackSurface failed to get CanvasSurface";
         return false;
       }
+      if (NS_WARN_IF(surf->GetSize().width < GLint(size.x)) ||
+          NS_WARN_IF(surf->GetSize().height < GLint(size.y))) {
+        RefPtr<gfx::DrawTarget> adjusted = gfx::Factory::CreateDrawTarget(
+            gfx::BackendType::SKIA, gfx::IntSize(size.x, size.y),
+            surf->GetFormat());
+        if (!adjusted) {
+          gfxCriticalNote
+              << "Failed to created adjusted target for CanvasSurface";
+          return false;
+        }
+        adjusted->CopySurface(surf, surf->GetRect(), gfx::IntPoint(0, 0));
+        if (RefPtr<gfx::SourceSurface> snapshot = adjusted->Snapshot()) {
+          surf = snapshot->GetDataSurface();
+          if (!surf) {
+            gfxCriticalNote
+                << "Failed to get adjusted snapshot data for CanvasSurface";
+            return false;
+          }
+        } else {
+          gfxCriticalNote
+              << "Failed to create adjusted snapshot for CanvasSurface";
+          return false;
+        }
+      }
     } else {
       MOZ_ASSERT_UNREACHABLE("Unexpected surface descriptor!");
     }
diff --git a/gfx/2d/2D.h b/gfx/2d/2D.h
index 11e44b5dad5..c4161836607 100644
--- a/gfx/2d/2D.h
+++ b/gfx/2d/2D.h
@@ -724,7 +724,7 @@ class SourceSurface : public SupportsThreadSafeWeakPtr<SourceSurface> {
   }
 
   /** Tries to generate a SurfaceDescriptor for the surface, if possible. */
-  virtual bool GetSurfaceDescriptor(layers::SurfaceDescriptor& aDesc) const {
+  virtual bool GetSurfaceDescriptor(layers::SurfaceDescriptor& aDesc) {
     return false;
   }
 
diff --git a/gfx/2d/InlineTranslator.h b/gfx/2d/InlineTranslator.h
index 3e52568b4cc..334edb8404a 100644
--- a/gfx/2d/InlineTranslator.h
+++ b/gfx/2d/InlineTranslator.h
@@ -54,10 +54,6 @@ class InlineTranslator : public Translator {
     return result;
   }
 
-  bool HasSourceSurface(ReferencePtr aRefPtr) const {
-    return mSourceSurfaces.GetWeak(aRefPtr) != nullptr;
-  }
-
   SourceSurface* LookupSourceSurface(ReferencePtr aRefPtr) final {
     SourceSurface* result = mSourceSurfaces.GetWeak(aRefPtr);
     MOZ_ASSERT(result);
diff --git a/gfx/layers/RecordedCanvasEventImpl.h b/gfx/layers/RecordedCanvasEventImpl.h
index 40d443b7769..ea3bef03a7f 100644
--- a/gfx/layers/RecordedCanvasEventImpl.h
+++ b/gfx/layers/RecordedCanvasEventImpl.h
@@ -49,7 +49,9 @@ const EventType PRESENT_TEXTURE = EventType(EventType::LAST + 18);
 const EventType DEVICE_RESET_ACKNOWLEDGED = EventType(EventType::LAST + 19);
 const EventType AWAIT_TRANSLATION_SYNC = EventType(EventType::LAST + 20);
 const EventType RESOLVE_EXTERNAL_SNAPSHOT = EventType(EventType::LAST + 21);
-const EventType LAST_CANVAS_EVENT_TYPE = RESOLVE_EXTERNAL_SNAPSHOT;
+const EventType ADD_EXPORT_SURFACE = EventType(EventType::LAST + 22);
+const EventType REMOVE_EXPORT_SURFACE = EventType(EventType::LAST + 23);
+const EventType LAST_CANVAS_EVENT_TYPE = REMOVE_EXPORT_SURFACE;
 
 class RecordedCanvasBeginTransaction final
     : public RecordedEventDerived<RecordedCanvasBeginTransaction> {
@@ -874,6 +876,92 @@ RecordedPresentTexture::RecordedPresentTexture(S& aStream)
   ReadElement(aStream, mLastRemoteTextureId.mId);
 }
 
+class RecordedAddExportSurface final
+    : public RecordedEventDerived<RecordedAddExportSurface> {
+ public:
+  RecordedAddExportSurface(ReferencePtr aExportID,
+                           const RefPtr<gfx::SourceSurface>& aActualSurface)
+      : RecordedEventDerived(ADD_EXPORT_SURFACE),
+        mExportID(aExportID),
+        mActualSurface(aActualSurface) {}
+
+  template <class S>
+  MOZ_IMPLICIT RecordedAddExportSurface(S& aStream);
+
+  bool PlayCanvasEvent(CanvasTranslator* aTranslator) const;
+
+  template <class S>
+  void Record(S& aStream) const;
+
+  std::string GetName() const final { return "RecordedAddExportSurface"; }
+
+ private:
+  ReferencePtr mExportID;
+  ReferencePtr mActualSurface;
+};
+
+inline bool RecordedAddExportSurface::PlayCanvasEvent(
+    CanvasTranslator* aTranslator) const {
+  RefPtr<gfx::SourceSurface> surface =
+      aTranslator->LookupSourceSurface(mActualSurface);
+  if (!surface) {
+    return false;
+  }
+
+  aTranslator->AddExportSurface(mExportID, surface);
+  return true;
+}
+
+template <class S>
+void RecordedAddExportSurface::Record(S& aStream) const {
+  WriteElement(aStream, mExportID);
+  WriteElement(aStream, mActualSurface);
+}
+
+template <class S>
+RecordedAddExportSurface::RecordedAddExportSurface(S& aStream)
+    : RecordedEventDerived(ADD_EXPORT_SURFACE) {
+  ReadElement(aStream, mExportID);
+  ReadElement(aStream, mActualSurface);
+}
+
+class RecordedRemoveExportSurface final
+    : public RecordedEventDerived<RecordedRemoveExportSurface> {
+ public:
+  explicit RecordedRemoveExportSurface(ReferencePtr aExportID)
+      : RecordedEventDerived(REMOVE_EXPORT_SURFACE), mExportID(aExportID) {}
+
+  template <class S>
+  MOZ_IMPLICIT RecordedRemoveExportSurface(S& aStream);
+
+  bool PlayCanvasEvent(CanvasTranslator* aTranslator) const;
+
+  template <class S>
+  void Record(S& aStream) const;
+
+  std::string GetName() const final { return "RecordedRemoveExportSurface"; }
+
+ private:
+  ReferencePtr mExportID;
+};
+
+inline bool RecordedRemoveExportSurface::PlayCanvasEvent(
+    CanvasTranslator* aTranslator) const {
+  aTranslator->RemoveExportSurface(mExportID);
+  return true;
+}
+
+template <class S>
+void RecordedRemoveExportSurface::Record(S& aStream) const {
+  WriteElement(aStream, mExportID);
+}
+
+template <class S>
+RecordedRemoveExportSurface::RecordedRemoveExportSurface(S& aStream)
+    : RecordedEventDerived(REMOVE_EXPORT_SURFACE) {
+  ReadElement(aStream, mExportID);
+}
+
 #define FOR_EACH_CANVAS_EVENT(f)                                    \
   f(CANVAS_BEGIN_TRANSACTION, RecordedCanvasBeginTransaction);      \
   f(CANVAS_END_TRANSACTION, RecordedCanvasEndTransaction);          \
@@ -896,7 +984,9 @@ RecordedPresentTexture::RecordedPresentTexture(S& aStream)
   f(PRESENT_TEXTURE, RecordedPresentTexture);                       \
   f(DEVICE_RESET_ACKNOWLEDGED, RecordedDeviceResetAcknowledged);    \
   f(AWAIT_TRANSLATION_SYNC, RecordedAwaitTranslationSync);          \
-  f(RESOLVE_EXTERNAL_SNAPSHOT, RecordedResolveExternalSnapshot);
+  f(RESOLVE_EXTERNAL_SNAPSHOT, RecordedResolveExternalSnapshot);    \
+  f(ADD_EXPORT_SURFACE, RecordedAddExportSurface);                  \
+  f(REMOVE_EXPORT_SURFACE, RecordedRemoveExportSurface);
 
 }  // namespace layers
 }  // namespace mozilla
diff --git a/gfx/layers/ipc/CanvasChild.cpp b/gfx/layers/ipc/CanvasChild.cpp
index c32621ef417..93378de00f5 100644
--- a/gfx/layers/ipc/CanvasChild.cpp
+++ b/gfx/layers/ipc/CanvasChild.cpp
@@ -115,19 +115,21 @@ class SourceSurfaceCanvasRecording final : public gfx::SourceSurface {
 
   ~SourceSurfaceCanvasRecording() {
     ReferencePtr surfaceAlias = this;
+    ReferencePtr exportID = mExportID;
     if (NS_IsMainThread()) {
       ReleaseOnMainThread(std::move(mRecorder), surfaceAlias,
-                          std::move(mRecordedSurface), std::move(mCanvasChild));
+                          std::move(mRecordedSurface), std::move(mCanvasChild),
+                          exportID);
       return;
     }
 
     mRecorder->AddPendingDeletion(
         [recorder = std::move(mRecorder), surfaceAlias,
          aliasedSurface = std::move(mRecordedSurface),
-         canvasChild = std::move(mCanvasChild)]() mutable -> void {
+         canvasChild = std::move(mCanvasChild), exportID]() mutable -> void {
           ReleaseOnMainThread(std::move(recorder), surfaceAlias,
-                              std::move(aliasedSurface),
-                              std::move(canvasChild));
+                              std::move(aliasedSurface), std::move(canvasChild),
+                              exportID);
         });
   }
 
@@ -162,10 +164,15 @@ class SourceSurfaceCanvasRecording final : public gfx::SourceSurface {
     return mRecordedSurface->ExtractSubrect(aRect);
   }
 
-  bool GetSurfaceDescriptor(SurfaceDescriptor& aDesc) const final {
+  bool GetSurfaceDescriptor(SurfaceDescriptor& aDesc) final {
+    static Atomic<uintptr_t> sNextExportID(0);
+    if (!mExportID) {
+      mExportID = gfx::ReferencePtr(++sNextExportID);
+      mRecorder->RecordEvent(RecordedAddExportSurface(mExportID, this));
+    }
     aDesc = SurfaceDescriptorCanvasSurface(
         static_cast<gfx::CanvasManagerChild*>(mCanvasChild->Manager())->Id(),
-        mCanvasChild->Id(), uintptr_t(gfx::ReferencePtr(this)));
+        mCanvasChild->Id(), uintptr_t(mExportID));
     return true;
   }
 
@@ -182,11 +189,15 @@ class SourceSurfaceCanvasRecording final : public gfx::SourceSurface {
   static void ReleaseOnMainThread(RefPtr<CanvasDrawEventRecorder> aRecorder,
                                   ReferencePtr aSurfaceAlias,
                                   RefPtr<gfx::SourceSurface> aAliasedSurface,
-                                  RefPtr<CanvasChild> aCanvasChild) {
+                                  RefPtr<CanvasChild> aCanvasChild,
+                                  ReferencePtr aExportID) {
     MOZ_ASSERT(NS_IsMainThread());
 
     aRecorder->RemoveStoredObject(aSurfaceAlias);
     aRecorder->RecordEvent(RecordedRemoveSurfaceAlias(aSurfaceAlias));
+    if (aExportID) {
+      aRecorder->RecordEvent(RecordedRemoveExportSurface(aExportID));
+    }
     aAliasedSurface = nullptr;
     aCanvasChild = nullptr;
     aRecorder = nullptr;
@@ -199,6 +210,7 @@ class SourceSurfaceCanvasRecording final : public gfx::SourceSurface {
   RefPtr<gfx::DataSourceSurface> mDataSourceSurface;
   bool mDetached = false;
   bool mMayInvalidate = false;
+  ReferencePtr mExportID;
 };
 
 class CanvasDataShmemHolder {
diff --git a/gfx/layers/ipc/CanvasTranslator.cpp b/gfx/layers/ipc/CanvasTranslator.cpp
index 2896b72ba81..b8872d9e463 100644
--- a/gfx/layers/ipc/CanvasTranslator.cpp
+++ b/gfx/layers/ipc/CanvasTranslator.cpp
@@ -399,7 +399,8 @@ already_AddRefed<gfx::DataSourceSurface> CanvasTranslator::WaitForSurface(
     return nullptr;
   }
   ReferencePtr idRef(aId);
-  if (!HasSourceSurface(idRef)) {
+  auto* surf = LookupExportSurface(idRef);
+  if (!surf) {
     if (!HasPendingEvent()) {
       return nullptr;
     }
@@ -413,12 +414,13 @@ already_AddRefed<gfx::DataSourceSurface> CanvasTranslator::WaitForSurface(
     mFlushCheckpoint = 0;
     // If there is still no surface, then it is unlikely to be produced
     // now, so give up.
-    if (!HasSourceSurface(idRef)) {
+    surf = LookupExportSurface(idRef);
+    if (!surf) {
       return nullptr;
     }
   }
   // The surface exists, so get its data.
-  return LookupSourceSurface(idRef)->GetDataSurface();
+  return surf->GetDataSurface();
 }
 
 void CanvasTranslator::RecycleBuffer() {
diff --git a/gfx/layers/ipc/CanvasTranslator.h b/gfx/layers/ipc/CanvasTranslator.h
index bd10e4c4e7c..2f567cb22be 100644
--- a/gfx/layers/ipc/CanvasTranslator.h
+++ b/gfx/layers/ipc/CanvasTranslator.h
@@ -326,6 +326,19 @@ class CanvasTranslator final : public gfx::InlineTranslator,
 
   static void Shutdown();
 
+  void AddExportSurface(gfx::ReferencePtr aRefPtr,
+                        gfx::SourceSurface* aSurface) {
+    mExportSurfaces.InsertOrUpdate(aRefPtr, RefPtr{aSurface});
+  }
+
+  void RemoveExportSurface(gfx::ReferencePtr aRefPtr) {
+    mExportSurfaces.Remove(aRefPtr);
+  }
+
+  gfx::SourceSurface* LookupExportSurface(gfx::ReferencePtr aRefPtr) {
+    return mExportSurfaces.GetWeak(aRefPtr);
+  }
+
  private:
   ~CanvasTranslator();
 
@@ -575,6 +588,8 @@ class CanvasTranslator final : public gfx::InlineTranslator,
   Mutex mCanvasTranslatorEventsLock;
   RefPtr<nsIRunnable> mCanvasTranslatorEventsRunnable;
   std::deque<UniquePtr<CanvasTranslatorEvent>> mPendingCanvasTranslatorEvents;
+
+  nsRefPtrHashtable<nsPtrHashKey<void>, gfx::SourceSurface> mExportSurfaces;
 };
 
 }  // namespace layers
Loading diff…