OOB read in GPU process -- YCbCrDescriptor display rect not validated against ySize, allows reading past shmem buffer
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: lorenzoauto311, Assigned: tnikkel)
References
()
Details
(Keywords: ai-involved, reporter-external, Whiteboard: [client-bounty-form])
Steps to reproduce:
Found this while going through the texture handling in gfx/layers. The YCbCrDescriptor IPDL struct (LayersSurfaces.ipdlh:159-173) has two separate size fields:
- ySize: actual Y channel buffer dimensions
- display: IntRect for the picture/rendering area
The buffer size validation in TextureHost.cpp:279-284 only checks ySize:
const YCbCrDescriptor& ycbcr = desc.get_YCbCrDescriptor();
reqSize = ImageDataSerializer::ComputeYCbCrBufferSize(
ycbcr.ySize(), ycbcr.yStride(), ycbcr.cbCrSize(),
ycbcr.cbCrStride(), ycbcr.yOffset(), ycbcr.cbOffset(),
ycbcr.crOffset());
// ...
if (reqSize == 0 || bufSize < reqSize) {
// reject
}
But then the rendering side uses display().Size() everywhere:
RenderBufferTextureHost.cpp:29 -- constructor sets mSize from display:
mSize = ycbcr.display().Size();
RenderBufferTextureHost.cpp:80-88 -- Lock() wraps the shmem buffer using display size:
mYSurface = gfx::Factory::CreateWrappingDataSourceSurface(
layers::ImageDataSerializer::GetYChannel(GetBuffer(), desc),
desc.yStride(), desc.display().Size(), gfx::SurfaceFormat::A8);
Same for CbCr via GetCroppedCbCrSize (ImageDataSerializer.cpp:358-361):
gfx::IntSize GetCroppedCbCrSize(const YCbCrDescriptor& aDescriptor) {
return ChromaSize(aDescriptor.display().Size(),
aDescriptor.chromaSubsampling());
}
BufferTextureHost::PushExternalImage (TextureHost.cpp:552) also uses display size for the WebRender image descriptors:
gfx::IntSize ySize = desc.display().Size();
And DataSourceSurfaceFromYCbCrDescriptor (ImageDataSerializer.cpp:284-331) uses display for the output surface and mPictureRect:
const gfx::IntRect display = aDescriptor.display();
const gfx::IntSize size = display.Size();
// ... creates result surface with size, converts YCbCr using display as picture rect
Nowhere does anyone check that display.Size() fits within ySize. CreateWrappingDataSourceSurface in Factory.cpp explicitly skips AllowedSurfaceSize validation because "the data is already allocated" (line 1054) -- but the size being used doesn't match the allocated data.
A compromised content process sends a YCbCrDescriptor over IPC with:
- ySize = (640, 480), yStride = 640, yOffset = 0
- cbCrSize = (320, 240), cbCrStride = 320
- display = IntRect(0, 0, 1920, 1080)
- shmem buffer sized for 640x480 YUV (passes validation since it checks ySize)
The GPU/compositor process wraps the shmem with a 1920x1080 DataSourceSurface. When WebRender reads the texture, it reads past the end of the shmem -- that's OOB read of GPU process memory.
The CbCr channels are also affected. GetCroppedCbCrSize computes chroma dimensions from display, not cbCrSize. So the Cb and Cr surfaces also extend past their buffers.
The buffer arrives through CompositableTransactionParent/TextureHost creation. The content process controls the descriptor fields completely.
Actual results:
The GPU/compositor process reads past the end of the shared memory buffer. The read area is (display.width * display.height - ySize.width * ySize.height) bytes past the buffer for the Y channel. For the example above that's (19201080 - 640480) = 1,766,400 bytes of OOB read. Depending on the shmem mapping, this either reads adjacent GPU process heap/data or causes an access violation.
Expected results:
TextureHost creation should validate that the display rect fits within the channel dimensions. Something like:
if (ycbcr.display().XMost() > ycbcr.ySize().width ||
ycbcr.display().YMost() > ycbcr.ySize().height) {
// reject the texture
}
Or better, all the rendering paths that use display().Size() should clamp to ySize, since the buffer was allocated for ySize.
Tested against current mozilla-central.
Comment 1•8 months ago
|
||
Tested against current mozilla-central.
The code in CreateBackendIndependentTextureHost for the TYCbCrDescriptor case has some additional checks as of February 26 (here is the full diff).
| Assignee | ||
Comment 2•8 months ago
|
||
This sounds like a dupe of an existing bug. I'll take and confirm.
Comment 3•8 months ago
|
||
What revision of mozilla-central? Have you tried with code from March 2026?
Updated•8 months ago
|
| Assignee | ||
Comment 4•8 months ago
|
||
It does not look like you are referring to the current mozilla-central. As Andrew pointed out checks have been landed for this specific problem. In fact that code you mention at TextureHost.cpp:279-284 and quoted code does not look like that in current mozilla-central as a check for the very problem you mention has been added there. And that landed Feb 26/27.
I'll wait for the reporter to reply, but this looks like a dupe of bug 2015268 so far.
I was working from a gecko-dev snapshot pulled before the February 26 fix. I didn't realize the issue had already been addressed. If bug 2015268 covers the same
display vs ySize validation gap, then this is indeed a duplicate. Thanks for pointing it out.
| Assignee | ||
Updated•8 months ago
|
Comment 7•8 months ago
•
|
||
(In reply to Lorenzo from comment #5)
I was working from a gecko-dev snapshot pulled before the February 26 fix.
This has been mentioned to you elsewhere I think, but just to reiterate, gecko-dev hasn't been updated since July 2025, as indicated by the banner at the top. The current git repository being used for Firefox is https://github.com/mozilla-firefox/firefox
Updated•8 months ago
|
Updated•8 months ago
|
Updated•3 months ago
|
Description
•