Closed Bug 592199 Opened 11 years ago Closed 11 years ago
Crash [@ js
_fun _apply ] with embedded Cloud Made / Open Street Map
STEPS TO REPRODUCE: 1. Load URL: http://pugio.net/2009/07/openstreetmap-geolocation-in-firefox-35.html 2. Click and pan around the embedded map. e.g. this reproduces it pretty reliably: Click the center of europe, drag it all the way to the left. Wait a second or two. RESULTS: Crash [@ js_fun_apply ] I've reproduced in m-c nightly: Mozilla/5.0 (X11; Linux i686; rv:2.0b5pre) Gecko/20100830 Firefox/4.0b5pre as well as 4.0b4: Mozilla/5.0 (X11; Linux i686; rv:2.0b4) Gecko/20100818 Firefox/4.0b4 However, I couldn't reproduce in a Firefox 3.6 nightly build: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:18.104.22.168pre) Gecko/20100708 Namoroka/3.6.8pre
Here are some crash report IDs: bp-05f49cda-5973-44b0-9564-2a7332100831 bp-893e53b1-2c9e-4e9b-9208-331d82100831 bp-b035f1e4-4295-4b74-80d8-60df82100831 bp-0e8dda67-9b83-4f9e-aa3e-cf4d52100831 Backtrace: 0 libxul.so js_fun_apply js/src/jsfun.cpp:2409 1 libxul.so js::Interpret js/src/jsinterp.cpp:4696 2 libxul.so js::InvokeCommon<JSBool > js/src/jsinterp.cpp:577 3 libxul.so js::Invoke js/src/jsinterp.cpp:696 4 libxul.so js::InternalInvoke js/src/jsinterp.cpp:736 5 libxul.so JS_CallFunctionValue js/src/jsinterp.h:651 6 libxul.so nsJSContext::CallEventHandler dom/base/nsJSEnvironment.cpp:2248 7 libxul.so nsGlobalWindow::RunTimeout dom/base/nsGlobalWindow.cpp:8585 8 libxul.so nsGlobalWindow::TimerCallback dom/base/nsGlobalWindow.cpp:8930 9 libxul.so nsTimerImpl::Fire xpcom/threads/nsTimerImpl.cpp:425 10 libxul.so nsTimerEvent::Run xpcom/threads/nsTimerImpl.cpp:517 11 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:547 12 libxul.so NS_ProcessNextEvent_P nsThreadUtils.cpp:250 13 libxul.so mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:110 14 libxul.so MessageLoop::RunInternal ipc/chromium/src/base/message_loop.cc:219 15 libxul.so MessageLoop::Run ipc/chromium/src/base/message_loop.cc:202 16 libxul.so nsBaseAppShell::Run widget/src/xpwidgets/nsBaseAppShell.cpp:175 17 libxul.so nsAppStartup::Run toolkit/components/startup/src/nsAppStartup.cpp:191 18 @0xb75a3f4f 19 libxul.so XRE_main toolkit/xre/nsAppRunner.cpp:3662 20 firefox-bin main browser/app/nsBrowserApp.cpp:158 21 libc-2.11.1.so libc-2.11.1.so@0x16bd5 22 firefox-bin firefox-bin@0x1390 23 firefox-bin Output browser/app/nsBrowserApp.cpp:77 24 @0x3 In my crash reports, the Crash Address is always 0xa162. I'm not sure what the significance of that value is. Tentatively marking security-sensitive.
Nightly regression range: 20100806031556 26ee1b556bd9 20100807025746 81c119fb86c7 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=26ee1b556bd9&tochange=81c119fb86c7 That range includes a fairly large tracemonkey merge (49 csets).
Summary: Crash [@ js_fun_apply ] → Crash [@ js_fun_apply ] with openstreetmap demo
Here's a somewhat reduced testcase. ("somewhat" because it references an external script from cloudmade, which is some sort of openstreetmap-wrapping service)
Summary: Crash [@ js_fun_apply ] with openstreetmap demo → Crash [@ js_fun_apply ] with embedded CloudMade / OpenStreetMap
I just confirmed that this affects the latest tracemonkey build, too: Mozilla/5.0 (X11; Linux i686; rv:2.0b5pre) Gecko/20100830 Firefox/4.0b5pre Crash report from there is: bp-0610a262-e03e-4f60-b544-b96c12100831
Thanks a lot! I think this is a reproducible testcase for the #2 crasher for FF4.0b4 (bug 586004).
Hooray! You're very welcome. FWIW, I tested tracemonkey nightlies, and the regression range there is actually the same date-range as on m-c: 20100806033340 c15ed7c71e27 20100807032731 bdd944b09a53 http://hg.mozilla.org/tracemonkey/pushloghtml?fromchange=c15ed7c71e27&tochange=bdd944b09a53
It looks like this is all just caused by an over-eager optimization: putActivationObjects doesn't call js_PutArguments on the arguments object created on trace if argc == 0, even though the object may be aliased elsewhere. This bug goes way back (http://hg.mozilla.org/tracemonkey/rev/78d111c4ab84), although perhaps we didn't trace enough back then to let it bite us. Now sleep.
Attachment #470739 - Attachment description: maybe fix → fixes crash
did we confirm the security sensitivity of this bug? if it is sg:crit? should we mark bug 586004 security sensitive as well, or just get this fixed soon and move on? should be b5 blocker if respins are done based on high volume crasher that might also introduce a vulnerability. definitely should try to pick this up for b6.
Oops, I forgot to qref before posting that last patch. I don't see how this crash is exploitable: since the bug only happens for argc == 0, code shouldn't be doing anything with the trashed Arguments private-ptr. Most sites guard on argc > 0 before even loading fp->argv, so this bug would have been innocuous, but js_fun_apply loads fp->argv in all cases (for a memset(fp->argv, ..., argc)).
Is this really exploitable if it involves the evil fixed JS_ARGUMENTS_OBJECT_ON_TRACE address? I think that hack is worth eliminating -- followup bug? /be
> I think that hack is worth eliminating -- followup bug? I don't think its so much a hack as a debugging tool: argc == 0 case aside; if you are accessing the private-ptr of an Arguments object on trace, something is wrong. OTOH, I think bug 590871 will make the whole on/off trace distinction go away by allowing activation objects to store the JSStackFrame* early.
Marking this blocking beta5 since this is likely to fix the #2 topcrash for beta4. David, can we get this reviewed? And sounds like this should not be a security bug per comment #9, any objections to me opening this up?
blocking2.0: ? → beta5+
Comment on attachment 470812 [details] [diff] [review] fix A trace-test test case would be nice, but I know we need this patch urgently, so add it if you can easily. Otherwise r+. And thanks for the description of the problem.
Attachment #470812 - Flags: review?(dmandelin) → review+
(In reply to comment #10) > Is this really exploitable if it involves the evil fixed > JS_ARGUMENTS_OBJECT_ON_TRACE address? Ah, interesting - FWIW, the crashes are at addresses slightly offset from that value, and the amount of offset seems to vary (despite what I said at the end of comment 1). That is, JS_ARGUMENT_OBJECT_ON_TRACE is #defined as 0xa126 vs. bp-0e8dda67-9b83-4f9e-aa3e-cf4d52100831 is at 0xa146 vs. bp-893e53b1-2c9e-4e9b-9208-331d82100831 is at 0xa162 (In reply to comment #12) > And sounds like this should not be a security bug per comment #9, any > objections to me opening this up? None here, given comment 9. (I originally hid it just to be on the safe side.)
(In reply to comment #14) > Ah, interesting - FWIW, the crashes are at addresses slightly offset from that > value, and the amount of offset seems to vary (despite what I said at the end > of comment 1). > > That is, JS_ARGUMENT_OBJECT_ON_TRACE is #defined as 0xa126 > vs. bp-0e8dda67-9b83-4f9e-aa3e-cf4d52100831 is at 0xa146 > vs. bp-893e53b1-2c9e-4e9b-9208-331d82100831 is at 0xa162 Those changes reflect the ever-changing offsetof(JSStackFrame, argv): http://hg.mozilla.org/projects/electrolysis/annotate/7652c4f2d60d/js/src/jsinterp.h#l86 http://hg.mozilla.org/mozilla-central/annotate/9d6448b6a677/js/src/jsinterp.h#l84
Is this a regression from bug 585231? This seems the most relevant in the regression range in comment 2 http://hg.mozilla.org/mozilla-central/rev/d695985dc913 Would like to pin down the regressing bug/changeset so we can link the bugs in case we ever want to take the regressing bug on branches. Seems unlikely if it really is bug 585231, but if it isn't it might be some other security-related fix.
http://hg.mozilla.org/tracemonkey/rev/9f2641871ce8 (In reply to comment #16) The problem originated in bug 509599.
(In reply to comment #12) > Marking this blocking beta5 since this is likely to fix the #2 topcrash for > beta4. David, can we get this reviewed? Uhhhh, beta5 has been tagged and built already. Are you proposing that we respin for this bug? I don't think that's necessary, really, and would rather save it for beta6. Let me know ASAP.
if we respin lets pick this up, otherwise land for b6
I was informed that we're respinning for some focus related issues, and if we are, we should make sure this gets in too, if not, we'll do beta6 here.
(In reply to comment #17) > The problem originated in bug 509599. Which would imply 1.9.2 needs the fix, although dholbert couldn't reproduce the problem on that branch.
See also comment 7. (It sounds like we've had this foot-gun for a while, but we only recently disabled the safety button.) Luke, any thoughts on whether this patch would be useful/appropriate/risky on 1.9.2?
http://hg.mozilla.org/mozilla-central/rev/c38ea6cd0ea2 I tried to exercise the bug on 1.9.2 but was unable to: the 1.9.2 version of js_fun_apply seems to guard on the arg count before touching fp->argv hence, while Arguments objects are escaping trace when argc == 0, no code (AFAICS) seems to care.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.