Bugzilla@Mozilla – Bug 615657
Buffer overflow/Memory corruption: cg->upvarList.count <= cg->upvarMap.length
Last modified: 2011-03-29 19:23:46 PDT
Summon comment box
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.12) Gecko/20101027 Ubuntu/10.10 (maverick) Firefox/3.6.12 Build Identifier: The following problem affects all Spidermonkey versions I tested (1.9.2 and 2.0) and hence Firefox 3.5.x and Firefox 4. Under test were mozilla-2.0-297086a0fb61 and mozilla-1-9-2-053f07027a38: There is a serious problem with the code that copies upvarMap.vector in jsscript.cpp. Using a combination of inner/outer functions with eval, we were able to trigger the following assertion (mozilla-2.0-297086a0fb61): Assertion failure: cg->upvarList.count <= cg->upvarMap.length, at jsscript.cpp:1224 The respective code looks as follows: if (cg->upvarList.count != 0) { JS_ASSERT(cg->upvarList.count <= cg->upvarMap.length); memcpy(script->upvars()->vector, cg->upvarMap.vector, cg->upvarList.count * sizeof(uint32)); cg->upvarList.clear(); cx->free(cg->upvarMap.vector); cg->upvarMap.vector = NULL; } In an optimized non-debug build, the memcpy simply either overruns the available space in script->upvars()->vector, or reads beyond cg->upvarMap.vector or both. The attacker can control cg->upvarList.count arbitrarily here. We suspect the issue to be exploitable although it shouldn't be too easy (the attacker can hardly control what is actually written, only the amount). I'll attach test.js to reproduce the problem and some valgrind logs showing different behavior depending on how large the upvar list gets. Using 24 additional upvars in the script triggers the assertion, using 32 crashes straight away without even asserting (I suspect two different problems here). Reproducible: Always Steps to Reproduce: 1. Run test.js 2. Observe crash/assertion Actual Results: Crash/Assertion Expected Results: Normal run.
Created attachment 494087 [details] Script to reproduce described crash This script demonstrates the crash.
Created attachment 494089 [details] Valgrind log for 32bit with 24 upVars on 1.9.2 Valgrind log for 32bit with 24 upVars on 1.9.2
Created attachment 494090 [details] Valgrind log for 32bit with 32 upVars on 1.9.2 Valgrind log for 32bit with 32 upVars on 1.9.2
Created attachment 494092 [details] Valgrind log for 32bit with 32 upVars on 2.0 Valgrind log for 32bit with 32 upVars on 2.0
Created attachment 494093 [details] Valgrind log for 64bit with 32 upVars on 2.0 Valgrind log for 64bit with 32 upVars on 2.0
Brendan, this looks like upvar code, so handing to you to start with. Sounds like a critical bug to me, but please let me know if you disagree.
Created attachment 496209 [details] testcase (click to crash) Incorporated the PoC script into a loadable page, and increased the upvars to ensure a crash. For some reason I hit the unresponsive script dialog twice just executing the loop 1000 times, which is odd because it doesn't do much except grow some strings. I've seen PoC's using tons more memory and loops that don't hit that warning. I think the following crash happened when I closed the page after running the testcase, not directly when I clicked it. bp-104c40b5-7653-4d6e-ac52-ff31e2101208
Created attachment 496287 [details] [review] fix I hate MakeUpvarForEval... (which means I hate SunSpider, which wants it -- but you knew that). /be
Comment on attachment 496287 [details] [review] fix It's cool how the assert basically documents what the other part of the patch is for. Made it super-easy to review.
http://hg.mozilla.org/tracemonkey/rev/fb3b0fd656bf /be
http://hg.mozilla.org/mozilla-central/rev/fb3b0fd656bf
Does this bug affect FF3.5 or did upvars come in with 1.9.2?
This affects 3.5 too. /be
I don't know how to prove this can't be exploited. With enough work, it probably could be, although it is not as easy to exploit as sg:criticals involving free memory reads during virtual call instruction sequences. /be
Created attachment 505586 [details] [review] 191 patch Pedestrian port to 191, nothing difficult to need re-review.
Created attachment 505588 [details] [review] 192 patch ...also a trivial port.
Comment on attachment 505586 [details] [review] 191 patch Approved for 1.9.2.14 and 1.9.1.17, a=dveditz for release-drivers
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/83f326171713 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/28ce03bbd37c
(In reply to comment #9) > Created attachment 496209 [details] > testcase (click to crash) > > Incorporated the PoC script into a loadable page, and increased the upvars to > ensure a crash. For some reason I hit the unresponsive script dialog twice > just executing the loop 1000 times, which is odd because it doesn't do much > except grow some strings. I've seen PoC's using tons more memory and loops that > don't hit that warning. Verified for 1.9.2 using this testcase. Crashes in 1.9.2.13 and verified fixed in 1.9.14 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.14) Gecko/20110121 Firefox/3.6.14 ( .NET CLR 3.5.30729)). Same with 1.9.1.16 and verified fixed in 1.9.17 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.17) Gecko/20110121 Firefox/3.5.17 ( .NET CLR 3.5.30729)).