686398 - WebGL crash, addressing Texture ImageInfo out of bounds

Status: VERIFIED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Brandon Sterne (:bsterne)]

Ben Hawkes from Google reported the following to security@m.o today:

Hi,

I have found a potentially exploitable crash in Firefox WebGL. The attached test case crashes Firefox 6.0.2 and Nightly on a Windows 7 machine (Nvidia Quadro NVS 140M with driver version 186.94). The following crash was observed after running the test case (refresh may be required to trigger crash):

7035f4bf 8b08            mov     ecx,dword ptr [eax]  ds:002b:69646172=????????
0:000:x86> k
ChildEBP RetAddr
WARNING: Stack unwind information not available. Following frames may be wrong.
0017bbe0 7035f4df libGLESv2!Ordinal158+0xa51a
0017bc14 7036118f libGLESv2!Ordinal158+0xa53a
0017bc34 70353c3c libGLESv2!Ordinal158+0xc1ea
*** ERROR: Symbol file could not be found.  Defaulted to export
symbols for xul.dll -
0017bc7c 69eed549 libGLESv2!glTexImage2D+0x331
0017bcc4 69ef3aba xul!NS_StringCloneData_P+0x8c95
0017bd04 6a10ddd1 xul!gfxFontUtils::RenameFont+0x3508
*** ERROR: Symbol file could not be found.  Defaulted to export
symbols for mozjs.dll -
0017bd64 705e779f xul!XRE_SendTestShellCommand+0x8f3e
0017c410 705e7658 mozjs!JS_GetTypeInferenceMemoryStats+0x666f
0017c42c 705e7847 mozjs!JS_GetTypeInferenceMemoryStats+0x6528
00000000 00000000 mozjs!JS_GetTypeInferenceMemoryStats+0x6717

7043f4bf 8b08            mov     ecx,dword ptr [eax]  ds:002b:3f800000=????????
7043f4c1 50              push    eax
7043f4c2 ff5108          call    dword ptr [ecx+8]

This is the most common crash that I have encountered on this test
case, but I've also observed a crash at:

libGLESv2!Ordinal158+0xa611:
70d11378 ff5134          call    dword ptr [ecx+34h]  ds:002b:00037c41=????????

Both instances suggest an exploitable condition in ANGLE/libGLESv2. Please cc on any bugs filed, and I'll be happy to help with reproduction and root cause analysis.

Thanks,
Ben

Comment 1

[Brandon Sterne (:bsterne)]

Attachment: testcase

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Thanks -- hats off, even, for such helpfulness -- again to Ben Hawkes and Google.

Reproduced on 1st try here on Firefox 9.0a1 32bit / Win7 64bit / NVIDIA 275.33

Crash link:
https://crash-stats.mozilla.com/report/index/bp-b6609934-fb49-4a26-bb83-dffcb2110919

(Note to Ben: you can get those by going to about:crashes)

This time it's not my code (yay!) but this ANGLE code:
http://hg.mozilla.org/mozilla-central/file/5319b0100025/gfx/angle/src/libGLESv2/Texture.cpp#l1101

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Shortly before this crash there is a couple of NS_ASSERTION failures about a nsTArray being accessed out of bounds. If only NS_ASSERTION were aborting by default, this bug would have been faster to debug (it wouldn't have looked like a bug in ANGLE, I wouldn't have rebooted to Windows, etc).

Call stack to the assertion failure obtained with XPCOM_DEBUG_BREAK=break:

Program received signal SIGTRAP, Trace/breakpoint trap.
RealBreak () at /home/bjacob/mozilla-inbound/xpcom/base/nsDebugImpl.cpp:422
422     }
(gdb) bt
#0  RealBreak () at /home/bjacob/mozilla-inbound/xpcom/base/nsDebugImpl.cpp:422
#1  0x00007ffff54e420b in Break (
    aMsg=0x7fffffff7d90 "###!!! ASSERTION: invalid array index: 'i < Length()', file ../../../dist/include/nsTArray.h, line 496") at /home/bjacob/mozilla-inbound/xpcom/base/nsDebugImpl.cpp:513
#2  0x00007ffff54e41d2 in NS_DebugBreak_P (aSeverity=1, 
    aStr=0x7ffff5e983fe "invalid array index", aExpr=0x7ffff5e983f1 "i < Length()", 
    aFile=0x7ffff5e98328 "../../../dist/include/nsTArray.h", aLine=496)
    at /home/bjacob/mozilla-inbound/xpcom/base/nsDebugImpl.cpp:380
#3  0x00007ffff45e9a1a in nsTArray<mozilla::WebGLTexture::ImageInfo, nsTArrayDefaultAllocator>::ElementAt (this=0xd09698, i=591) at ../../../dist/include/nsTArray.h:496
#4  0x00007ffff45df87e in mozilla::WebGLTexture::ImageInfoAt (this=0xd09640, level=98, face=3)
    at /home/bjacob/mozilla-inbound/content/canvas/src/WebGLContext.h:999
#5  0x00007ffff45f5f7b in mozilla::WebGLContext::CopyTexImage2D (this=0xd09190, target=34072, 
    level=98, internalformat=6409, x=0, y=0, width=256, height=256, border=0)
    at /home/bjacob/mozilla-inbound/content/canvas/src/WebGLContextGL.cpp:892
#6  0x00007ffff4e045c3 in nsIDOMWebGLRenderingContext_CopyTexImage2D (cx=0x1420820, argc=8, 
    vp=0x7fffe4300190)
    at /home/bjacob/build/inbound/js/src/xpconnect/src/dom_quickstubs.cpp:29863
#7  0x00007ffff59a8bd1 in js::CallJSNative (cx=0x1420820, 
    native=0x7ffff4e0433d <nsIDOMWebGLRenderingContext_CopyTexImage2D(JSContext*, uintN, jsval*)>, args=...) at /home/bjacob/mozilla-inbound/js/src/jscntxtinlines.h:300
#8  0x00007ffff598196f in js::InvokeKernel (cx=0x1420820, argsRef=..., 
    construct=js::NO_CONSTRUCT) at /home/bjacob/mozilla-inbound/js/src/jsinterp.cpp:660
#9  0x00007ffff5996c2c in js::Interpret (cx=0x1420820, entryFrame=0x7fffe43000a0, 
    interpMode=js::JSINTERP_NORMAL) at /home/bjacob/mozilla-inbound/js/src/jsinterp.cpp:4040
#10 0x00007ffff59816e9 in js::RunScript (cx=0x1420820, script=0x7fffe412c740, fp=0x7fffe43000a0)
    at /home/bjacob/mozilla-inbound/js/src/jsinterp.cpp:614
#11 0x00007ffff5981a59 in js::InvokeKernel (cx=0x1420820, argsRef=..., 
    construct=js::NO_CONSTRUCT) at /home/bjacob/mozilla-inbound/js/src/jsinterp.cpp:678
#12 0x00007ffff58d7563 in js::Invoke (cx=0x1420820, args=..., construct=js::NO_CONSTRUCT)
    at /home/bjacob/mozilla-inbound/js/src/jsinterp.h:168
#13 0x00007ffff5981c1a in js::Invoke (cx=0x1420820, thisv=..., fval=..., argc=1,

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: check that image info exists before addressing it

Meh, I'm really stupid. Was addressing ImageInfoAt(level,face) in the texture without first checking that image info for this level and face existed. I scanned all of WebGLContextGL.cpp for occurences of this bug and found more than one. Some (like the one you ran into in Firefox 6) probably date back to Firefox 4 while some have been introduced in bug 665070.

This patch adds HasImageInfoAt checks everywhere we call ImageInfoAt.

Note that SetImageInfo doesn't need a similar check, as it takes care to grow the ImageInfo array as needed.

The fact that I didn't get a crash on Linux means that this could be used to scribble the heap, but with enough constraints that this shouldn't be exploitable. The heap can be scribbled with an array of

struct {
        int32 mWidth, mHeight;
        uint32 mFormat, mType;
        PRBool mIsDefined;
};

where mWidth and mHeight can be integers between 0 and the max texture size (at most 16k), mFormat and mType can be symbolic values chosen among a few possibilities below 64k, and mIsDefined can only be 1 if any other value is nonzero, and is 0 otherwise.

In other words, using 1 character to represent a 16-bit word, with the following legend,
   0    represents a word that's blocked to 0
   .    represents a word that can only take a few symbolic values
   #    represents a word that can take almost any value,

the heap can be scribbled with the following pattern:

   0#0#0.0.0.

Because of the very constrained pattern, and the fact that content can't decide or know the address where it's scribbling, I would be surprised if this could be exploited, but I don't know anything about security.

Comment 6

[Benoit Jacob [:bjacob] (mostly away)]

This bug has very rarely intermittently hit by the WebGL conformance tests, see bug 687149, but I could never reproduce it myself using conformance tests.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Any chance we can address this in an way that makes it so that things don't go bad if we miss a check?

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

To that effect, we could implement an always-enabled (even in release builds) assert in ImageInfoAt. This way, in the worst case we'd have a plain abort, not a security flaw. The problem is of course the overhead, as we do iterate over images in textures, but I suppose that it's not that performance critical.

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

Ping. Hope you didn't get the wrong impression that I didn't need a review anymore :-)

We definitely need the above patch as it's the only real fix; then if you want we can also do comment 8 as an additional precaution.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 561104 [details] [diff] [review]
check that image info exists before addressing it

Sorry, I forgot. Let's also do 8 as a precaution.

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/mozilla-central/rev/412d0be8158a

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: protect ImageInfoAt (paranoid precaution)

Comment 13

[Johnathan Nightingale [:johnath]]

Benoit, this is nominated without rationale or risk assessment.

Comment 14

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Johnathan Nightingale [:johnath] from comment #13)
> Benoit, this is nominated without rationale or risk assessment.

Risk assessment is in comment 4.

The risk is heap scribbling (invalid write access to heap) however the following factors limit the severity of this vulnerability:
  * scribbling is constrained to a restrictive pattern, so it would be very hard or impossible to scribble any code execution payload.
  * the write location is not decided by the attacker.

See comment 4 for details.

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

Oops -- misunderstood "risk assessment" as "severity assessment".

The risk of taking this patch is very low. This patch is basically just adding HasImageInfo(level,face) checks before dereferencing ImageInfoAt(level,face).

Comment 16

[LegNeato]

Comment on attachment 561104 [details] [diff] [review]
check that image info exists before addressing it

a=LegNeato. Please land this today on releases/mozilla-aurora

Comment 17

[Benoit Jacob [:bjacob] (mostly away)]

I forgot to land this on Aurora at the time (for Firefox 8), so this has just been pulled during the Central->Aurora merge meaning it's only fixed in Firefox 9.

Comment 18

[[e-mail] [:onecyrenus]]

have been trying to repro this in the browser by loading ff_webgl2.html several times, but wasn't able to hit this scenario. 

How shoudl i attempt to repro this ? Can it be repro'ed ? Given the contents of the e-mail above it might be quite difficult to get a crash scenario.

Comment 19

[[e-mail] [:onecyrenus]]

giving a try on my windows 7 machine at Home. 

Currently on Firefox 7.01 Mac OSX 10.6.8

Comment 20

[u279076]

This should be reproducible on Windows 7, Firefox 8 or earlier, with an NVidia GPU.

Comment 21

[[e-mail] [:onecyrenus]]

Was able to verify that this crash occurs windows 7 ff 8.01 / 10.0a2, 9.0b6. 
But I cannot find the scenario on Windows 7 where it does not crash. 
Version of Chrome is not crashing, btw. 

I have a GeForce 7300GS nvidia card.

Comment 22

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to [e-mail] from comment #21)
> Was able to verify that this crash occurs windows 7 ff 8.01 / 10.0a2, 9.0b6.

Really!? This was supposed to be fixed in 9.0b6 and 10.0a2.

Can you get us crash links from about:crashes?

Comment 23

[Daniel Veditz [:dveditz]]

crash in Fx8: bp-e5cebdff-4fa4-43a0-8425-181a22111219
crash in Fx9: bp-72ee64b6-f1d9-46c6-bfe3-d1ba12111219

definitely not fixed in 9.0.  Didn't see a crash in Nightly on the same machine, haven't yet tried Fx 10.

Comment 24

[Daniel Veditz [:dveditz]]

I do not crash in Aurora (Firefox 10.0a2). Thanks for catching this dclarke, we were about to release the advisory for this one.

Comment 25

[Benoit Jacob [:bjacob] (mostly away)]

The present patch was landed on mozilla-beta, changeset 412d0be8158a.

The 2 crash links in comment 23 are bug 695076, which is an unrelated ANGLE bug, should be fixed in Firefox >= 10.

I suggest undoing the tracking- flags changes of comment 23 and comment 24, and consider the present bug fixed in Firefox 9.

Comment 26

[Benoit Jacob [:bjacob] (mostly away)]

OK, Daniel rightly pointed out that the stack in comment 23 is the same as the one I linked in comment 2...

here's what happened. The bug is twofold. One part, which has been dealt with here and is fixed in Firefox 9, was a bug in our WebGL impl, failing to guard against out of bounds access in an array. The other part, bug 695076, was a bug in ANGLE, fixed in r885, the fix is imported in Firefox >= 10.

I agree with Daniel now that we can't disclose this bug until the Firefox 10 release.

Comment 27

[Daniel Veditz [:dveditz]]

Lowering the security rating based on the bug that was fixed here described in comment 4, and the [sg:critical] crash in gl::Texture::createSurface seen in this bug's testcase will be fixed in Fx10 by bug 695076.

Since we don't have a testcase that demonstrates just this bug we won't easily be able to have QA verify it.

Comment 28

[[e-mail] [:onecyrenus]]

Verified this is working in FF10.0a2 (2011-12-19) Windows 7 Vista, was not working in FF10.0a2 (2011-12-16)