Closed
Bug 1187459
Opened 9 years ago
Closed 8 years ago
Memory-safety bug in Image11::map
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla44
People
(Reporter: q1, Assigned: jgilbert)
References
Details
(Keywords: sec-high)
Image11::map (gfx\angle\src\libGLESv2\renderer\d3d\d3d11\image11.cpp) does not properly report error status if its call to deviceContext -> Map fails due to a "device lost" error. Instead, it returns GL_NO_ERROR. This can cause a caller to use an uninitialized D3D11_MAPPED_SUBRESOURCE object. One such caller is Image11::loadData, which uses an uninitialized D3D11_MAPPED_SUBRESOURCE to determine where to copy the data specified by its |input| argument. This can cause loadData to overwrite an arbitrary (and possibly quite large) block of memory with data from |input|. The bug is at line 613, which should be followed by something like |return result;| 588:gl::Error Image11::map(D3D11_MAP mapType, D3D11_MAPPED_SUBRESOURCE *map) ... [*map is not initialized] 608: HRESULT result = deviceContext->Map(stagingTexture, subresourceIndex, mapType, 0, map); 609: 610: // this can fail if the device is removed (from TDR) 611: if (d3d11::isDeviceLostError(result)) 612: { 613: mRenderer->notifyDeviceLost(); 614: } 615: else if (FAILED(result)) 616: { 617: return gl::Error(GL_OUT_OF_MEMORY, "Failed to map staging texture, result: 0x%X.", result); 618: } 619: 620: mDirty = true; 621: 622: return gl::Error(GL_NO_ERROR); 623:} The bug is activated by, e.g., loadData: ... 264: D3D11_MAPPED_SUBRESOURCE mappedImage; 265: gl::Error error = map(D3D11_MAP_WRITE, &mappedImage); 266: if (error.isError()) 267: { 268: return error; 269: } 270: 271: uint8_t* offsetMappedData = (reinterpret_cast<uint8_t*>(mappedImage.pData) + (yoffset * mappedImage.RowPitch + xoffset * outputPixelSize + zoffset * mappedImage.DepthPitch)); 272: loadFunction(width, height, depth, 273: reinterpret_cast<const uint8_t*>(input), inputRowPitch, inputDepthPitch, 274: offsetMappedData, mappedImage.RowPitch, mappedImage.DepthPitch); 275: 276: unmap(); 277: 278: return gl::Error(GL_NO_ERROR); 279:} mappedImage is neither explicitly nor implicitly initialized before line 265 passes it to map. Line 271 then uses it to compute the destination address for a copy operation, and line 272 calls loadFunction to do the copy (which it does without testing for the error status set by map at line 613). I have verified the above by using the debugger to to skip line 608 and force isDeviceLostError on line 611 to return |true|. Additionally, unmap() on line 276 probably does something bad if line 608 fails.
Updated•9 years ago
|
Flags: sec-bounty?
Comment 1•9 years ago
|
||
I'm not sure how hard it is to force a device-lost error, but this sounds pretty bad.
Keywords: sec-high
Just disable the graphics card in the system device panel should force a device lost error. Chances are we'll crash before we can do a lot of harm, but perhaps not quickly enough. We should check if this is still the case in the trunk ANGLE.
Flags: needinfo?(jgilbert)
(In reply to Milan Sreckovic [:milan] from comment #2) > Just disable the graphics card in the system device panel should force a > device lost error. Chances are we'll crash before we can do a lot of harm, > but perhaps not quickly enough. We should check if this is still the case > in the trunk ANGLE. It's still there: github.com/google/angle/blob/master/src/libANGLE/renderer/d3d/d3d11/Image11.cpp . I will report this to Angle and post the bug number here.
Added kbrussel@alum.mit.edu to CC list at the request of the Angle team.
Comment 6•9 years ago
|
||
Thanks for CC'ing me. Do you have a test case which provokes this bug?
(In reply to Kenneth Russell from comment #6) > Thanks for CC'ing me. Do you have a test case which provokes this bug? No. I suppose that an attacker might cause the GPU driver to lose its mind and reset the device (= possible device-lost error) by using up all available texture memory, or possibly by causing the GPU to compute something very lengthy. Or she might leverage another bug in the GPU driver or OS that spuriously generates a device-lost error, as apparently is occurring in, e.g., http://steamcommunity.com/app/223470/discussions/1/882964760964387159/ and http://www.gamedev.net/topic/451670-persistent-d3derr_devicelost-problem-solved/ .
> Or she might leverage another bug in the GPU driver
> or OS that spuriously generates a device-lost error, as apparently is
> occurring in, e.g.,
> http://steamcommunity.com/app/223470/discussions/1/882964760964387159/ and
> http://www.gamedev.net/topic/451670-persistent-d3derr_devicelost-problem-
> solved/ .
What's occurring there is some bug that's generating a device-lost error, not an exploitation of this bug.
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(jgilbert)
Updated•9 years ago
|
Group: core-security → gfx-core-security
Updated•9 years ago
|
Flags: sec-bounty? → sec-bounty+
Comment 9•9 years ago
|
||
Ron, if you find a testcase for this, we can re-evaluate the bounty payment higher.
Reporter | ||
Comment 10•9 years ago
|
||
(In reply to Al Billings [:abillings] from comment #9) > Ron, if you find a testcase for this, we can re-evaluate the bounty payment > higher. Thanks!
Updated•8 years ago
|
Assignee: nobody → jgilbert
Comment 11•8 years ago
|
||
Jeff, are you still working on this or can you help find an owner?
Flags: needinfo?(jgilbert)
Assignee | ||
Comment 12•8 years ago
|
||
Man, this fell through the cracks, sorry! I see the bug, but it seems hard to act on. It's generally tricky to deterministically trigger a TDR, and I'm not sure how possible it would be to race to act on it before our internal mechanisms recognize the TDR occurred and forbid further WebGL calls. This seems an impractical vulnerability to act on, though I won't rule it out. This was fixed in a previous ANGLE update we did (bug 1211774): https://hg.mozilla.org/mozilla-central/diff/0c0c6ebfa5a5/gfx/angle/src/libANGLE/renderer/d3d/d3d11/Image11.cpp#l639
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(jgilbert)
Resolution: --- → FIXED
Updated•8 years ago
|
Group: gfx-core-security → core-security-release
Comment 13•8 years ago
|
||
Can't see us updating ANGLE on ESR 38 at this point given the lack of testing on that branch.
status-firefox45:
--- → fixed
status-firefox46:
--- → fixed
status-firefox-esr38:
--- → wontfix
status-firefox-esr45:
--- → fixed
Depends on: 1211774
Target Milestone: --- → mozilla44
Comment 14•8 years ago
|
||
Marking qe-verify- based on comment 12 and complexity of testing without PoC or test.
Flags: qe-verify-
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•