374083 - HTML "document.writeln" from SVG onload handler crashes [@ PresShell::PopCurrentEventInfo] Firefox

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Duncan Loveday]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070315 Minefield/3.0a3pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070315 Minefield/3.0a3pre

The following code crashes firefox

var x;
alert("x.y=" + x.y);

Reproducible: Always

Steps to Reproduce:
1. open the attached nullref.html in firefox
2.
3.
Actual Results:  
Firefox crashes

Expected Results:  
Firefox should display an error in the javascript console.

Comment 1

[Duncan Loveday]

This bug is not what I thought it was....turns out to be to do with writing to the HTML document from an SVG onload handler. See attached test html.

Have updated the bug description, please ignore the thing about javascript undefined references.

Comment 2

[Duncan Loveday]

Attachment: test HTML source

Comment 3

[Duncan Loveday]

Attachment: test SVG source

Comment 4

[Jesse Ruderman]

Attachment: single-file testcase

Comment 5

[Jesse Ruderman]

Crashes dereferencing 0x40000008, so it's likely to be an exploitable security hole, not just a crash.

Comment 6

[Martijn Wargers (dead)]

This regressed for me between 2007-03-04 and 2007-03-05:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-03-04+04&maxdate=2007-03-05+09&cvsroot=%2Fcvsroot
I would think a regression from bug 363253.

Talkback ID: TB30267652K
PresShell::PopCurrentEventInfo  [mozilla/layout/base/nspresshell.cpp, line 5013]
nsXMLContentSink::HandleEndElement  [mozilla/content/xml/document/src/nsxmlcontentsink.cpp, line 1122]
nsExpatDriver::HandleEndElement  [mozilla/parser/htmlparser/src/nsexpatdriver.cpp, line 439]
contentProcessor  [mozilla/parser/expat/lib/xmlparse.c, line 2095]
prologProcessor  [mozilla/parser/expat/lib/xmlparse.c, line 3810]
prologInitProcessor  [mozilla/parser/expat/lib/xmlparse.c, line 3626]
nsExpatDriver::ParseBuffer  [mozilla/parser/htmlparser/src/nsexpatdriver.cpp, line 991]
nsExpatDriver::ConsumeToken  [mozilla/parser/htmlparser/src/nsexpatdriver.cpp, line 1097]
nsParser::Tokenize  [mozilla/parser/htmlparser/src/nsparser.cpp, line 2398]
nsParser::ResumeParse  [mozilla/parser/htmlparser/src/nsparser.cpp, line 1624]
nsDocumentOpenInfo::OnDataAvailable  [mozilla/uriloader/base/nsuriloader.cpp, line 362]
nsBaseChannel::OnDataAvailable  [mozilla/netwerk/base/src/nsbasechannel.cpp, line 641]
nsInputStreamPump::OnStateTransfer  [mozilla/netwerk/base/src/nsinputstreampump.cpp, line 503]
nsInputStreamPump::OnInputStreamReady  [mozilla/netwerk/base/src/nsinputstreampump.cpp, line 394]
nsOutputStreamReadyEvent::Run  [mozilla/xpcom/io/nsstreamutils.cpp, line 112]
NS_ProcessNextEvent_P  [mozilla/xpcom/build/nsthreadutils.cpp, line 225]
nsBaseAppShell::Run  [mozilla/widget/src/xpwidgets/nsbaseappshell.cpp, line 153]

Comment 7

[Jesse Ruderman]

For me, this crash goes back at least to September 2006, so I don't think it's a regression from bug 18333 or bug 370210 or bug 363253.  Could it be related to bug 372147?

I can't get Firefox 2.0.0.2 to crash, but its behavior seems different too: clicking the back button doesn't immediately fire onload again.

Comment 8

[Martijn Wargers (dead)]

(In reply to comment #7)
> For me, this crash goes back at least to September 2006, so I don't think it's
> a regression from bug 18333 or bug 370210 or bug 363253.  Could it be related
> to bug 372147?

Well, I don't crash with builds before 2007-03-04, so I can't test. I guess a fix for bug 372147 might help, indeed.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 258685 [details]
single-file testcase

This works for me, but the original test case does crash.

Comment 10

[Daniel Veditz [:dveditz]]

Tor: if you can't work on this please find another owner for this security bug.

Comment 11

[Jonathan Watt [:jwatt]]

The correct way to fix this is probably to fix bug 372147 as Martijn says. That said, what's happening seems to be that the PresShell gets destroyed as a result of the the writeln. As a result, when PresShell::HandleDOMEventWithTarget has finished dispatching the event and calls PresShell::PopCurrentEventInfo, mCurrentEventContent is dead. Stack inside writeln is:

PresShell::`scalar deleting destructor'()
PresShell::Release()
nsCOMPtr<nsIPresShell>::assign_assuming_AddRef(nsIPresShell * newPtr)
nsCOMPtr<nsIPresShell>::assign_with_AddRef(nsISupports * rawPtr)
nsCOMPtr<nsIPresShell>::operator=(nsIPresShell * rhs)
DocumentViewerImpl::Hide()
nsDocShell::SetVisibility(int aVisibility)
nsSubDocumentFrame::Destroy()
nsLineBox::DeleteLineList(nsPresContext * aPresContext, nsLineList & aLines)
nsBlockFrame::Destroy()
nsBlockFrame::DoRemoveFrame(nsIFrame * aDeletedFrame, int aDestroyFrames, int aRemoveOnlyFluidContinuations)
nsBlockFrame::RemoveFrame(nsIAtom * aListName, nsIFrame * aOldFrame)
nsFrameManager::RemoveFrame(nsIFrame * aParentFrame, nsIAtom * aListName, nsIFrame * aOldFrame)
nsCSSFrameConstructor::ContentRemoved(nsIContent * aContainer, nsIContent * aChild, int aIndexInContainer, int aInReinsertContent)
PresShell::ContentRemoved(nsIDocument * aDocument, nsIContent * aContainer, nsIContent * aChild, int aIndexInContainer)
nsBindingManager::ContentRemoved(nsIDocument * aDocument, nsIContent * aContainer, nsIContent * aChild, int aIndexInContainer)
nsNodeUtils::ContentRemoved(nsINode * aContainer, nsIContent * aChild, int aIndexInContainer)
nsGenericElement::doRemoveChildAt(unsigned int aIndex, int aNotify, nsIContent * aKid, nsIContent * aParent, nsIDocument * aDocument, nsAttrAndChildArray & aChildArray)
nsGenericElement::RemoveChildAt(unsigned int aIndex, int aNotify)
nsHTMLDocument::OpenCommon(const nsACString_internal & aContentType, int aReplace)
nsHTMLDocument::Open(const nsACString_internal & aContentType, int aReplace, nsIDOMDocument * * aReturn)
nsHTMLDocument::Open()
nsHTMLDocument::WriteCommon(const nsAString_internal & aText, int aNewlineTerminate)
nsHTMLDocument::ScriptWriteCommon(int aNewlineTerminate)
nsHTMLDocument::Writeln()
NS_InvokeByIndex(nsISupports * that, unsigned int methodIndex, unsigned int paramCount, nsXPTCVariant * params)
XPCWrappedNative::CallMethod(XPCCallContext & ccx, XPCWrappedNative::CallMode mode)
XPCWrappedNative::CallMethod(XPCCallContext & ccx, XPCWrappedNative::CallMode mode)
XPC_WN_CallMethod(JSContext * cx, JSObject * obj, unsigned int argc, long * argv, long * vp)
js_Invoke(JSContext * cx, unsigned int argc, unsigned int flags)	C
js_Interpret(JSContext * cx, unsigned char * pc, long * result)	C
js_Invoke(JSContext * cx, unsigned int argc, unsigned int flags)	C
js_InternalInvoke(JSContext * cx, JSObject * obj, long fval, unsigned int flags, unsigned int argc, long * argv, long * rval)	C
JS_CallFunctionValue(JSContext * cx, JSObject * obj, long fval, unsigned int argc, long * argv, long * rval)	C
nsJSContext::CallEventHandler(nsISupports * aTarget, void * aScope, void * aHandler, nsIArray * aargv, nsIVariant * * arv)
nsJSEventListener::HandleEvent(nsIDOMEvent * aEvent)
nsEventListenerManager::HandleEventSubType(nsListenerStruct * aListenerStruct, nsIDOMEventListener * aListener, nsIDOMEvent * aDOMEvent, nsISupports * aCurrentTarget, unsigned int aPhaseFlags)
nsEventListenerManager::HandleEvent(nsPresContext * aPresContext, nsEvent * aEvent, nsIDOMEvent * * aDOMEvent, nsISupports * aCurrentTarget, unsigned int aFlags, nsEventStatus * aEventStatus)
nsEventTargetChainItem::HandleEvent(nsEventChainPostVisitor & aVisitor, unsigned int aFlags)
nsEventTargetChainItem::HandleEventTargetChain(nsEventChainPostVisitor & aVisitor, unsigned int aFlags, nsDispatchingCallback * aCallback)
nsEventDispatcher::Dispatch(nsISupports * aTarget, nsPresContext * aPresContext, nsEvent * aEvent, nsIDOMEvent * aDOMEvent, nsEventStatus * aEventStatus, nsDispatchingCallback * aCallback)
PresShell::HandleDOMEventWithTarget(nsIContent * aTargetContent, nsEvent * aEvent, nsEventStatus * aStatus)
nsXMLContentSink::HandleEndElement(const unsigned short * aName)

Comment 12

[Jonathan Watt [:jwatt]]

Attachment: stack - PresShell being destroyed in writeln

Here's the stack in a text file so it can be viewed without all the damn line wrapping.

Comment 13

[chris hofmann]

tor, does that suggestion about fixing as part of bug 372147 make sense?  who should review the patches there to help keep moving forward on this one?

Comment 14

[tor]

I'm not familiar enough with the HTML code right now to say definitively whether this SVG testcase is just exposing an existing vulnerability on the HTML side, or if the early event is something that can't happen any other way.

Bug 372147 was put on hold while we waited for clarification from the SVG working group regarding the SVGLoad event.  While they haven't issued the official errata yet (next week if their plans hold), based on our conversations with them we think we know their intentions.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

The crash seen in comment #6 should be fixed in bug 375196, 
but I'm not sure about the stack in comment #11. Possibly also that one.

Comment 16

[Martijn Wargers (dead)]

Indeed, the testcase doesn't crash for me in a 2007-03-25 build, but it does crash in a 2007-03-25 build, so for me this is fixed by bug 375196.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Marking FIXED. If anyone can still reproduce, please reopen.

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

After landing bug 375196, this still crashes on 1.8.0 debug build,
but the crash is actually happening somewhere in nsTraceRefcntImpl.
I guess I (or someone else) need to build non-debug-1.8.0 and retest.

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Using non-debug-1.8.0 build I can't reproduce the crash.
(on 1.8 it doesn't matter whether the build is debug or non-debug)
But I guess some more testing would be great, especially using
non-debug windows 1.8.0 build. (I use only linux)