splitText() overflow causes Minefield to crash [@ nsBlockFrame::AppendFrames]




12 years ago
3 years ago


(Reporter: msg, Assigned: roc)


({crash, regression})

Windows XP
crash, regression
Bug Flags:
blocking1.9 +
wanted1.8.1.x -
wanted1.8.0.x -

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [sg:critical?] post-1.8-branch [works for me?], crash signature)


(3 attachments)



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

splitText() appears to have a buffer overflow when presented with long string arguments.  This has been verified on the latest versions of Minefield on Win32.

FOund w/Javascript introspection fuzzer.  Calling splitText() directly doesn't have same results -- likely requires other dom elements to be manipulated prior to the splitText call.  See attached script that has been tweaked to only trigger this flaw.  Expect to respond to error popups for bad protocol 2-3 times prior to crash.

See win32 talkback output attached.

Reproducible: Always

Steps to Reproduce:
1. Go to above url.
2. click ok when it says its starting
3. click ok in response to bad protocol dialogs (unrelated to vuln)

Actual Results:  
Browser crashes.  EIP appears to be null.

Expected Results:  
Browser chugs along...

see attachments for test code + talkback.  test code has *only* been tested when served from a webserver.

Comment 1

12 years ago
Created attachment 258815 [details]
talkback output

talkback output from crash

Comment 2

12 years ago
Created attachment 258816 [details]
repro script

repro script.  expect to click ok to proto error dialog a couple of times prior to crash
The repro script testcase only crashes for me when I test it locally, after I've clicked 2 or 3 alerts away.

Reporter, thanks for attaching the talkback output, but only a talkback ID is useful, normally.
I get this Talkback ID: TB30306141E
nsTextFragment::CharAt  [mozilla/dist/include/content/nstextfragment.h, line 184]
nsBlockFrame::AppendFrames  [mozilla/layout/generic/nsblockframe.cpp, line 4561]
nsCSSFrameConstructor::AppendFrames  [mozilla/layout/base/nscssframeconstructor.cpp, line 7855]
nsCSSFrameConstructor::ContentAppended  [mozilla/layout/base/nscssframeconstructor.cpp, line 8589]

I can't reproduce the crash on branch.
It's crashing in layout, so I'm reassigning this to layout.

I get a regression range between 2006-09-21 and 2006-09-22:
Regression from bug 312963 or bug 350137?  
Assignee: general → nobody
Component: DOM: Level 0 → Layout
Ever confirmed: true
QA Contact: ian → layout
Summary: splitText() overflow causes Minefield to crash → splitText() overflow causes Minefield to crash [@ nsBlockFrame::AppendFrames]


12 years ago
Flags: blocking1.9?
Keywords: crash, regression
Whiteboard: [sg:critical?] post-1.8

Comment 4

12 years ago
roc, can you have a look?
Assignee: nobody → roc


12 years ago
Whiteboard: [sg:critical?] post-1.8 → [sg:critical?] post-1.8-branch
Flags: wanted1.8.1.x-
Flags: wanted1.8.0.x-
roc, have you had a chance to look at this yet?

Comment 6

12 years ago
WFM using Mac trunk debug and a local copy of the repro script.

Comment 7

12 years ago
Oops, I don't think I was testing properly.  I was using a fresh profile, which allowed the script to resize the window, so I didn't see the third alert and thought it was done.

I got way too many alerts, so I uncommented the 
  x != "location" &&
line and I'm trying again.

Comment 8

12 years ago
I also replaced the XMLHttpRequest logging with calls to dump() so I could see what was going on cheaply :)

After running for about 10 minutes, it allocated nearly 4GB of memory, failed three times to allocate 16MB chunks, and crashed.  This leaves me wondering (1) why it ran out of memory and (2) why it crashed dereferencing the bogus address 0x5fc3e000 rather than with a null deref.  More soon.

Comment 9

12 years ago
Created attachment 267371 [details]
tail of console output + crash log

Crashed within nsTextFragment::AppendTo, suggesting that nsTextFragment doesn't handle OOM well.

Comment 10

12 years ago
Martijn or lsg-mtso, do you still see a non-OOM crash here?  I'm going to file a separate bug for the(?) OOM crash.

Comment 11

12 years ago
I filed bug 383426 with a testcase that triggers what looks like a buffer overflow in OOM situations.

Roc, any news on this?
Jesse has a patch in bug 383426 for the security-sensitive part of this bug. I don't think any other work has to be done here.

Comment 14

12 years ago
I'd mark this as WFM if I had tested on the same platform as the reporter.
So, is this WFM?

Comment 16

12 years ago
This needs to be retested on Windows XP, preferably by the reporter (lsg-mtso), before it can be marked as WFM.
I crash immediately on the testcase in WinXP on the trunk. Couldn't figure out which breakpad report was mine, but in a trunk build I get the assertion

WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: file c:/dev/fftrunk/mozilla/content/xbl/src/nsXBLService.cpp, line 121
###!!! ASSERTION: bad index: 'PRUint32(aIndex) < mState.mLength', file c:/dev/fftrunk/mozilla/layout/base/../../content/base/src\nsTextFragment.h, line 184

it looks like a null deref (plus offset mIndex which might be controllable by the content):

 	nsTextFragment::CharAt() Line 185	C++
 	nsTextFrame::HasTerminalNewline() Line 5841	C++
 	ShouldPutNextSiblingOnNewLine() Line 4657	C++
 	nsBlockFrame::AddFrames() Line 4743	C++
>	nsBlockFrame::AppendFrames() Line 4592	C++
 	nsFrameManager::AppendFrames() Line 155	C++
 	nsCSSFrameConstructor::AppendFrames() Line 7840	C++
 	nsCSSFrameConstructor::ContentAppended() Line 8582	C++
 	PresShell::ContentAppended() Line 4431	C++
 	nsBindingManager::ContentAppended() Line 1246	C++
 	nsNodeUtils::ContentAppended() Line 110	C++
 	nsGenericElement::doInsertChildAt() Line 2627	C++
 	nsGenericElement::InsertChildAt() Line 2560	C++
 	nsGenericDOMDataNode::SplitText() Line 895	C++
 	nsTextNode::SplitText() Line 72	C++
 	NS_InvokeByIndex_P() Line 102	C++
 	XPCWrappedNative::CallMethod() Line 2240	C++
 	XPC_WN_CallMethod() Line 1467	C++
 	js_Invoke() Line 1306	C
 	js_Interpret() Line 3992	C
 	js_Execute() Line 1567	C
 	JS_EvaluateUCScriptForPrincipals() Line 4792	C
 	nsJSContext::EvaluateString() Line 1381	C++
 	nsScriptLoader::EvaluateScript() Line 607	C++
 	nsScriptLoader::ProcessRequest() Line 521	C++
 	nsScriptLoader::ProcessScriptElement() Line 486	C++
 	nsScriptElement::MaybeProcessScript() Line 200	C++
 	nsHTMLScriptElement::MaybeProcessScript() Line 534	C++
 	nsHTMLScriptElement::DoneAddingChildren() Line 480	C++
 	HTMLContentSink::ProcessSCRIPTEndTag() Line 3142	C++
 	SinkContext::CloseContainer() Line 1008	C++
 	HTMLContentSink::CloseContainer() Line 2405	C++
 	CNavDTD::CloseContainer() Line 2678	C++
 	CNavDTD::HandleEndToken() Line 1588	C++
 	CNavDTD::HandleToken() Line 703	C++
 	CNavDTD::BuildModel() Line 331	C++
 	nsParser::BuildModel() Line 1717	C++
 	nsParser::ResumeParse() Line 1594	C++
 	nsParser::OnDataAvailable() Line 2231	C++
 	nsDocumentOpenInfo::OnDataAvailable() Line 360	C++
 	nsStreamListenerTee::OnDataAvailable() Line 97	C++
 	nsHttpChannel::OnDataAvailable() Line 4388	C++
 	nsInputStreamPump::OnStateTransfer() Line 502	C++
 	nsInputStreamPump::OnInputStreamReady() Line 392	C++
 	nsInputStreamReadyEvent::Run() Line 111	C++
 	nsThread::ProcessNextEvent() Line 482	C++
 	NS_ProcessNextEvent_P() Line 227	C++
 	nsBaseAppShell::Run() Line 154	C++
 	nsAppStartup::Run() Line 171	C++
 	XRE_main() Line 2810	C++
 	main() Line 69	C++
 	mainCRTStartup() Line 398	C

Still a problem, but not the same stack as originally reported.

Comment 18

12 years ago
That assertion also shows up in bug 385426, but I don't think the testcase in this bug (which does exhaustive testing of DOM functions up to a certain depth) triggers it for the same reason.

If you modify the testcase to output exactly what it's doing right before it makes each call, and then paste the sequence back in, you should be able to make a reduced testcase pretty easily.

Comment 19

12 years ago
> Still a problem, but not the same stack as originally reported.

Your stack looks pretty similar to Martijn's in comment 3, actually.
Is this something we can get QA to test/verify to get progress on this?
It's still crashing for me, after clicking a few alerts away, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007092605 Minefield/3.0a9pre

0  	nsTextFragment::CharAt(int)  	 nsTextFragment.h:185
1 	HasTerminalNewline 	mozilla/layout/generic/nsTextFrameThebes.cpp:1054
2 	nsTextFrame::HasTerminalNewline() 	mozilla/layout/generic/nsTextFrameThebes.cpp:5824
3 	ShouldPutNextSiblingOnNewLine 	mozilla/layout/generic/nsBlockFrame.cpp:4756
4 	nsBlockFrame::AddFrames(nsIFrame*, nsIFrame*) 	mozilla/layout/generic/nsBlockFrame.cpp:4854
5 	nsBlockFrame::AppendFrames(nsIAtom*, nsIFrame*) 	mozilla/layout/generic/nsBlockFrame.cpp:4691
6 	nsCSSFrameConstructor::AppendFrames(nsFrameConstructorState&, nsIContent*, nsIFrame*, nsFrameItems&, nsIFrame*) 	mozilla/layout/base/nsCSSFrameConstructor.cpp:8039
7 	nsCSSFrameConstructor::ContentAppended(nsIContent*, int) 	mozilla/layout/base/nsCSSFrameConstructor.cpp:8714
On Mac, latest trunk, I don't crash. It's kinda hard to see what's going on because the window becomes super-wide, but alerts keep popping up (after a while I can't see them, but I can dismiss them with the keyboard) and eventually the page stops loading or doing anything. Then I can quit the browser. There are assertions about bad widths and stuff, and the window looks garbled, but nothing dangerous-looking.
Whiteboard: [sg:critical?] post-1.8-branch → [sg:critical?] post-1.8-branch [works for me?]


12 years ago
Last Resolved: 12 years ago
Resolution: --- → WORKSFORME
Crash Signature: [@ nsBlockFrame::AppendFrames]


4 years ago
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.