Firefox (more precisely the gecko engine) crashed by the scanit browser crash test [@ SinkContext::Begin]

VERIFIED FIXED

Status

()

Core
HTML: Parser
--
critical
VERIFIED FIXED
13 years ago
8 years ago

People

(Reporter: Yuan Qi, Assigned: mrbkap)

Tracking

({crash, verified1.7.13})

Trunk
x86
Windows ME
crash, verified1.7.13
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.7.13 +
blocking-aviary1.0.8 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

13 years ago
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:
(Reporter)

Comment 1

13 years ago
Created attachment 165522 [details]
The crash inducing page
(Reporter)

Updated

13 years ago
Summary: Firefox (more precisely the gecko engine) crashed when viewing this page → Firefox (more precisely the gecko engine) crashed by the scanit browser crash test
(Reporter)

Comment 2

13 years ago
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.

Updated

13 years ago
Severity: normal → critical
Keywords: crash
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

13 years ago
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

13 years ago
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
Assignee: nobody → parser
Status: UNCONFIRMED → NEW
Component: Layout → HTML: Parser
Ever confirmed: true
QA Contact: core.layout → mrbkap

Updated

13 years ago
Blocks: 264944
Created attachment 165566 [details]
reduced testcase
Attachment #165522 - Attachment is obsolete: true
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"
(Assignee)

Comment 8

13 years ago
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.

Updated

13 years ago
Summary: Firefox (more precisely the gecko engine) crashed by the scanit browser crash test → Firefox (more precisely the gecko engine) crashed by the scanit browser crash test [@ SinkContext::Begin]
(Assignee)

Comment 9

13 years ago
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>
      >
    >
  >
>
Assignee: parser → mrbkap
Status: NEW → ASSIGNED
(Assignee)

Comment 10

13 years ago
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.
Attachment #165581 - Attachment is obsolete: true
(Assignee)

Comment 11

13 years ago
Comment on attachment 165610 [details] [diff] [review]
another idea

jst, can you give this r= (and/or have any thoughts on this)?
Attachment #165610 - Flags: review?(jst)
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
Attachment #165610 - Flags: review?(jst) → review+
(Assignee)

Comment 13

13 years ago
Comment on attachment 165610 [details] [diff] [review]
another idea

Looking for sr=.
Attachment #165610 - Flags: superreview?(rbs)
(Reporter)

Comment 14

13 years ago
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.
(Reporter)

Comment 15

13 years ago
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 17

13 years ago
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
Attachment #165610 - Flags: superreview?(rbs) → superreview+
(Assignee)

Comment 18

13 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Verified FIXED for me with build 2004-11-18-05 on Windows XP.
Status: RESOLVED → VERIFIED
Depends on: 286733
Blocks: 320182
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8+
Keywords: fixed-aviary1.0.8, fixed1.7.13

Comment 20

11 years ago
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.
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
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
Keywords: fixed1.7.13 → verified1.7.13

Comment 22

8 years ago
parser/htmlparser/tests/crashtests/269095-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3
Flags: in-testsuite+
Crash Signature: [@ SinkContext::Begin]
You need to log in before you can comment on or make changes to this bug.