Last Comment Bug 269095 - Firefox (more precisely the gecko engine) crashed by the scanit browser crash test [@ SinkContext::Begin]
: Firefox (more precisely the gecko engine) crashed by the scanit browser crash...
Status: VERIFIED FIXED
: crash, verified1.7.13
Product: Core
Classification: Components
Component: HTML: Parser (show other bugs)
: Trunk
: x86 Windows ME
: -- critical (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
: Blake Kaplan (:mrbkap) (please use needinfo!)
Mentors:
Depends on: 286733
Blocks: Zalewski ZDI-CAN-008
  Show dependency treegraph
 
Reported: 2004-11-11 01:01 PST by Yuan Qi
Modified: 2009-05-09 11:29 PDT (History)
9 users (show)
dveditz: blocking1.7.13+
dveditz: blocking‑aviary1.0.8+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
The crash inducing page (47.66 KB, text/html)
2004-11-11 01:02 PST, Yuan Qi
no flags Details
reduced testcase (64 bytes, text/html)
2004-11-11 08:40 PST, Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com]
no flags Details
this fixes the crash (3.64 KB, patch)
2004-11-11 11:33 PST, Blake Kaplan (:mrbkap) (please use needinfo!)
no flags Details | Diff | Review
another idea (3.93 KB, patch)
2004-11-11 13:07 PST, Blake Kaplan (:mrbkap) (please use needinfo!)
jst: review+
rbs: superreview+
Details | Diff | Review

Description Yuan Qi 2004-11-11 01:01:04 PST
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:
Comment 1 Yuan Qi 2004-11-11 01:02:12 PST
Created attachment 165522 [details]
The crash inducing page
Comment 2 Yuan Qi 2004-11-11 01:08:31 PST
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.
Comment 3 Robert Strong [:rstrong] (use needinfo to contact me) 2004-11-11 03:37:52 PST
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
Comment 4 Paul van Schayck 2004-11-11 04:39:02 PST
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
Comment 5 Bernd 2004-11-11 07:21:05 PST
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[0].mType = aNodeType;
  mStack[0].mContent = aRoot;
  mStack[0].mFlags = APPENDED;
  mStack[0].mNumFlushed = aNumFlushed;
  mStack[0].mInsertionPoint = aInsertionPoint;
===>  NS_ADDREF(aRoot);
  mStackPos = 1;
  mTextLength = 0;
aRoot is 0xcdcdcdcd
Comment 6 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2004-11-11 08:40:06 PST
Created attachment 165566 [details]
reduced testcase
Comment 7 Chris Thomas (CTho) [formerly cst@andrew.cmu.edu cst@yecc.com] 2004-11-11 08:43:31 PST
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"
Comment 8 Blake Kaplan (:mrbkap) (please use needinfo!) 2004-11-11 10:16:21 PST
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.
Comment 9 Blake Kaplan (:mrbkap) (please use needinfo!) 2004-11-11 11:33:36 PST
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>
      >
    >
  >
>
Comment 10 Blake Kaplan (:mrbkap) (please use needinfo!) 2004-11-11 13:07:12 PST
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 11 Blake Kaplan (:mrbkap) (please use needinfo!) 2004-11-11 13:11:40 PST
Comment on attachment 165610 [details] [diff] [review]
another idea

jst, can you give this r= (and/or have any thoughts on this)?
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2004-11-11 13:18:30 PST
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 13 Blake Kaplan (:mrbkap) (please use needinfo!) 2004-11-11 13:23:15 PST
Comment on attachment 165610 [details] [diff] [review]
another idea

Looking for sr=.
Comment 14 Yuan Qi 2004-11-12 08:42:48 PST
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.
Comment 15 Yuan Qi 2004-11-12 08:43:56 PST
I mean the source code of the crash test page.
Comment 16 Boris Zbarsky [:bz] 2004-11-12 09:13:00 PST
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 17 rbs 2004-11-12 17:17:17 PST
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
Comment 18 Blake Kaplan (:mrbkap) (please use needinfo!) 2004-11-12 22:39:37 PST
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
Comment 19 Stephen Donner [:stephend] - PTO; back on 5/28 2004-11-18 13:54:40 PST
Verified FIXED for me with build 2004-11-18-05 on Windows XP.
Comment 20 Jay Patel [:jay] 2006-02-13 13:25:13 PST
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.
Comment 21 Tracy Walker [:tracy] 2006-02-14 13:03:36 PST
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
Comment 22 Bob Clary [:bc:] 2009-05-09 11:29:32 PDT
parser/htmlparser/tests/crashtests/269095-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3

Note You need to log in before you can comment on or make changes to this bug.