Closed Bug 665934 (CVE-2011-2987) Opened 8 years ago Closed 8 years ago

GrowAtomTable() return code not checked, crash due to ANGLE

Categories

(Core :: Canvas: WebGL, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox5 - wontfix
firefox6 + fixed
firefox7 + fixed
status2.0 --- wontfix
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: dveditz, Assigned: bjacob)

References

Details

(Whiteboard: [sg:critical?][qa-])

Attachments

(2 files)

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 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.
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.
Attachment #540781 - Attachment mime type: application/zip → application/java-archive
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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [sg:critical?]
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.
Attachment #541388 - Flags: review?(daniel-bzmz)
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?
(There also is a path with realloc(), don't know its status either).
(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.
(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 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.
Assignee: nobody → bjacob
status2.0: --- → wontfix
Above patch was committed to ANGLE repo as r699
Comment on attachment 541388 [details] [diff] [review]
fix bug: arev was meant instead of amap

Marking as r+: the patch has been taken upstream.
Attachment #541388 - Flags: review?(daniel-bzmz) → review+
Attachment #541388 - Flags: approval-mozilla-aurora?
Attachment #541388 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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?
Yes, unless there are more bugs here, which I don't know. I've not been able to reproduce the bug myself.
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.
Sure, seems reasonable.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Alias: CVE-2011-2987
Blocks: 672761
Depends on: CVE-2011-3002
qa- as no QA fix verification needed
Whiteboard: [sg:critical?] → [sg:critical?][qa-]
rforbes-bugspam-for-setting-that-bounty-flag-20130719
Flags: sec-bounty+
You need to log in before you can comment on or make changes to this bug.