User-Agent: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0 I tested firefox 1.0 on the following browser crash test from scanit: http://webtest.scanit.be/bcheck/mangle.php. It is based on mangleme by Michal Zalewski: http://www.securityfocus.com/archive/1/378632. Firefox handled the broken pages for a while, then crashed. I attached the crash inducing html file. Reproducible: Always Steps to Reproduce:
It is reported that Microsoft Internet Explorer does not crash with this test! Gecko developers should look into this test, which will help gecko to become more crash proof and allow it to handle malformed and nonstandard html better.
I get a crash with Firefox and it hangs Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041109. Reporter - will you provide a simplified testcase? http://www.mozilla.org/newlayout/bugathon.html#testcase
It even crashes X here. With this build: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.5) Gecko/20041109 Firefox/1.0
SinkContext::Begin(nsHTMLTag 0xcdcdcdcd, nsIHTMLContent * 0xcdcdcdcd, unsigned int 0xcdcdcdcd, int 0xffffffff) line 1106 + 3 bytes HTMLContentSink::BeginContext(HTMLContentSink * const 0x034ae090, int 0x00000003) line 2527 CNavDTD::HandleSavedTokens(int 0x00000003) line 2129 CNavDTD::HandleToken(CNavDTD * const 0x03667068, CToken * 0x035a4d18, nsIParser * 0x0334e0e0) line 894 + 17 bytes CNavDTD::HandleSavedTokens(int 0x00000003) line 2159 + 23 bytes CNavDTD::HandleToken(CNavDTD * const 0x03667068, CToken * 0x03560888, nsIParser * 0x0334e0e0) line 894 + 17 bytes CNavDTD::BuildModel(CNavDTD * const 0x03667068, nsIParser * 0x0334e0e0, nsITokenizer * 0x0343c640, nsITokenObserver * 0x00000000, nsIContentSink * 0x034ae090) line 471 + 17 bytes nsParser::BuildModel(nsParser * const 0x0334e0e0) line 2027 + 31 bytes nsParser::ResumeParse(int 0x00000001, int 0x00000001, int 0x00000001) line 1894 + 11 bytes nsParser::OnStopRequest(nsParser * const 0x0334e0e4, nsIRequest * 0x0336e800, nsISupports * 0x00000000, unsigned int 0x00000000) line 2608 + 21 bytes nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x033fc2a8, nsIRequest * 0x0336e800, nsISupports * 0x00000000, unsigned int 0x00000000) line 360 nsStreamListenerTee::OnStopRequest(nsStreamListenerTee * const 0x033db390, nsIRequest * 0x0336e800, nsISupports * 0x00000000, unsigned int 0x00000000) line 66 nsHttpChannel::OnStopRequest(nsHttpChannel * const 0x0336e808, nsIRequest * 0x032cddb8, nsISupports * 0x00000000, unsigned int 0x00000000) line 3755 nsInputStreamPump::OnStateStop() line 505 nsInputStreamPump::OnInputStreamReady(nsInputStreamPump * const 0x032cddbc, nsIAsyncInputStream * 0x02ff1cc8) line 341 + 11 bytes nsInputStreamReadyEvent::EventHandler(PLEvent * 0x034b7674) line 119 PL_HandleEvent(PLEvent * 0x034b7674) line 692 + 9 bytes PL_ProcessPendingEvents(PLEventQueue * 0x00ed4ff8) line 627 + 8 bytes _md_EventReceiverProc(HWND__ * 0x0006029a, unsigned int 0x0000c11a, unsigned int 0x00000000, long 0x00ed4ff8) line 1433 + 8 bytes USER32! 77d18709() USER32! 77d187eb() USER32! 77d189a5() USER32! 77d189e8() nsAppShell::Run(nsAppShell * const 0x00f9ac58) line 135 nsAppStartup::Run(nsAppStartup * const 0x00f9a9d8) line 221 main1(int 0x00000003, char * * 0x002a4230, nsISupports * 0x00edaee8) line 1321 + 31 bytes main(int 0x00000003, char * * 0x002a4230) line 1799 + 34 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 7c816d4f() This is a parser crash mStack.mType = aNodeType; mStack.mContent = aRoot; mStack.mFlags = APPENDED; mStack.mNumFlushed = aNumFlushed; mStack.mInsertionPoint = aInsertionPoint; ===> NS_ADDREF(aRoot); mStackPos = 1; mTextLength = 0; aRoot is 0xcdcdcdcd
Created attachment 165566 [details] reduced testcase
Note that I don't always actually crash if the testcase isn't the first page I load - instead, the browser hangs. To crash, I have to run "mozilla.exe https://bugzilla.mozilla.org/attachment.cgi?id=165566&action=view"
This is really complicated, and it's my first real experience with table stuff in CNavDTD to boot. Basically, what's happening is that we're getting nested HandleSavedToken() calls. Since we simply pass indexes from the DTD to the content sink, when we end up in this sort of nested call, the indexes are off (i.e., we've pushed stuff onto our stack that the content sink doesn't know about) so when the sink tries to use the DTD's indexes in the nested call, they're invalid and we crash. I need to investigate more to see what we *should* be doing in this case. I think we definately need to disallow going back into HandleSavedTokens(), but I'm not sure yet.
Created attachment 165581 [details] [diff] [review] this fixes the crash So, this patch makes us not crash, but I'm not sure I'm happy about it. Basically, we refuse to deal with (new) misplaced content if we're already dealing with misplaced content. Then, we need to make sure that we actually handled all of the content (and don't drop any on the floor). For reference, here is the content model created from the reduced testcase: html@03A01AE8 refcount=3< head@03A01C20 refcount=2< > frameset@03A5D628 refcount=3< Text@03A5D690 refcount=1<\n> > Text@03A608E0 refcount=1<\n\n> map@03A61580 refcount=2< Text@03A61628 refcount=1<\n> form@03A61758 refcount=1< Text@03A61890 refcount=1<\n> table@03A66D68 refcount=1< Text@03A3B960 refcount=1<\n> > param@03A59F68 refcount=1< > rtable@03A5A128 _moz-userdefined="" refcount=1< > table@03A66960 refcount=1< Text@03A3B760 refcount=1<\n> > > > >
Created attachment 165610 [details] [diff] [review] another idea I like this idea better. I don't think the other patch allows more tokens into the misplaced content stack if we were already processing misplaced content. This does that. My one question is whether or not the assertion in the comment in CNavDTD::DidBuildModel is valid (i.e., that mContentTopIndex really can contain all of the misplaced contents). Note: this patch passes the parser regression tests.
Comment on attachment 165610 [details] [diff] [review] another idea jst, can you give this r= (and/or have any thoughts on this)?
Comment on attachment 165610 [details] [diff] [review] another idea This deals with the maybe impossible case of nesting into this code with misplaced content that document.write()'s out more misplaced content etc? Looks like it does, so r=jst
Comment on attachment 165610 [details] [diff] [review] another idea Looking for sr=.
Remember that gecko might not just crash on this particular page, this crash test shows that gecko has some general problems when dealing with malformed html. If might be worth it to request the source code of the .php page.
I mean the source code of the crash test page.
There is no such thing as "general problems". There are specific problems caused by not anticipating certain types of input. Rest assured that Blake's patch here fixes not just this testcase but also various variants of it...
Comment on attachment 165610 [details] [diff] [review] another idea > My one question is whether or not the assertion in the comment The comment puzzled me as well. sr=rbs
Checked in with some minor comment tweaks. Checking in src/CNavDTD.cpp; /cvsroot/mozilla/parser/htmlparser/src/CNavDTD.cpp,v <-- CNavDTD.cpp new revision: 3.440; previous revision: 3.439 done
Verified FIXED for me with build 2004-11-18-05 on Windows XP.
v.fixed on 1.0.1 Aviary branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060213 Firefox/1.0.8, no crash or hang with reduced testcase.
verified with: Windows: Moz - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060214 Fx - Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060214 Firefox/1.0.8 Macintosh: Moz - Mozilla/5.0 (Macintosh; U;PPC Mac OS X Mach-O; en-US; rv:1.7.13) Gecko/20060214 Firefox/1.0.8 Fx - Mozilla/5.0 (Macintosh; U;PPC Mac OS X Mach-O; en-US; rv:1.7.13) Gecko/20060214 Firefox/1.0.8 Linux Moz - Mozilla/5.0 (X11; U;Linux i686; en-US; rv:1.7.13) Gecko/20060214 Fx - Mozilla/5.0 (X11; U;Linux i686; en-US; rv:1.7.13) Gecko/20060214 Firefox/1.0.8