Low firefox Logic Error 🔧 Commit mapped

Overview

Low
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Impactlow
DescriptionSite isolation issue in the Audio/Video: Web Codecs component
ComponentGraphics
Bug ClassLogic Error
Tracker2050480
Fix commit7ae92e67d094 (firefox) +61/-66
CISA KEVNot listed
Credited5up3rh3i
Disclosed2026-08-18

Background

`NVImage`
a layers image type that holds a decoded NV12/NV21 (semi-planar, interleaved-chroma) video frame produced by the Web Codecs / media pipeline.
`NV12`/`NV21`
YUV formats storing a Y plane followed by a single plane of interleaved Cb/Cr bytes (mCbSkip == 1, mCrSkip == 1).
`I420` (YUV420P)
a fully planar YUV layout with separate Y, Cb, and Cr planes, which NVImage produces via libyuv::NV12ToI420/libyuv::NV21ToI420 so it can reuse PlanarYCbCrImage RGB conversion.
`PlanarYCbCrData`
the descriptor struct holding per-plane pointers (mYChannel, mCbChannel, mCrChannel) and strides that libyuv and gfx::ConvertYCbCrToRGB read and write.

Root Cause Analysis

In NVImage::BuildSurfaceDescriptorBuffer, the destination descriptor aData was copied from mData and its mCbChannel/mCrChannel were computed as aData.mYChannel + ... before aData.mYChannel was reassigned to the freshly allocated conversion buffer, so the destination chroma pointers were derived from the original source mData.mYChannel (the input NV12/NV21 buffer) rather than from the new buffer.

As a result, libyuv::NV12ToI420/NV21ToI420 wrote the planar Cb/Cr output into the source image buffer instead of the destination allocation, corrupting the shared decoded-frame memory and leaving the destination chroma region uninitialized. The invariant violated is that every destination plane pointer passed to libyuv must point inside the newly allocated output buffer, an ordering that the sibling NVImage::GetAsSourceSurface already got right by computing chroma offsets after setting mYChannel = buffer.get().

The fix factors the allocation and pointer setup into a single helper, CreateI420Buffer, that sets dst.mYChannel = result.mBuffer.get() first and only then derives dst.mCbChannel/dst.mCrChannel from it, so both call sites compute destination pointers consistently against the output buffer. It also stops passing the wrongly-populated aData to gfx::GetYCbCrToRGBDestFormatAndSize, using mData (from which only subsampling and picture size are read) on the cached-surface path where no conversion buffer exists.

Key insight
The single core mistake was computing the destination Cb/Cr plane pointers from the source mYChannel before reallocating mYChannel to the conversion buffer, so libyuv wrote chroma into the source frame; the fix centralizes buffer allocation and pointer derivation in CreateI420Buffer, guaranteeing all destination pointers reference the freshly allocated output.

Attack Path

  1. Deliver a decodable frame A site feeds NV12/NV21 video frames through Web Codecs so the compositor materializes an NVImage.
  2. Trigger surface build The rendering path invokes NVImage::BuildSurfaceDescriptorBuffer on the first (uncached) surface request, entering the conversion branch.
  3. Mis-targeted conversion libyuv writes the I420 chroma planes into the source buffer using pointers derived from the source mYChannel, corrupting the decoded-frame memory and yielding a surface built from stale/mismatched bytes.
  4. Observe crossed data Because the shared frame memory is mutated and the destination chroma is not properly populated, the resulting composited surface can reflect image data that does not correspond cleanly to the requesting context’s frame.

Impact Assessment

An attacker gains a logic-level site-isolation defect in the graphics/Web Codecs path: mishandled decoded-frame buffers can cause a rendered surface to be built from corrupted or mismatched pixel memory rather than the intended frame. This occurs in the process performing the NV12/NV21-to-I420 conversion for compositing, and requires the victim to decode and display a controlled video frame. Severity is low (LogicError, impact: low) with no memory-corruption primitive established by the diff.

Changed Functions

FunctionChangeNotes
if
gfx/layers/ImageContainer.cpp
modified

Files Changed

  • gfx/layers/ImageContainer.cpp

Audit Directions

  • Reassign-before-derive
    Flag any code that computes plane or sub-buffer pointers from a base pointer that is subsequently reassigned, especially where the derivation precedes the allocation of the real destination buffer.
  • libyuv destination pointers
    Verify that mYChannel/mCbChannel/mCrChannel passed as libyuv destinations all point inside the freshly allocated output buffer and never alias the source frame.
  • Cached vs conversion paths
    Audit surface builders that skip allocation when a cached surface exists, ensuring descriptor fields consumed later (e.g. by gfx::GetYCbCrToRGBDestFormatAndSize) are valid on every branch.
diff --git a/gfx/layers/ImageContainer.cpp b/gfx/layers/ImageContainer.cpp
index 5e9b8316dda..0c8d3321c55 100644
--- a/gfx/layers/ImageContainer.cpp
+++ b/gfx/layers/ImageContainer.cpp
@@ -1034,6 +1034,53 @@ IntSize NVImage::GetSize() const { return mSize; }
 
 IntRect NVImage::GetPictureRect() const { return mData.mPictureRect; }
 
+// Planar I420 conversion of an NV12/NV21 image: mBuffer owns the pixels, mData
+// points into it.
+struct I420Buffer {
+  UniquePtr<uint8_t[]> mBuffer;
+  PlanarYCbCrData mData;
+};
+
+// Convert |aData| (NV12 or NV21) to planar I420 in a freshly allocated buffer.
+// Returns an empty buffer (null mBuffer) if the required length overflows.
+static I420Buffer CreateI420Buffer(const PlanarYCbCrData& aData) {
+  // NV12/NV21 store interleaved chroma with Cb and Cr in adjacent bytes.
+  MOZ_ASSERT(aData.mCbSkip == 1 && aData.mCrSkip == 1,
+             "CreateI420Buffer expects NV12 or NV21 (interleaved chroma)");
+  MOZ_ASSERT((int)std::abs(aData.mCbChannel - aData.mCrChannel) == 1,
+             "NV12/NV21 chroma planes must be adjacent (interleaved)");
+  auto ySize = aData.YDataSize();
+  auto cbcrSize = aData.CbCrDataSize();
+  const CheckedInt32 bufferLength =
+      CheckedInt32(ySize.height) * aData.mYStride +
+      CheckedInt32(cbcrSize.height) * cbcrSize.width * 2;
+  if (NS_WARN_IF(!bufferLength.isValid())) {
+    return {};
+  }
+  I420Buffer result{MakeUnique<uint8_t[]>(bufferLength.value()), aData};
+
+  PlanarYCbCrData& dst = result.mData;
+  dst.mCbCrStride = cbcrSize.width;
+  dst.mCbSkip = 0;
+  dst.mCrSkip = 0;
+  dst.mYChannel = result.mBuffer.get();
+  dst.mCbChannel = dst.mYChannel + ySize.height * dst.mYStride;
+  dst.mCrChannel = dst.mCbChannel + cbcrSize.height * dst.mCbCrStride;
+
+  if (aData.mCbChannel < aData.mCrChannel) {  // NV12
+    libyuv::NV12ToI420(aData.mYChannel, aData.mYStride, aData.mCbChannel,
+                       aData.mCbCrStride, dst.mYChannel, dst.mYStride,
+                       dst.mCbChannel, dst.mCbCrStride, dst.mCrChannel,
+                       dst.mCbCrStride, ySize.width, ySize.height);
+  } else {  // NV21
+    libyuv::NV21ToI420(aData.mYChannel, aData.mYStride, aData.mCrChannel,
+                       aData.mCbCrStride, dst.mYChannel, dst.mYStride,
+                       dst.mCbChannel, dst.mCbCrStride, dst.mCrChannel,
+                       dst.mCbCrStride, ySize.width, ySize.height);
+  }
+  return result;
+}
+
 already_AddRefed<SourceSurface> NVImage::GetAsSourceSurface() {
   if (RefPtr<gfx::DataSourceSurface> cached = mSourceSurface.Get()) {
     return cached.forget();
@@ -1041,42 +1088,16 @@ already_AddRefed<SourceSurface> NVImage::GetAsSourceSurface() {
 
   // Convert the current NV12 or NV21 data to YUV420P so that we can follow the
   // logics in PlanarYCbCrImage::GetAsSourceSurface().
-  auto ySize = mData.YDataSize();
-  auto cbcrSize = mData.CbCrDataSize();
-  auto bufferLength = CheckedInt32(ySize.height) * mData.mYStride +
-                      CheckedInt32(cbcrSize.height) * cbcrSize.width * 2;
-  if (!bufferLength.isValid()) {
-    NS_ERROR("Image buffer length exceeds integer limits.");
+  auto i420 = CreateI420Buffer(mData);
+  if (!i420.mBuffer) {
     return nullptr;
   }
 
-  auto buffer = MakeUnique<uint8_t[]>(bufferLength.value());
-
-  Data aData = mData;
-  aData.mCbCrStride = cbcrSize.width;
-  aData.mCbSkip = 0;
-  aData.mCrSkip = 0;
-  aData.mYChannel = buffer.get();
-  aData.mCbChannel = aData.mYChannel + ySize.height * aData.mYStride;
-  aData.mCrChannel = aData.mCbChannel + cbcrSize.height * aData.mCbCrStride;
-
-  if (mData.mCbChannel < mData.mCrChannel) {  // NV12
-    libyuv::NV12ToI420(mData.mYChannel, mData.mYStride, mData.mCbChannel,
-                       mData.mCbCrStride, aData.mYChannel, aData.mYStride,
-                       aData.mCbChannel, aData.mCbCrStride, aData.mCrChannel,
-                       aData.mCbCrStride, ySize.width, ySize.height);
-  } else {  // NV21
-    libyuv::NV21ToI420(mData.mYChannel, mData.mYStride, mData.mCrChannel,
-                       mData.mCbCrStride, aData.mYChannel, aData.mYStride,
-                       aData.mCbChannel, aData.mCbCrStride, aData.mCrChannel,
-                       aData.mCbCrStride, ySize.width, ySize.height);
-  }
-
   // The logics in PlanarYCbCrImage::GetAsSourceSurface().
   gfx::IntSize size(mSize);
   gfx::SurfaceFormat format = gfx::ImageFormatToSurfaceFormat(
       gfxPlatform::GetPlatform()->GetOffscreenFormat());
-  gfx::GetYCbCrToRGBDestFormatAndSize(aData, format, size);
+  gfx::GetYCbCrToRGBDestFormatAndSize(i420.mData, format, size);
   if (mSize.width > PlanarYCbCrImage::MAX_DIMENSION ||
       mSize.height > PlanarYCbCrImage::MAX_DIMENSION) {
     NS_ERROR("Illegal image dest width or height");
@@ -1095,7 +1116,7 @@ already_AddRefed<SourceSurface> NVImage::GetAsSourceSurface() {
   }
 
   if (NS_WARN_IF(NS_FAILED(gfx::ConvertYCbCrToRGB(
-          aData, format, size, mapping.GetData(), mapping.GetStride())))) {
+          i420.mData, format, size, mapping.GetData(), mapping.GetStride())))) {
     return nullptr;
   }
 
@@ -1109,49 +1130,23 @@ nsresult NVImage::BuildSurfaceDescriptorBuffer(
     const std::function<MemoryOrShmem(uint32_t)>& aAllocate) {
   // Convert the current NV12 or NV21 data to YUV420P so that we can follow the
   // logics in PlanarYCbCrImage::GetAsSourceSurface().
-  auto ySize = mData.YDataSize();
-  auto cbcrSize = mData.CbCrDataSize();
-
-  Data aData = mData;
-  aData.mCbCrStride = cbcrSize.width;
-  aData.mCbSkip = 0;
-  aData.mCrSkip = 0;
-  aData.mCbChannel = aData.mYChannel + ySize.height * aData.mYStride;
-  aData.mCrChannel = aData.mCbChannel + cbcrSize.height * aData.mCbCrStride;
-
-  UniquePtr<uint8_t[]> buffer;
-
   RefPtr<gfx::DataSourceSurface> sourceSurface = mSourceSurface.Get();
+  I420Buffer i420;
   if (!sourceSurface) {
-    auto bufferLength = CheckedInt32(ySize.height) * mData.mYStride +
-                        CheckedInt32(cbcrSize.height) * cbcrSize.width * 2;
-
-    if (!bufferLength.isValid()) {
-      NS_ERROR("Image buffer length exceeds integer limits.");
-      return NS_ERROR_FAILURE;
-    }
-
-    buffer = MakeUnique<uint8_t[]>(bufferLength.value());
-    aData.mYChannel = buffer.get();
-
-    if (mData.mCbChannel < mData.mCrChannel) {  // NV12
-      libyuv::NV12ToI420(mData.mYChannel, mData.mYStride, mData.mCbChannel,
-                         mData.mCbCrStride, aData.mYChannel, aData.mYStride,
-                         aData.mCbChannel, aData.mCbCrStride, aData.mCrChannel,
-                         aData.mCbCrStride, ySize.width, ySize.height);
-    } else {  // NV21
-      libyuv::NV21ToI420(mData.mYChannel, mData.mYStride, mData.mCrChannel,
-                         mData.mCbCrStride, aData.mYChannel, aData.mYStride,
-                         aData.mCbChannel, aData.mCbCrStride, aData.mCrChannel,
-                         aData.mCbCrStride, ySize.width, ySize.height);
+    i420 = CreateI420Buffer(mData);
+    if (!i420.mBuffer) {
+      return NS_ERROR_INVALID_ARG;
     }
   }
 
-  // The logics in PlanarYCbCrImage::GetAsSourceSurface().
+  // The logics in PlanarYCbCrImage::GetAsSourceSurface(). mData is used here
+  // rather than i420.mData because only the chroma subsampling and picture size
+  // are read, which are identical, and i420.mData is left unset on the
+  // cached-surface path.
   gfx::IntSize size(mSize);
   gfx::SurfaceFormat format = gfx::ImageFormatToSurfaceFormat(
       gfxPlatform::GetPlatform()->GetOffscreenFormat());
-  gfx::GetYCbCrToRGBDestFormatAndSize(aData, format, size);
+  gfx::GetYCbCrToRGBDestFormatAndSize(mData, format, size);
   if (mSize.width > PlanarYCbCrImage::MAX_DIMENSION ||
       mSize.height > PlanarYCbCrImage::MAX_DIMENSION) {
     NS_ERROR("Illegal image dest width or height");
@@ -1171,7 +1166,7 @@ nsresult NVImage::BuildSurfaceDescriptorBuffer(
   }
 
   if (!sourceSurface) {
-    rv = gfx::ConvertYCbCrToRGB(aData, format, size, output, stride);
+    rv = gfx::ConvertYCbCrToRGB(i420.mData, format, size, output, stride);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       MOZ_ASSERT_UNREACHABLE("Failed to convert YUV into RGB data");
       return rv;
Loading diff…