Last Comment Bug 656752 - (CVE-2011-2367) WebGL crash [@gleRunVertexSubmitImmediate()]
(CVE-2011-2367)
: WebGL crash [@gleRunVertexSubmitImmediate()]
Status: RESOLVED FIXED
: [sg:high] arbitrary memory read [4.0....
: crash, testcase
Product: Core
Classification: Components
Component: Canvas: WebGL
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: canvas.webgl
:
:
: 658170
  Show dependency treegraph
 
Reported: 2011-05-12 14:05 PDT by Christoph Diehl [:cdiehl]
Modified: 2011-07-12 08:26 PDT (History)
16 users (show)
See Also:
Crash Signature:
[@gleRunVertexSubmitImmediate()]
  +
  fixed
  ---
  ---
  ---
  ---
  ---
  ---
  ---
  ?
  wanted
  ---
  unaffected
  ---
  unaffected


Attachments
testcase (6.59 KB, application/zip)
2011-05-12 14:05 PDT, Christoph Diehl [:cdiehl]
no flags Details
callstack (12.54 KB, text/plain)
2011-05-12 14:05 PDT, Christoph Diehl [:cdiehl]
no flags Details
fix bindBuffer() bookkeeping (1.64 KB, patch)
2011-05-16 10:51 PDT, Benoit Jacob [:bjacob] on vacation until July 22
jmuizelaar: review+
clegnitto: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Summon comment box

Description Christoph Diehl [:cdiehl] 2011-05-12 14:05:16 PDT
Created attachment 532023 [details]
testcase

Environment:

ProductName:	Mac OS X
ProductVersion:	10.6.7
BuildVersion:	10J869

OpenGL renderer string: NVIDIA GeForce GT 330M OpenGL Engine
OpenGL version string: 2.1 NVIDIA-1.6.26

and 

OpenGL renderer string: ATI Radeon HD 6750M OpenGL Engine
OpenGL version string: 2.1 ATI-1.6.32
Comment 1 Christoph Diehl [:cdiehl] 2011-05-12 14:05:47 PDT
Created attachment 532024 [details]
callstack
Comment 2 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 08:22:31 PDT
Bingo: I can reproduce on Linux:
https://crash-stats.mozilla.com/report/index/a58d28f4-8125-447b-8ff8-39d102110516

That means it's almost certainly a bug in my code.
Comment 3 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 08:34:30 PDT
Seems to be that the WebGL state tracking code is getting confused into believing that there is an element array buffer bound (mBoundElementArrayBuffer is non-null) when there really isn't.
Comment 4 Jeff Muizelaar [:jrmuizel] 2011-05-16 08:49:00 PDT
Seems like you might be able to use this to upload arbitrary data to the gpu and then extract it's contents.
Comment 5 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 08:49:57 PDT
Indeed... very scary.
Comment 6 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 10:51:08 PDT
Created attachment 532684 [details] [review]
fix bindBuffer() bookkeeping

There you go: bindBuffer() was updating its bookkeeping before parameter validation was complete. The testcase was hitting a case where the bookkeeping was updated but the corresponding GL call was not performed, so the bookkeeping was no longer reflecting the actual GL state.
Comment 7 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 11:03:45 PDT
(In reply to comment #4)
> Seems like you might be able to use this to upload arbitrary data to the gpu
> and then extract it's contents.

Just for the record, here's how an attack could have proceeded, to implement PEEK:
1. set up vertex data (bound vertex buffers) so that the i-th vertex contains the value i.
2. set up a trivial vertex shader that just copies the vertex, and a fragment shader that encodes the vertex x-coord as color
3. reproduce the situation hit by Christoph's test case, call glDrawElements drawing 1 vertex as GL_POINTS, with an 'indices' parameter equal to some pointer you want to read the memory at.
4. now your rendered pixel's color tells you what was at this memory location
Comment 8 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 11:47:00 PDT
For sure we want this in Firefox 5.

What do we do for Firefox 4? Is this worth a chemspill?

Note, regarding comment 7: there's one little mitigation of the severity of this bug, it's that this pointer parameter gets validated as an index into a vertex array, so if it's greater than the length of currently bound arrays, it gets rejected. Of course that's not enough to make one feel comfortable.
Comment 9 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 11:55:30 PDT
Important: this bug has been open for 1 day before getting hidden.
Comment 10 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 11:56:21 PDT
Ugh, been open for 4 days actually.
Comment 11 Christian Legnitto [:LegNeato] 2011-05-16 12:18:26 PDT
Comment on attachment 532684 [details] [review]
fix bindBuffer() bookkeeping

Approved for mozilla-aurora
Comment 12 Daniel Veditz 2011-05-16 12:34:17 PDT
If we did a 4.0.x chemspill it might be worth it to take this fix.
Comment 13 Sheila Mooney 2011-05-16 13:29:18 PDT
Can someone land this. We are soon going to merge over to Beta.
Comment 14 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 14:09:18 PDT
Landed on Mozilla-Aurora:

http://hg.mozilla.org/mozilla-aurora/rev/549dfefec7c0

But now I'm a bit scared: what if the bad guys are watching the logs?
Comment 15 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 14:14:05 PDT
Maybe I shouldn't land this on mozilla-central for now. In case the bad guys are watching only mozilla-central and not mozilla-aurora.
Comment 16 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 14:23:04 PDT
After discussion with Jeff and Ehsan, landed this on mozilla-central
http://hg.mozilla.org/mozilla-central/rev/db4ba8614fb6

Looking back at the patch, I find some relative comfort in the fact that it's only touching bindBuffer() and doesn't give away the clue that there was an exploit specifically with element array pointers i.e. the old OpenGL-1-ish signature of glDrawElements that's not supposed to be exposed by WebGL.
Comment 17 Benoit Jacob [:bjacob] on vacation until July 22 2011-05-16 14:24:09 PDT
FIXED in firefox 5 and 6, but the question is still open for Firefox 4.

Note You need to log in before you can comment on or make changes to this bug.