Bugzilla@Mozilla – Bug 502832
TM: Crash [@ memcpy]
Last modified: 2010-03-12 21:37:38 PST
Summon comment box
see bug 502449 comment 3 and bug 502672. Filing new bug and making sensitive due to stacks etc. start browser, load http://bonds.finam.ru/issue/info/ , close browser. on windows only afaict. on win2k3/1.9.1 build from this weekend with vc 8 express I got. Exploitability Classification: EXPLOITABLE Recommended Bug Title: Exploitable - User Mode Write AV starting at MSVCR80D!memcpy+0x000000000000005a (Hash=0x1f2a4941.0x39393718) User mode write access violations that are not near NULL are exploitable. ChildEBP RetAddr 0012c558 004e80c2 js3250!js_Invoke+0x95c 0012c5a4 0050aeea js3250!js_fun_apply+0x2c2 0012ccd4 004f70cc js3250!js_Interpret+0x1179a 0012cdb0 004e80c2 js3250!js_Invoke+0x95c 0012cdfc 0050aeea js3250!js_fun_apply+0x2c2 0012d52c 004f70cc js3250!js_Interpret+0x1179a 0012d608 004e80c2 js3250!js_Invoke+0x95c 0012d654 0050aeea js3250!js_fun_apply+0x2c2 0012dd84 004f70cc js3250!js_Interpret+0x1179a 0012de60 012f6c57 js3250!js_Invoke+0x95c with fresh builds on windows xp and vc 8 pro and 1.9.1/1.9.2 and tracemonkey > msvcr80d.dll!memcpy(unsigned char * dst=0x05dc3bdc, unsigned char * src=0x0012b380, unsigned long count=4201) Line 188 Asm js3250.dll!TraceRecorder::snapshot(ExitType exitType=TIMEOUT_EXIT) Line 2948 + 0x1a bytes C++ js3250.dll!TraceRecorder::TraceRecorder(JSContext * cx=0x04f99268, VMSideExit * _anchor=0x00000000, nanojit::Fragment * _fragment=0x06a7f958, TreeInfo * ti=0x06a8b4e8, unsigned int stackSlots=4201, unsigned int ngslots=0, JSTraceType_ * typeMap=0x06886068, VMSideExit * innermostNestedGuard=0x00000000, unsigned char * outer=0x00000000, unsigned long outerArgc=0) Line 1637 + 0xa bytes C++ js3250.dll!js_StartRecorder(JSContext * cx=0x04f99268, VMSideExit * anchor=0x00000000, nanojit::Fragment * f=0x06a7f958, TreeInfo * ti=0x06a8b4e8, unsigned int stackSlots=4201, unsigned int ngslots=0, JSTraceType_ * typeMap=0x06886068, VMSideExit * expectedInnerExit=0x00000000, unsigned char * outer=0x00000000, unsigned long outerArgc=0) Line 4140 + 0x4b bytes C++ js3250.dll!js_RecordTree(JSContext * cx=0x04f99268, JSTraceMonitor * tm=0x00d264c8, nanojit::Fragment * f=0x06a7f958, unsigned char * outer=0x00000000, unsigned long outerArgc=0, JSObject * globalObj=0x061f3be0, unsigned long globalShape=61629, Queue<unsigned short> * globalSlots=0x00d30758, unsigned long argc=4196) Line 4440 + 0x3c bytes C++ js3250.dll!js_MonitorLoopEdge(JSContext * cx=0x04f99268, unsigned int & inlineCallCount=0) Line 5517 + 0x28 bytes C++ js3250.dll!js_Interpret(JSContext * cx=0x04f99268) Line 3940 + 0x668 bytes C++ js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=4196, int * vp=0x0699c044, unsigned int flags=0) Line 1397 + 0x9 bytes C++ js3250.dll!js_fun_apply(JSContext * cx=0x04f99268, unsigned int argc=4196, int * vp=0x07e71514) Line 2080 + 0x13 bytes C++ js3250.dll!js_Interpret(JSContext * cx=0x04f99268) Line 5210 + 0x17 bytes C++ js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e71118, unsigned int flags=0) Line 1397 + 0x9 bytes C++ js3250.dll!js_fun_apply(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e710e0) Line 2080 + 0x13 bytes C++ js3250.dll!js_Interpret(JSContext * cx=0x04f99268) Line 5210 + 0x17 bytes C++ js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e710d4, unsigned int flags=0) Line 1397 + 0x9 bytes C++ js3250.dll!js_fun_apply(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e7109c) Line 2080 + 0x13 bytes C++ js3250.dll!js_Interpret(JSContext * cx=0x04f99268) Line 5210 + 0x17 bytes C++ js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e7106c, unsigned int flags=0) Line 1397 + 0x9 bytes C++ js3250.dll!js_fun_apply(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e71034) Line 2080 + 0x13 bytes C++ js3250.dll!js_Interpret(JSContext * cx=0x04f99268) Line 5210 + 0x17 bytes C++ js3250.dll!js_Invoke(JSContext * cx=0x04f99268, unsigned int argc=1, int * vp=0x07e71028, unsigned int flags=0) Line 1397 + 0x9 bytes C++
!exploitable says this second stack is exploitable as well.
Bob, For what it's worth, I was able to reproduce this on Linux (Mozilla/5.0 X11; U; Linux; i686; en-US; rv:1.9.1) Gecko/20090624 Firefox 3.5) but not latest 1.9.2 trunk. So far not able to reproduce on Mac.
http://bonds.finam.ru/trades/today/ also.
I was able to repro this once on http://bonds.finam.ru/trades/today/ on Ubuntu Linux with Fx 3.5 but I couldn't repro it again.
sayrer: Can we get an owner for this bug?
Created attachment 388497 [details] [review] v1 This one-line patch avoids crashing in memcpy. But the alloca call preceding this is just asking for crashes, too. Graydon, do you already have a patch somewhere that eliminates alloca calls in jstracer? If so, I think we should land it. If not, I can patch just this one call.
Created attachment 388526 [details] [review] v2 This version allocates that memory from cx->tempPool rather than alloca.
Any way to capture the crashing page and attach, or better, reduce first?
Comment on attachment 388526 [details] [review] v2 Fine by me. That's the general thing I was doing with alloca()-squashing anyways.
This is going to be an unpopular comment given yesterday but did you measure the performance impact?
(In reply to comment #10) > This is going to be an unpopular comment given yesterday but did you measure > the performance impact? bench.sh shows nothing significant, but it's rather noisy. I'll run SunSpider and v8 in a sec.
SunSpider shows no change. On v8, I get "not conclusive: might be *1.007x as slow*", but every test is slower by a few tenths of a percent.
http://hg.mozilla.org/tracemonkey/rev/e51fb83dbd64 I don't see how this crash could actually be exploited. Still, I'm not terribly comfortable having likely stack overruns in the product. Graydon had the fix for this bug three months ago and we did not land it. The sentiment at the time was that the actual crash (bug 484693) had been fixed, and Graydon's patch removing the other alloca calls was an unnecessary perf loss. Do we still feel that way? Do we have some reason to believe the remaining alloca calls are safe? I took a look, and I'm not prepared to vouch for them. I think the performance benefit of the remaining alloca calls is likely not worth the time it would take to check that they are safe. In other words, if we were to remove them (and I think we should), it would not be worth our time to write--and review with appropriate care--the patches to re-add them.
http://hg.mozilla.org/mozilla-central/rev/e51fb83dbd64
(In reply to comment #13) > http://hg.mozilla.org/tracemonkey/rev/e51fb83dbd64 > > I don't see how this crash could actually be exploited. Still, I'm not terribly > comfortable having likely stack overruns in the product. > > Graydon had the fix for this bug three months ago and we did not land it. The > sentiment at the time was that the actual crash (bug 484693) had been fixed, > and Graydon's patch removing the other alloca calls was an unnecessary perf > loss. > > Do we still feel that way? > > Do we have some reason to believe the remaining alloca calls are safe? I took a > look, and I'm not prepared to vouch for them. > > I think the performance benefit of the remaining alloca calls is likely not > worth the time it would take to check that they are safe. In other words, if we > were to remove them (and I think we should), it would not be worth our time to > write--and review with appropriate care--the patches to re-add them. FWIW, I stumbled upon another test URL on 3.5.1 EXPLOITABLE: Exploitable - User Mode Write AV starting at MSVCR80D!memcpy+0x000000000000005a (Hash=0x1f2a4941.0x11133665): http://www.ylighting.com/kitchendining-allitems-price.html: EXIT STATUS: NORMAL (119.310000 seconds) http://www.ylighting.com/kitchendining-allitems-price.html
(In reply to comment #13) > Do we still feel that way? Not I -- we shipped, we have a bit more time to be safer and fix any perf regressions. /be
(In reply to comment #16) > (In reply to comment #13) > > Do we still feel that way? > > Not I -- we shipped, we have a bit more time to be safer and fix any perf > regressions. > > /be I'm thus guessing that some version of the patch here can be ported in time for 1.9.1.2 then, since it's now baking on m-c (so far for a day or so)
Please, please, please get a reduced testcase in this bug before we lose the live site.
Created attachment 390151 [details] unreduced testcase.tar.bz2 this reproduced the stack.
(In reply to comment #19) > Created an attachment (id=390151) [details] > unreduced testcase.tar.bz2 > > this reproduced the stack. I'm getting a crash with 3.5.1 debug build on Linux - way to go, Bob! I'm on it with Lithium now.
(In reply to comment #20) > (In reply to comment #19) > > Created an attachment (id=390151) [details] [details] > > unreduced testcase.tar.bz2 > > > > this reproduced the stack. > > I'm getting a crash with 3.5.1 debug build on Linux - way to go, Bob! I'm on it > with Lithium now. CC'ing Jesse too.
Gary, Thanks. I've been trying to reduce it on WinXp and have been having a hard time. Did you get the same stack? We need to make sure the reduced test reproduces the exact stack.
Created attachment 390164 [details] Linux stack I must admit too, even with your testcase, it's highly unreliable - I need to execute a random set of actions on the webpage to get it to crash. Still working on it.
Created attachment 390190 [details] half-localized testcase off bc's This half-localized ~10k-liner crashes Linux reliably without random actions - I wrote in a setTimeout auto-reload to get it to crash. (Thanks gal)
(In reply to comment #24) > Created an attachment (id=390190) [details] > localized testcase off bc's Remember to have a local copy and change the path at the top "window.location" variable to reflect the path on your local copy. (I'm on Ubuntu 9.04 btw) I am unable to reproduce this crash on Mac 10.5 so far.
Created attachment 390205 [details] slightly more localized, in no way reduced I'm now tearing my hair out at localizing/reducing this - it's still not fully localized yet. Remember to have a local copy then change the window.location path at the top of the testcase to your location. (Any suggestions to help is greatly appreciated!)
Created attachment 390259 [details] slightly more localized and a wee bit reduced Someone else please feel free to take over - that's all for my day. :-(
*** Bug 506701 has been marked as a duplicate of this bug. ***
Can we get this fix into 3.5.2?
Comment on attachment 388526 [details] [review] v2 Sure. I have this sitting in a patch queue ready to push to mozilla-1.9.1 actually.
Comment on attachment 388526 [details] [review] v2 Approved for 1.9.1.2. a=ss for release-drivers. Please land and use the ".2-fixed" option of the status1.9.1 flag to indicate fixed status.
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/57c2310ac1d5
v 1.9.1 mac/win/linux
*** Bug 502672 has been marked as a duplicate of this bug. ***
*** Bug 503310 has been marked as a duplicate of this bug. ***