This crashes Firefox (tested with today's trunk on Mac) <body><script>document.body.compareDocumentPosition(<foo/>);</script></body> The crash appears to be exploitable, so I'm making this a security-sensitive bug.
Created attachment 211343 [details] stack trace Note: it hangs for a while before crashing, so it's hard to get the crash in a debug build :(
nsXPCWrappedJSClass::CallMethod is failing to protect its result jsval local from the JS GC. /be
Created attachment 211350 [details] [diff] [review] read and commence puking If you minus it, you own it. /be
Much sickness here. Besides the GC hazard (how many more must we endure?) in XPCWrappedJSClass:CallMethod, E4X sucks: it returns an empty XMLList for every property get of an unnamed child. So the <foo/> point tag has no QueryInterface child, no parentNode, etc. The empty XMLList is a JSObject, so XPConnect tries to wrap this result to make it look like an XPCOM object, etc. The JS_GetMethod API was added for XPConnect, which oddly uses char *, not jschar * or PRUnichar * string name to call. I didn't add a JS_GetUCMethod or (better) a JS_GetMethodById taking a jsid. So the attached patch hacks mightily. Not for checkin, but to point the way to a better fix. Should XPConnect even try to JS_GetMethod a getter (parentNode) instead of calling JS_GetProperty? That doesn't make sense. But JS_GetProperty is doomed to get an empty XMLList (typically -- could be non-empty if tha <foo/> tag were non-empty and had a child named QueryInterface or whatever). And wrapping an empty XMLList targeting the <foo/> object produces another list targeting the first list, and off we go to an infinite target-linked chain. Better to fail an attempt to do a property get or set on an E4X object wrapped by XPConnect. E4X really messes with JS's object model. /be
Every time I try the testcase in a debug windows build I crash trying to access 0xd5d5d5d5 -- is that some kind of a magic value we use internally? I'm also getting a different stack than Jesse (handling the start token, not the end token). I suppose it's a moot point since there's a patch, but including anyway > GetProperty Line 3977 + 0x9 xml_getProperty nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject nsXPCWrappedJSClass::GetRootJSObject nsXPCWrappedJS::GetNewOrUsed XPCConvert::JSObject2NativeInterface XPCConvert::JSData2Native nsXPCWrappedJSClass::CallMethod nsXPCWrappedJS::CallMethod PrepareAndDispatch SharedStub nsContentUtils::GetFirstDifferentAncestors nsContentUtils::ComparePositionWithAncestors nsNode3Tearoff::CompareDocumentPosition XPTC_InvokeByIndex XPCWrappedNative::CallMethod XPC_WN_CallMethod js_Invoke js_Interpret js_Execute JS_EvaluateUCScriptForPrincipals nsJSContext::EvaluateString nsScriptLoader::EvaluateScript nsScriptLoader::ProcessScriptElement nsHTMLScriptElement::MaybeProcessScript nsHTMLScriptElement::BindToTree nsGenericElement::AppendChildTo HTMLContentSink::ProcessSCRIPTTag HTMLContentSink::AddLeaf CNavDTD::AddLeaf CNavDTD::HandleScriptToken CNavDTD::OpenContainer CNavDTD::HandleDefaultStartToken CNavDTD::HandleStartToken CNavDTD::HandleToken CNavDTD::BuildModel nsParser::BuildModel nsParser::ResumeParse nsDocumentOpenInfo::OnDataAvailable nsDataChannel::OnDataAvailable nsInputStreamPump::OnStateTransfer nsInputStreamPump::OnInputStreamReady nsInputStreamReadyEvent::EventHandler PL_HandleEvent PL_ProcessPendingEvents nsEventQueueImpl::ProcessPendingEvents nsWindow::DispatchPendingEvents nsWindow::ProcessMessage nsWindow::WindowProc 77d48744 77d48826 77d489dd 77d48a20 nsAppShell::Run nsAppStartup::Run XRE_main main mainCRTStartup 7c816d4f 7c8399f3
I was using a current 1.8.0 branch build if it makes a difference
Er... AUTO_MARK_JSVAL takes a jsval, not a jsval*.... How'd that compile?
(In reply to comment #8) > Er... AUTO_MARK_JSVAL takes a jsval, not a jsval*.... How'd that compile? It takes either. Read xpcprivate.h. I want someone else to own this bug. I'm happy to help, but a bit overloaded lately (not just with CTO biz, but that doesn't help). /be
(In reply to comment #6) > Every time I try the testcase in a debug windows build I crash trying to access > 0xd5d5d5d5 See http://lxr.mozilla.org/mozilla/search?string=0xd5 -- yes, this is a DEBUG only tombstone over dead E4X data. The patch fixes this bug, but it needs massaging. /be
Comment on attachment 211350 [details] [diff] [review] read and commence puking With bug 327697 fixed, most of this isn't necessary any more. Brendan, do you want to just submit the GC hazard part of this fix?
Created attachment 213415 [details] [diff] [review] just the rooting fix
Created attachment 213416 [details] [diff] [review] just the rooting fix
Created attachment 213417 [details] [diff] [review] just the rooting fix Is this needed for aviary1.0.x/mozilla1.7.x? /be
Created attachment 213418 [details] [diff] [review] just the rooting fix Is this needed for aviary1.0.x/mozilla1.7.x? /be
Created attachment 213419 [details] [diff] [review] just the rooting fix Is this needed for aviary1.0.x/mozilla1.7.x? /be
Created attachment 213420 [details] [diff] [review] just the rooting fix Is this needed for aviary1.0.x/mozilla1.7.x? /be
Created attachment 213421 [details] [diff] [review] just the rooting fix Is this needed for aviary1.0.x/mozilla1.7.x? /be
Good grief! Bugzilla kept giving me Internal Server Error, so I kept retrying. I'll know better next time.... /be
<brendan> shouldn't give an error page if the mysql commit went through <justdave> yeah, it died trying to send the email <justdave> I'm trying to track down why... <justdave> found it <justdave> https://bugzilla.mozilla.org/show_bug.cgi?id=266147 <justdave> it's fixed in 2.20.1 <justdave> the patch fails miserably trying to apply it to our current install, and I get a ton of conflicts updating to the tip of the 2.20 branch :( <justdave> (on the test copy of bmo of course) <justdave> hmm, take that back, the patch doesn't fail quite so miserably as I thought... <justdave> the failed chunk is a change in a comment. :) <justdave> and the other was a one-line conflict with a local customization that was easy to merge * justdave applies the patch to production <justdave> done <brendan> thanks Thanks again for fixing b.m.o. /be
Comment on attachment 213421 [details] [diff] [review] just the rooting fix r=mrbkap, thanks.
Fixed on the trunk. /be
Comment on attachment 213421 [details] [diff] [review] just the rooting fix approved for 1.8.0 branch, a=dveditz for drivers
Fixed on the 1.8 branches. /be
Marking [rft-dl] (ready for testing in Firefox 188.8.131.52 release candidates)
testcase crashes Mac Firefox 184.108.40.206 build from 20060306 reopening
Tracy, does this crash on the trunk as well? If this only crashes on 220.127.116.11, then I don't think the bug should be reopened but that the fixed18.104.22.168 keyword should be removed.
okay, no crash on trunk. re-resolving fixed. leaving out fixed22.214.171.124 keyword
Brendan- can you look at this for 126.96.36.199. It is a blocker for that release and we need final bits ASAP! Thx!
FWIW, the test case also crashes on 188.8.131.52 Mac.
I'm suffering terrible wireless connectivity here at ETech. Suggest mrbkap investigate. Did I fail to land the patch on the 1.8.0 branch? Re: 184.108.40.206 crashes, that's expected I think -- bclary can confirm. /be
The hang is apparently bug 327697 -- applying that patch fixes my problems here. Dunno if the Mac crash is the same thing though :-(
The fix for bug 327697 should re-fix this and has been checked in.
Looks good on the 1.8 branch using the respun build Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:220.127.116.11) Gecko/20060307 Firefox/18.104.22.168 (2006-03-07-15). I see no crash using Jesse's testcase. Adding verified keyword.
Sorry for the bugspam, I mistyped and meant the 1.8.0 branch, not 1.8. (In reply to comment #34) > Looks good on the 1.8 branch using the respun build Mozilla/5.0 (Macintosh; U; > PPC Mac OS X Mach-O; en-US; rv:22.214.171.124) Gecko/20060307 Firefox/126.96.36.199 > (2006-03-07-15). I see no crash using Jesse's testcase. Adding verified > keyword. >
verified on the 1.8.1 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1b1) Gecko/20060810 BonEcho/2.0b1. No crash with testcase. Adding keyword.
Crashtest checked in.