Latent out-of-bounds write in TexSubImage2DWithoutUnpackSubimage
Categories
(Core :: Graphics, defect, P2)
Tracking
()
People
(Reporter: deian, Assigned: sotaro)
Details
(Keywords: csectype-intoverflow, sec-low, Whiteboard: [post-critsmash-triage][adv-main68+])
Attachments
(1 file)
I believe there is a potential out-of-bounds write in TexSubImage2DWithoutUnpackSubimage [1]:
static void TexSubImage2DWithoutUnpackSubimage(
GLContext* gl, GLenum target, GLint level, GLint xoffset, GLint yoffset,
GLsizei width, GLsizei height, GLsizei stride, GLint pixelsize,
GLenum format, GLenum type, const GLvoid* pixels) {
...
unsigned char* newPixels =
new (fallible) unsigned char[width * height * pixelsize]; // width * height * pixelsize overflows to allocate small buffer (smaller than width * pixelsize)
if (newPixels) {
unsigned char* rowDest = newPixels;
const unsigned char* rowSource = (const unsigned char*)pixels;
for (int h = 0; h < height; h++) {
memcpy(rowDest, rowSource, width * pixelsize); // copy width * pixelsize bytes
I'm not sure if this is easy to trigger, but seemed worth reporting. Apologies
if this turns out to not be a concern.
[1] https://searchfox.org/mozilla-central/source/gfx/gl/GLUploadHelpers.cpp#147-154
Updated•5 years ago
|
Updated•5 years ago
|
Comment 1•5 years ago
|
||
width
and height
are never more than 16384. At 16Ki*16Ki, 4bpp is 1GiB, but if we allow 8bpp (or higher) here, that's 2GiB, which would overflow on 32-bit.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 2•5 years ago
|
||
Sotaro, could you help with this one? Thank you!
Assignee | ||
Comment 3•5 years ago
•
|
||
TexSubImage2DWithoutUnpackSubimage() is called under UploadImageDataToTexture().
There are 2 call sequences.
[1]
- UploadImageDataToTexture()
- TexSubImage2DHelper()
- TexSubImage2DWithoutUnpackSubimage()
[2]
- UploadImageDataToTexture()
- TexImage2DHelper()
- TexSubImage2DHelper()
- TexSubImage2DWithoutUnpackSubimage()
UploadImageDataToTexture() supports the following surface formats. All pixel sizes of surface formats are less than or equal to 4. Then TexSubImage2DWithoutUnpackSubimage() does not exceed 2GiB and does not overflow on 32-bit.
- SurfaceFormat::B8G8R8X8
- SurfaceFormat::R8G8B8A8
- SurfaceFormat::R8G8B8X8
- SurfaceFormat::R5G6B5_UINT16
- SurfaceFormat::A8
- SurfaceFormat::A16
https://searchfox.org/mozilla-central/source/gfx/gl/GLUploadHelpers.cpp#293
Assignee | ||
Comment 4•5 years ago
|
||
Similar buffer allocation exists in TexImage2DHelper(). It is "new unsigned char[paddedWidth * paddedHeight * pixelsize]".
Comment 6•5 years ago
|
||
Should we perhaps use CheckedInt here to ensure that even if we change things in the future we catch any overflows that may occur?
Comment 7•5 years ago
|
||
And maybe a runtime assert in UploadImageDataToTexture about the image size because I'm sure we have other spots that will keel over if we ever exceed that size.
Assignee | ||
Comment 9•5 years ago
|
||
Assignee | ||
Comment 10•5 years ago
|
||
Comment 11•5 years ago
|
||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 12•5 years ago
|
||
This unfortunately does not qualify for a bounty because the latent bug is unreachable.
Comment 13•5 years ago
|
||
Because this is a latent issue, I'm unassigned the CVE and putting this into the Firefox rollup. (The CVE is burned due to exposure though.)
Updated•4 years ago
|
Description
•