Moderate firefox Logic Error 🔧 Commit mapped

Overview

Moderate
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionSpoofing issue due to invalid pointer in the Graphics component
ComponentGraphics
Bug ClassLogic Error
Tracker2061393
Fix commit72d17dcee334 (firefox) +64/-19
CISA KEVNot listed
CreditedMozilla
Disclosed2026-09-15

Changed Functions

FunctionChangeNotes
if
gfx/ipc/GPUProcessManager.cpp
modified
mNamespace
gfx/layers/ipc/CompositorManagerParent.cpp
modified
mContentBridgeNamespace
gfx/layers/ipc/CompositorManagerParent.cpp
modified

Files Changed

  • gfx/ipc/GPUParent.cpp
  • gfx/ipc/GPUParent.h
  • gfx/ipc/GPUProcessManager.cpp
  • gfx/ipc/GPUProcessManager.h
  • gfx/ipc/PGPU.ipdl
  • gfx/layers/ipc/CompositorManagerParent.cpp
  • gfx/layers/ipc/CompositorManagerParent.h
  • gfx/layers/wr/AsyncImagePipelineManager.h
  • gfx/layers/wr/WebRenderBridgeParent.cpp
diff --git a/gfx/ipc/GPUParent.cpp b/gfx/ipc/GPUParent.cpp
index 57b56de9805..6073e3e0b66 100644
--- a/gfx/ipc/GPUParent.cpp
+++ b/gfx/ipc/GPUParent.cpp
@@ -447,7 +447,8 @@ mozilla::ipc::IPCResult GPUParent::RecvInitSandboxTesting(
 mozilla::ipc::IPCResult GPUParent::RecvInitCompositorManager(
     Endpoint<PCompositorManagerParent>&& aEndpoint, uint32_t aNamespace) {
   CompositorManagerParent::Create(std::move(aEndpoint), ContentParentId(),
-                                  aNamespace, /* aIsRoot */ true);
+                                  aNamespace, /* aContentBridgeNamespace */ 0,
+                                  /* aIsRoot */ true);
   return IPC_OK();
 }
 
@@ -600,8 +601,10 @@ mozilla::ipc::IPCResult GPUParent::RecvSimulateDeviceReset() {
 
 mozilla::ipc::IPCResult GPUParent::RecvNewContentCompositorManager(
     Endpoint<PCompositorManagerParent>&& aEndpoint,
-    const ContentParentId& aChildId, uint32_t aNamespace) {
+    const ContentParentId& aChildId, uint32_t aNamespace,
+    uint32_t aContentBridgeNamespace) {
   CompositorManagerParent::Create(std::move(aEndpoint), aChildId, aNamespace,
+                                  aContentBridgeNamespace,
                                   /* aIsRoot */ false);
   return IPC_OK();
 }
diff --git a/gfx/ipc/GPUParent.h b/gfx/ipc/GPUParent.h
index 73d43234540..fb5904bcf76 100644
--- a/gfx/ipc/GPUParent.h
+++ b/gfx/ipc/GPUParent.h
@@ -81,7 +81,8 @@ class GPUParent final : public PGPUParent {
       const BatteryInformation& aBatteryInfo);
   mozilla::ipc::IPCResult RecvNewContentCompositorManager(
       Endpoint<PCompositorManagerParent>&& aEndpoint,
-      const ContentParentId& aChildId, uint32_t aNamespace);
+      const ContentParentId& aChildId, uint32_t aNamespace,
+      uint32_t aContentBridgeNamespace);
   mozilla::ipc::IPCResult RecvNewContentImageBridge(
       Endpoint<PImageBridgeParent>&& aEndpoint, const ContentParentId& aChildId,
       uint32_t aNamespace);
diff --git a/gfx/ipc/GPUProcessManager.cpp b/gfx/ipc/GPUProcessManager.cpp
index 004ab4ac457..84f5323fe8e 100644
--- a/gfx/ipc/GPUProcessManager.cpp
+++ b/gfx/ipc/GPUProcessManager.cpp
@@ -1389,9 +1389,9 @@ bool GPUProcessManager::CreateContentBridges(
   const uint32_t compositorBridgeNamespace = AllocateNamespace();
   const uint32_t imageBridgeNamespace = AllocateNamespace();
   const uint32_t vrManagerNamespace = AllocateNamespace();
-  if (!CreateContentCompositorManager(aOtherProcess, aChildId,
-                                      compositorManagerNamespace,
-                                      aOutCompositor) ||
+  if (!CreateContentCompositorManager(
+          aOtherProcess, aChildId, compositorManagerNamespace,
+          compositorBridgeNamespace, aOutCompositor) ||
       !CreateContentImageBridge(aOtherProcess, aChildId, imageBridgeNamespace,
                                 aOutImageBridge) ||
       !CreateContentVRManager(aOtherProcess, aChildId, vrManagerNamespace,
@@ -1413,7 +1413,8 @@ bool GPUProcessManager::CreateContentBridges(
 
 bool GPUProcessManager::CreateContentCompositorManager(
     ipc::EndpointProcInfo aOtherProcess, dom::ContentParentId aChildId,
-    uint32_t aNamespace, ipc::Endpoint<PCompositorManagerChild>* aOutEndpoint) {
+    uint32_t aNamespace, uint32_t aContentBridgeNamespace,
+    ipc::Endpoint<PCompositorManagerChild>* aOutEndpoint) {
   MOZ_DIAGNOSTIC_ASSERT(IsGPUReady());
 
   ipc::Endpoint<PCompositorManagerParent> parentPipe;
@@ -1432,10 +1433,11 @@ bool GPUProcessManager::CreateContentCompositorManager(
   }
 
   if (mGPUChild) {
-    mGPUChild->SendNewContentCompositorManager(std::move(parentPipe), aChildId,
-                                               aNamespace);
+    mGPUChild->SendNewContentCompositorManager(
+        std::move(parentPipe), aChildId, aNamespace, aContentBridgeNamespace);
   } else if (!CompositorManagerParent::Create(std::move(parentPipe), aChildId,
                                               aNamespace,
+                                              aContentBridgeNamespace,
                                               /* aIsRoot */ false)) {
     return false;
   }
diff --git a/gfx/ipc/GPUProcessManager.h b/gfx/ipc/GPUProcessManager.h
index edb4fad41c1..0bf1d2272b4 100644
--- a/gfx/ipc/GPUProcessManager.h
+++ b/gfx/ipc/GPUProcessManager.h
@@ -245,6 +245,7 @@ class GPUProcessManager final : public GPUProcessHost::Listener {
   bool CreateContentCompositorManager(
       mozilla::ipc::EndpointProcInfo aOtherProcess,
       dom::ContentParentId aChildId, uint32_t aNamespace,
+      uint32_t aContentBridgeNamespace,
       mozilla::ipc::Endpoint<PCompositorManagerChild>* aOutEndpoint);
   bool CreateContentImageBridge(
       mozilla::ipc::EndpointProcInfo aOtherProcess,
diff --git a/gfx/ipc/PGPU.ipdl b/gfx/ipc/PGPU.ipdl
index 0bcb62f453a..39f017c37a4 100644
--- a/gfx/ipc/PGPU.ipdl
+++ b/gfx/ipc/PGPU.ipdl
@@ -87,7 +87,7 @@ parent:
   async NotifyBatteryInfo(BatteryInformation aBatteryInfo);
 
   // Create a new content-process compositor bridge.
-  async NewContentCompositorManager(Endpoint<PCompositorManagerParent> endpoint, ContentParentId childId, uint32_t aNamespace);
+  async NewContentCompositorManager(Endpoint<PCompositorManagerParent> endpoint, ContentParentId childId, uint32_t aNamespace, uint32_t aContentBridgeNamespace);
   async NewContentImageBridge(Endpoint<PImageBridgeParent> endpoint, ContentParentId childId, uint32_t aNamespace);
   async NewContentVRManager(Endpoint<PVRManagerParent> endpoint, ContentParentId childId, uint32_t aNamespace);
   async NewContentRemoteMediaManager(Endpoint<PRemoteMediaManagerParent> endpoint, ContentParentId childId);
diff --git a/gfx/layers/ipc/CompositorManagerParent.cpp b/gfx/layers/ipc/CompositorManagerParent.cpp
index 190e5326288..4f67e9d7d9f 100644
--- a/gfx/layers/ipc/CompositorManagerParent.cpp
+++ b/gfx/layers/ipc/CompositorManagerParent.cpp
@@ -42,8 +42,8 @@ CompositorManagerParent::CreateSameProcess(uint32_t aNamespace) {
   // The child is responsible for setting up the IPC channel in the same
   // process case because if we open from the child perspective, we can do it
   // on the main thread and complete before we return the manager handles.
-  RefPtr<CompositorManagerParent> parent =
-      new CompositorManagerParent(dom::ContentParentId(), aNamespace);
+  RefPtr<CompositorManagerParent> parent = new CompositorManagerParent(
+      dom::ContentParentId(), aNamespace, /* aContentBridgeNamespace */ 0);
   parent->SetOtherEndpointProcInfo(ipc::EndpointProcInfo::Current());
   return parent.forget();
 }
@@ -51,7 +51,8 @@ CompositorManagerParent::CreateSameProcess(uint32_t aNamespace) {
 /* static */
 bool CompositorManagerParent::Create(
     Endpoint<PCompositorManagerParent>&& aEndpoint,
-    dom::ContentParentId aChildId, uint32_t aNamespace, bool aIsRoot) {
+    dom::ContentParentId aChildId, uint32_t aNamespace,
+    uint32_t aContentBridgeNamespace, bool aIsRoot) {
   MOZ_ASSERT(NS_IsMainThread());
 
   // We are creating a manager for the another process, inside the GPU process
@@ -62,8 +63,8 @@ bool CompositorManagerParent::Create(
     return false;
   }
 
-  RefPtr<CompositorManagerParent> bridge =
-      new CompositorManagerParent(aChildId, aNamespace);
+  RefPtr<CompositorManagerParent> bridge = new CompositorManagerParent(
+      aChildId, aNamespace, aContentBridgeNamespace);
 
   RefPtr<Runnable> runnable =
       NewRunnableMethod<Endpoint<PCompositorManagerParent>&&, bool>(
@@ -114,11 +115,13 @@ CompositorManagerParent::CreateSameProcessWidgetCompositorBridge(
 }
 
 CompositorManagerParent::CompositorManagerParent(
-    dom::ContentParentId aContentId, uint32_t aNamespace)
+    dom::ContentParentId aContentId, uint32_t aNamespace,
+    uint32_t aContentBridgeNamespace)
     : mCompositorThreadHolder(CompositorThreadHolder::GetSingleton()),
       mSharedSurfacesHolder(MakeRefPtr<SharedSurfacesHolder>(aNamespace)),
       mContentId(aContentId),
-      mNamespace(aNamespace) {}
+      mNamespace(aNamespace),
+      mContentBridgeNamespace(aContentBridgeNamespace) {}
 
 CompositorManagerParent::~CompositorManagerParent() = default;
 
@@ -227,6 +230,17 @@ CompositorManagerParent::AllocPCompositorBridgeParent(
     const CompositorBridgeOptions& aOpt, const uint32_t& aNamespace) {
   switch (aOpt.type()) {
     case CompositorBridgeOptions::TContentCompositorOptions: {
+      // The namespace is chosen by the child, but it must match the one the
+      // UI process assigned for its compositor bridge. Otherwise a
+      // compromised content process could claim another namespace (e.g. the
+      // LayersId namespace) and pass ownership checks on pipeline ids that
+      // belong to other bridges (CompositorBridgeParentBase::OwnsPipelineId).
+      if (NS_WARN_IF(mContentBridgeNamespace == 0 ||
+                     aNamespace != mContentBridgeNamespace)) {
+        MOZ_ASSERT_UNREACHABLE("Invalid content compositor namespace!");
+        break;
+      }
+
       RefPtr bridge =
           MakeRefPtr<ContentCompositorBridgeParent>(this, aNamespace);
       return bridge.forget();
diff --git a/gfx/layers/ipc/CompositorManagerParent.h b/gfx/layers/ipc/CompositorManagerParent.h
index d80647da74e..48f300ff619 100644
--- a/gfx/layers/ipc/CompositorManagerParent.h
+++ b/gfx/layers/ipc/CompositorManagerParent.h
@@ -37,7 +37,7 @@ class CompositorManagerParent final : public PCompositorManagerParent {
       uint32_t aNamespace);
   static bool Create(Endpoint<PCompositorManagerParent>&& aEndpoint,
                      dom::ContentParentId aContentId, uint32_t aNamespace,
-                     bool aIsRoot);
+                     uint32_t aContentBridgeNamespace, bool aIsRoot);
   static void Shutdown();
 
   static already_AddRefed<CompositorBridgeParent>
@@ -93,7 +93,8 @@ class CompositorManagerParent final : public PCompositorManagerParent {
 
   static void ShutdownInternal();
 
-  CompositorManagerParent(dom::ContentParentId aContentId, uint32_t aNamespace);
+  CompositorManagerParent(dom::ContentParentId aContentId, uint32_t aNamespace,
+                          uint32_t aContentBridgeNamespace);
   virtual ~CompositorManagerParent();
 
   void Bind(Endpoint<PCompositorManagerParent>&& aEndpoint, bool aIsRoot);
@@ -105,6 +106,9 @@ class CompositorManagerParent final : public PCompositorManagerParent {
   AutoTArray<RefPtr<CompositorBridgeParent>, 1> mPendingCompositorBridges;
   const dom::ContentParentId mContentId;
   const uint32_t mNamespace;
+  // The id namespace the UI process assigned for this (content) child's
+  // compositor bridge; 0 when the child may not create content compositors.
+  const uint32_t mContentBridgeNamespace;
   uint32_t mLastSharedSurfaceResourceId MOZ_GUARDED_BY(sMonitor) = 0;
   RefPtr<RemoteTextureTxnScheduler> mRemoteTextureTxnScheduler;
 };
diff --git a/gfx/layers/wr/AsyncImagePipelineManager.h b/gfx/layers/wr/AsyncImagePipelineManager.h
index 47d64a09a4f..240bbe394ad 100644
--- a/gfx/layers/wr/AsyncImagePipelineManager.h
+++ b/gfx/layers/wr/AsyncImagePipelineManager.h
@@ -98,6 +98,15 @@ class AsyncImagePipelineManager final {
 
   void AddAsyncImagePipeline(const wr::PipelineId& aPipelineId,
                              WebRenderImageHost* aImageHost);
+  // Whether aPipelineId is currently registered by a live pipeline, which may
+  // be another bridge's root pipeline (registered via AddPipeline) as well as
+  // an async image pipeline. A holder that is merely waiting to be destroyed
+  // does not count: its id may legitimately be re-registered while the old
+  // holder drains, e.g. when a tab moves between windows.
+  bool HasLivePipeline(const wr::PipelineId& aPipelineId) const {
+    auto* holder = mPipelineTexturesHolders.Get(wr::AsUint64(aPipelineId));
+    return holder && holder->mDestroyedEpoch.isNothing();
+  }
   void RemoveAsyncImagePipeline(const wr::PipelineId& aPipelineId,
                                 AsyncImagePipelineOps* aPendingOps,
                                 wr::TransactionBuilder& aTxn);
diff --git a/gfx/layers/wr/WebRenderBridgeParent.cpp b/gfx/layers/wr/WebRenderBridgeParent.cpp
index c62aa9497b2..58842a96f2d 100644
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -2121,6 +2121,17 @@ void WebRenderBridgeParent::AddPipelineIdForCompositable(
     return;
   }
 
+  // The AsyncImagePipelineManager is shared by every bridge composited into
+  // this window, including the window's root pipeline. A pipeline id must not
+  // collide with one that is already live there, possibly registered by
+  // another bridge. Pipeline ids are never legitimately reused, so reject
+  // collisions outright.
+  if (mLateInit->mAsyncImageManager->HasLivePipeline(aPipelineId)) {
+    gfxCriticalNote << "Content attempted AddPipelineIdForCompositable with "
+                       "pipelineId already registered";
+    return;
+  }
+
   wrHost->SetWrBridge(aPipelineId, this);
   mAsyncCompositables.emplace(wr::AsUint64(aPipelineId), wrHost);
   mLateInit->mAsyncImageManager->AddAsyncImagePipeline(aPipelineId, wrHost);
Loading diff…