665934 - (CVE-2011-2987) GrowAtomTable() return code not checked, crash due to ANGLE

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Daniel Veditz [:dveditz]]

Michael Jordon of Context IS sent this bug to us, based on testing on a WinXP SP3 machine and Firefox 4.0.1 (I believe, based on the other bug). He writes:

Comment 1

[Daniel Veditz [:dveditz]]

(comment 0 continued...):

This is really a bug in the ANGLE library, but as it isn't clear who you report security issues to with that so I am giving it to you as you include the vulnerable code. The file gfx/angle/src/compiler/preprocessor/atom.c has a couple of issues which could be used to cause a heap overflow. The function GrowAtomTable on line 320 has an incorrect assignment on line 338 (I am sure it should be atable->arev = newrev) exploiting that one would be difficult, although if you could you would alias atom allocations and also overflow the arev buffer (because that is never changed). 

Of course the real issue with this is the places which call GrowAtomTable don't check the return code (except in init and only then to check if the amap pointer is NULL). Therefore if you can get the allocations to fail then any new atoms added to the hash will start to overwrite the end of the arrays causing a heap overflow, the values are somewhat under the attacks control because it depends on the current atom number. 

The poc contains three files, index.html which hosts two copies of eatmemory.html. Run this first, this just allocates alot of memory (on my XP SP3 machine, around 1.3GB). When the page shows the "Wait" alert open a second tab and load the fuzzshader.html file. This builds a shader with alot of #define pre-processor lines, I then get the crash in the pre-processor code as shown in the including stack trace and crash dump. I am not sure if it is exactly where I would expect it to be, but it is worth pointing out that just running fuzzshader.html does not crash, therefore it seems almost likely it is exploiting the allocation issue. And based on analysis of the code I believe it is an problem which needs fixing.

Comment 2

[Daniel Veditz [:dveditz]]

Attachment: PoC. load index.html, wait, in a new tab open fuzzshader.html

Testcase sounds somewhat dependent on how much memory your machine has. Might have to play with eatmemory.html or limit a VM's available memory.

Comment 4

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

Forwarded to ANGLE:
http://code.google.com/p/angleproject/issues/detail?id=173

Comment 5

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

Seems that on linux, |cgroup| could be used to limit available memory to reproduce the crash.
http://jlebar.com/2011/6/15/Limiting_the_amount_of_RAM_a_program_can_use.html

Comment 6

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

Attachment: fix bug: arev was meant instead of amap

This is from the original report: "I am sure it should be atable->arev = newrev"  (by Michael Jordon). Looking at the surrounding code and the explanatory comment in struct AtomTable_Rec, that makes sense.

Comment 7

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

Notice that the bug here only happens when malloc() fails to allocate memory. This page:
https://developer.mozilla.org/en/Infallible_memory_allocation

Says that malloc() "will become infallible in the relatively near future". Has this already happened in mozilla-central?

Comment 8

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

(There also is a path with realloc(), don't know its status either).

Comment 9

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #7)
> Notice that the bug here only happens when malloc() fails to allocate
> memory. This page:
> https://developer.mozilla.org/en/Infallible_memory_allocation
> 
> Says that malloc() "will become infallible in the relatively near future".
> Has this already happened in mozilla-central?

I believe so yes.

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #7)
> Notice that the bug here only happens when malloc() fails to allocate
> memory. This page:
> https://developer.mozilla.org/en/Infallible_memory_allocation
> 
> Says that malloc() "will become infallible in the relatively near future".
> Has this already happened in mozilla-central?

Not yet: malloc() is still fallible.

Comment 11

[daniel-bzmz]

Comment on attachment 541388 [details] [diff] [review]
fix bug: arev was meant instead of amap

Review of attachment 541388 [details] [diff] [review]:
-----------------------------------------------------------------

Seems like the right thing to do.  Go ahead and commit to ANGLE repository if you need to.   We'll continue to investigate other fixes related to GrowAtomTable return values not being checked.

Comment 12

[daniel-bzmz]

Above patch was committed to ANGLE repo as r699

Comment 13

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

Comment on attachment 541388 [details] [diff] [review]
fix bug: arev was meant instead of amap

Marking as r+: the patch has been taken upstream.

Comment 14

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

Landed (same as ANGLE r699):
http://hg.mozilla.org/mozilla-central/rev/51e18435b793

Comment 15

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

Landed on Aurora:
http://hg.mozilla.org/releases/mozilla-aurora/rev/9eabe65eb25a

Comment 16

[Johnny Stenback (:jst)]

Does the fact that we took a patch for this on m-c and aurora mean that it's fixed on trunk and aurora, which means it got uplifted to beta?

Comment 17

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

Yes, unless there are more bugs here, which I don't know. I've not been able to reproduce the bug myself.

Comment 18

[Johnny Stenback (:jst)]

Ok, thanks. Would it make sense to close this bug out then? I'm suggesting we do so, and open new bugs as more stuff is found, if that's even the case. I'll leave closing this bug for trunk up to you tho, but I doubt we'll do any more than what we've done here for 6 and 7.

Comment 19

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

Sure, seems reasonable.

Comment 20

[u279076]

qa- as no QA fix verification needed

Comment 21

[Raymond Forbes[:rforbes]]

rforbes-bugspam-for-setting-that-bounty-flag-20130719