Medium firefox Logic Error 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactmoderate
DescriptionIncorrect boundary conditions in the Graphics component
ComponentDOM
Bug ClassLogic Error
Tracker2018113
Fix commit629a8193cb25 (firefox) +50/-57
CISA KEVNot listed
CreditedSajeeb Lohani
Disclosed2026-03-24

Changed Functions

FunctionChangeNotes
if
gfx/layers/BufferTexture.cpp
modified
if
gfx/layers/ImageDataSerializer.cpp
modified

Files Changed

  • dom/media/ipc/RemoteImageHolder.cpp
  • gfx/layers/BufferTexture.cpp
  • gfx/layers/ImageContainer.cpp
  • gfx/layers/ImageDataSerializer.cpp
  • gfx/layers/ImageDataSerializer.h
  • gfx/layers/composite/TextureHost.cpp
  • gfx/layers/ipc/SharedPlanarYCbCrImage.cpp
diff --git a/dom/media/ipc/RemoteImageHolder.cpp b/dom/media/ipc/RemoteImageHolder.cpp
index 4cc53858d53..e5dcf8c0b03 100644
--- a/dom/media/ipc/RemoteImageHolder.cpp
+++ b/dom/media/ipc/RemoteImageHolder.cpp
@@ -79,27 +79,15 @@ already_AddRefed<Image> RemoteImageHolder::DeserializeImage(
     const YCbCrDescriptor& descriptor = sdBuffer.desc().get_YCbCrDescriptor();
 
     size_t descriptorSize = ImageDataSerializer::ComputeYCbCrBufferSize(
-        descriptor.ySize(), descriptor.yStride(), descriptor.cbCrSize(),
-        descriptor.cbCrStride(), descriptor.yOffset(), descriptor.cbOffset(),
-        descriptor.crOffset(), descriptor.colorDepth());
+        descriptor.display(), descriptor.ySize(), descriptor.yStride(),
+        descriptor.cbCrSize(), descriptor.cbCrStride(), descriptor.yOffset(),
+        descriptor.cbOffset(), descriptor.crOffset(), descriptor.colorDepth(),
+        descriptor.chromaSubsampling());
     if (NS_WARN_IF(descriptorSize == 0 || descriptorSize > bufferSize)) {
       MOZ_ASSERT_UNREACHABLE("Buffer too small to fit descriptor!");
       return nullptr;
     }
 
-    if (!IntRect(IntPoint(), descriptor.ySize())
-             .Contains(descriptor.display())) {
-      MOZ_ASSERT_UNREACHABLE("YCbCr display rect exceeds Y plane dimensions!");
-      return nullptr;
-    }
-
-    auto croppedCbCr = ImageDataSerializer::GetCroppedCbCrSize(descriptor);
-    if (croppedCbCr.width > descriptor.cbCrSize().width ||
-        croppedCbCr.height > descriptor.cbCrSize().height) {
-      MOZ_ASSERT_UNREACHABLE("YCbCr chroma dimensions exceed CbCr plane size!");
-      return nullptr;
-    }
-
     PlanarYCbCrData pData;
     pData.mYStride = descriptor.yStride();
     pData.mCbCrStride = descriptor.cbCrStride();
diff --git a/gfx/layers/BufferTexture.cpp b/gfx/layers/BufferTexture.cpp
index 13e04d2213a..373a3d31fc1 100644
--- a/gfx/layers/BufferTexture.cpp
+++ b/gfx/layers/BufferTexture.cpp
@@ -160,7 +160,8 @@ BufferTextureData* BufferTextureData::CreateForYCbCr(
     gfx::ColorRange aColorRange, gfx::ChromaSubsampling aSubsampling,
     TextureFlags aTextureFlags) {
   uint32_t bufSize = ImageDataSerializer::ComputeYCbCrBufferSize(
-      aYSize, aYStride, aCbCrSize, aCbCrStride, aColorDepth);
+      aDisplay, aYSize, aYStride, aCbCrSize, aCbCrStride, aColorDepth,
+      aSubsampling);
   if (bufSize == 0) {
     return nullptr;
   }
diff --git a/gfx/layers/ImageContainer.cpp b/gfx/layers/ImageContainer.cpp
index dcd577aed4e..b98a10454bc 100644
--- a/gfx/layers/ImageContainer.cpp
+++ b/gfx/layers/ImageContainer.cpp
@@ -815,8 +815,9 @@ nsresult PlanarYCbCrImage::BuildSurfaceDescriptorBuffer(
                                            yOffset, cbOffset, crOffset);
 
   uint32_t bufferSize = ImageDataSerializer::ComputeYCbCrBufferSize(
-      ySize, pdata->mYStride, cbcrSize, pdata->mCbCrStride, yOffset, cbOffset,
-      crOffset, pdata->mColorDepth);
+      pdata->mPictureRect, ySize, pdata->mYStride, cbcrSize, pdata->mCbCrStride,
+      yOffset, cbOffset, crOffset, pdata->mColorDepth,
+      pdata->mChromaSubsampling);
 
   aSdBuffer.data() = aAllocate(bufferSize);
 
diff --git a/gfx/layers/ImageDataSerializer.cpp b/gfx/layers/ImageDataSerializer.cpp
index d27e2437b64..d7ce2a4c868 100644
--- a/gfx/layers/ImageDataSerializer.cpp
+++ b/gfx/layers/ImageDataSerializer.cpp
@@ -67,40 +67,47 @@ static bool CheckYCbCrStride(const gfx::IntSize& aSize, int32_t aStride,
 }
 
 // Minimum required shmem size in bytes
-uint32_t ComputeYCbCrBufferSize(const gfx::IntSize& aYSize, int32_t aYStride,
+uint32_t ComputeYCbCrBufferSize(const gfx::IntRect& aDisplay,
+                                const gfx::IntSize& aYSize, int32_t aYStride,
                                 const gfx::IntSize& aCbCrSize,
-                                int32_t aCbCrStride, gfx::ColorDepth aDepth) {
+                                int32_t aCbCrStride, gfx::ColorDepth aDepth,
+                                const ChromaSubsampling aSubsampling) {
   MOZ_ASSERT(aYSize.height >= 0 && aYSize.width >= 0);
 
-  if (aYSize.height < 0 || aYSize.width < 0 || aCbCrSize.height < 0 ||
+  if (aDisplay.IsEmpty() || aDisplay.x < 0 || aDisplay.y < 0 ||
+      !gfx::IntRect(gfx::IntPoint(), aYSize).Contains(aDisplay) ||
+      aYSize.height < 0 || aYSize.width < 0 || aCbCrSize.height < 0 ||
       aCbCrSize.width < 0 ||
       !gfx::Factory::AllowedSurfaceSize(IntSize(aYStride, aYSize.height)) ||
       !gfx::Factory::AllowedSurfaceSize(
           IntSize(aCbCrStride, aCbCrSize.height)) ||
       !CheckYCbCrStride(aYSize, aYStride, aDepth) ||
-      !CheckYCbCrStride(aCbCrSize, aCbCrStride, aDepth)) {
+      !CheckYCbCrStride(aCbCrSize, aCbCrStride, aDepth) ||
+      (aCbCrSize != ChromaSize(aYSize, aSubsampling))) {
     return 0;
   }
 
-  // Overflow checks are performed in AllowedSurfaceSize
-  return GetAlignedStride<4>(aYSize.height, aYStride) +
-         2 * GetAlignedStride<4>(aCbCrSize.height, aCbCrStride);
+  // Overflow checks are performed only individually in AllowedSurfaceSize
+  auto bufLen =
+      CheckedInt<uint32_t>(GetAlignedStride<4>(aYSize.height, aYStride)) +
+      CheckedInt<uint32_t>(GetAlignedStride<4>(aCbCrSize.height, aCbCrStride)) *
+          2;
+  if (!bufLen.isValid()) {
+    return 0;
+  }
+  return bufLen.value();
 }
 
-uint32_t ComputeYCbCrBufferSize(const gfx::IntSize& aYSize, int32_t aYStride,
+uint32_t ComputeYCbCrBufferSize(const gfx::IntRect& aDisplay,
+                                const gfx::IntSize& aYSize, int32_t aYStride,
                                 const gfx::IntSize& aCbCrSize,
                                 int32_t aCbCrStride, uint32_t aYOffset,
                                 uint32_t aCbOffset, uint32_t aCrOffset,
-                                gfx::ColorDepth aDepth) {
-  MOZ_ASSERT(aYSize.height >= 0 && aYSize.width >= 0);
-
-  if (aYSize.height < 0 || aYSize.width < 0 || aCbCrSize.height < 0 ||
-      aCbCrSize.width < 0 ||
-      !gfx::Factory::AllowedSurfaceSize(IntSize(aYStride, aYSize.height)) ||
-      !gfx::Factory::AllowedSurfaceSize(
-          IntSize(aCbCrStride, aCbCrSize.height)) ||
-      !CheckYCbCrStride(aYSize, aYStride, aDepth) ||
-      !CheckYCbCrStride(aCbCrSize, aCbCrStride, aDepth)) {
+                                gfx::ColorDepth aDepth,
+                                const ChromaSubsampling aSubsampling) {
+  uint32_t minBufLen = ComputeYCbCrBufferSize(
+      aDisplay, aYSize, aYStride, aCbCrSize, aCbCrStride, aDepth, aSubsampling);
+  if (minBufLen == 0) {
     return 0;
   }
 
@@ -118,7 +125,8 @@ uint32_t ComputeYCbCrBufferSize(const gfx::IntSize& aYSize, int32_t aYStride,
   crEnd += cbCrLength;
 
   if (!yEnd.isValid() || !cbEnd.isValid() || !crEnd.isValid() ||
-      yEnd.value() > aCbOffset || cbEnd.value() > aCrOffset) {
+      yEnd.value() > aCbOffset || cbEnd.value() > aCrOffset ||
+      crEnd.value() < minBufLen) {
     return 0;
   }
 
diff --git a/gfx/layers/ImageDataSerializer.h b/gfx/layers/ImageDataSerializer.h
index 8130b764c63..ee79edb6f9a 100644
--- a/gfx/layers/ImageDataSerializer.h
+++ b/gfx/layers/ImageDataSerializer.h
@@ -39,14 +39,18 @@ uint32_t ComputeRGBBufferSize(gfx::IntSize aSize, gfx::SurfaceFormat aFormat);
 /// This function is meant as a helper to know how much shared memory we need
 /// to allocate in a shmem in order to place a shared YCbCr image blob of
 /// given dimensions.
-uint32_t ComputeYCbCrBufferSize(const gfx::IntSize& aYSize, int32_t aYStride,
+uint32_t ComputeYCbCrBufferSize(const gfx::IntRect& aDisplay,
+                                const gfx::IntSize& aYSize, int32_t aYStride,
                                 const gfx::IntSize& aCbCrSize,
-                                int32_t aCbCrStride, gfx::ColorDepth aDepth);
-uint32_t ComputeYCbCrBufferSize(const gfx::IntSize& aYSize, int32_t aYStride,
+                                int32_t aCbCrStride, gfx::ColorDepth aDepth,
+                                const gfx::ChromaSubsampling aSubsampling);
+uint32_t ComputeYCbCrBufferSize(const gfx::IntRect& aDisplay,
+                                const gfx::IntSize& aYSize, int32_t aYStride,
                                 const gfx::IntSize& aCbCrSize,
                                 int32_t aCbCrStride, uint32_t aYOffset,
                                 uint32_t aCbOffset, uint32_t aCrOffset,
-                                gfx::ColorDepth aDepth);
+                                gfx::ColorDepth aDepth,
+                                const gfx::ChromaSubsampling aSubsampling);
 uint32_t ComputeYCbCrBufferSize(uint32_t aBufferSize);
 
 void ComputeYCbCrOffsets(int32_t yStride, int32_t yHeight, int32_t cbCrStride,
diff --git a/gfx/layers/composite/TextureHost.cpp b/gfx/layers/composite/TextureHost.cpp
index b03447b0b70..af86919540f 100644
--- a/gfx/layers/composite/TextureHost.cpp
+++ b/gfx/layers/composite/TextureHost.cpp
@@ -277,21 +277,11 @@ already_AddRefed<TextureHost> CreateBackendIndependentTextureHost(
           switch (desc.type()) {
             case BufferDescriptor::TYCbCrDescriptor: {
               const YCbCrDescriptor& ycbcr = desc.get_YCbCrDescriptor();
-              if (!gfx::IntRect(gfx::IntPoint(), ycbcr.ySize())
-                       .Contains(ycbcr.display())) {
-                NS_ERROR("YCbCr display rect exceeds Y plane dimensions!");
-                return nullptr;
-              }
-              auto croppedCbCr = ImageDataSerializer::GetCroppedCbCrSize(ycbcr);
-              if (croppedCbCr.width > ycbcr.cbCrSize().width ||
-                  croppedCbCr.height > ycbcr.cbCrSize().height) {
-                NS_ERROR("YCbCr display rect exceeds CbCr plane dimensions!");
-                return nullptr;
-              }
               reqSize = ImageDataSerializer::ComputeYCbCrBufferSize(
-                  ycbcr.ySize(), ycbcr.yStride(), ycbcr.cbCrSize(),
-                  ycbcr.cbCrStride(), ycbcr.yOffset(), ycbcr.cbOffset(),
-                  ycbcr.crOffset(), ycbcr.colorDepth());
+                  ycbcr.display(), ycbcr.ySize(), ycbcr.yStride(),
+                  ycbcr.cbCrSize(), ycbcr.cbCrStride(), ycbcr.yOffset(),
+                  ycbcr.cbOffset(), ycbcr.crOffset(), ycbcr.colorDepth(),
+                  ycbcr.chromaSubsampling());
Loading diff…