430574 - Infinite document.write() loop hangs, fills memory, and crashes

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase 1

The testcase referenced in this post crashes FF3b5, by doing a document.write inside an infinite loop to fill up memory.

http://www.securityfocus.com/archive/1/491196/30/0/threaded

Here are 2 crash reports for it:
http://crash-stats.mozilla.com/report/index/8d59248f-1188-11dd-99a3-001cc45a2c28
http://crash-stats.mozilla.com/report/index/413dc1ab-1187-11dd-9dd9-001cc4e2bf68

Filing a security-sensitive bug on this because apparently this is related to a Safari vulnerability, so perhaps FF is vulnerable as well...(?)

Comment 1

[Daniel Holbert [:dholbert]]

I see the hang & crash on Ubuntu Linux 8.04 as well, btw.

Comment 2

[Daniel Holbert [:dholbert]]

(In reply to comment #1)
> I see the hang & crash on Ubuntu Linux 8.04 as well, btw.

(Mentioned that because the crash reports linked in comment 0 are both for windows)

I didn't get a crash-reporter dialog after the crash in Ubuntu, so I don't have a crash ID for that one.

Comment 3

[Jeff Walden [:Waldo]]

#3  0x2e4b23fc in nsScanner::AppendToBuffer (this=0x37261fb0, aStr=@0xbfffcedc) at /Users/jwalden/moz/trunk/mozilla/parser/htmlparser/src/nsScanner.h:323
323             AppendToBuffer(nsScannerString::AllocBufferFromString(aStr), nsnull);

The first argument evaluated to null, and of course that should have been checked before doing the append.  Do we care about playing this sort of whack-a-mole?  Easy to whack -- if we decide it's worth doing.  I dunno how this meshes with memory thinking going forward.

Comment 4

[Jeff Walden [:Waldo]]

This isn't the exact same stack -- I blew that away and haven't been able to reproduce that specific crash (we're handling OOM poorly everywhere, really) -- but this is about what I was hitting, going from memory of examining the stack and doing some source listing:


nsScanner::Append(const nsAString& aStr) at 
/Users/jwalden/moz/trunk/mozilla/parser/htmlparser/src/nsScanner.h:323
323         AppendToBuffer(nsScannerString::AllocBufferFromString(aStr), nsnull); // first argument evaluated to null
nsScanner::Append(const nsAString& aBuffer) at /Users/jwalden/moz/trunk/mozilla/parser/htmlparser/src/nsScanner.cpp:89
280   AppendToBuffer(aBuffer);
nsParser::Parse at /Users/jwalden/moz/trunk/mozilla/parser/htmlparser/src/nsParser.cpp:1452
1452       pc->mScanner->Append(aSourceBuffer);

(...below copied from a different crash, above is correct in what it shows)

#22 0x15a2452f in nsHTMLDocument::WriteCommon (this=0x2d51800, aText=@0xbfffcedc, aNewlineTerminate=0) at /Users/jwalden/moz/trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:2475
#23 0x15a2483a in nsHTMLDocument::ScriptWriteCommon (this=0x2d51800, aNewlineTerminate=0) at /Users/jwalden/moz/trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:2531
#24 0x15a24a03 in nsHTMLDocument::Write (this=0x2d51800) at /Users/jwalden/moz/trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:2559
#25 0x0173b322 in NS_InvokeByIndex_P (that=0x2d519e8, methodIndex=20, paramCount=0, params=0xbfffd174) at /Users/jwalden/moz/trunk/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179
#26 0x3540afdf in XPCWrappedNative::CallMethod (ccx=@0xbfffd3bc, mode=CALL_METHOD) at /Users/jwalden/moz/trunk/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2369
#27 0x35412b56 in XPC_WN_CallMethod (cx=0x3f00e480, obj=0x3efc9d40, argc=1, argv=0x2d1589c, vp=0xbfffd4fc) at /Users/jwalden/moz/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1473
#28 0x01410058 in js_Invoke (cx=0x3f00e480, argc=1, vp=0x2d15894, flags=2048) at /Users/jwalden/moz/trunk/mozilla/js/src/jsinterp.c:1283
#29 0x01402f9f in js_Interpret (cx=0x3f00e480) at /Users/jwalden/moz/trunk/mozilla/js/src/jsinterp.c:4834
#30 0x01410945 in js_Execute (cx=0x3f00e480, chain=0x3efc9a00, script=0x3f2e0df0, down=0x0, flags=2048, result=0xbfffdd9c) at /Users/jwalden/moz/trunk/mozilla/js/src/jsinterp.c:1522
#31 0x013afe97 in JS_EvaluateUCScriptForPrincipals (cx=0x3f00e480, obj=0x3efc9a00, principals=0x3f2db394, chars=0xbfffe060, length=7, filename=0xbfffde64 "javascript:crash()", lineno=1, rval=0xbfffdd9c) at /Users/jwalden/moz/trunk/mozilla/js/src/jsapi.c:4998
#32 0x15b0512b in nsJSContext::EvaluateString (this=0x3f00d900, aScript=@0xbfffe04c, aScopeObject=0x3efc9a00, aPrincipal=0x3f2db390, aURL=0xbfffde64 "javascript:crash()", aLineNo=1, aVersion=0, aRetValue=0xbfffdf48, aIsUndefined=0xbfffdf44) at /Users/jwalden/moz/trunk/mozilla/dom/src/base/nsJSEnvironment.cpp:1530
#33 0x15b70abe in nsJSThunk::EvaluateScript (this=0x372f65c0, aChannel=0x3f2bf240, aPopupState=openAllowed, aExecutionPolicy=2, aOriginalInnerWindow=0x3f2de200) at /Users/jwalden/moz/trunk/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp:331
#34 0x15b70faf in nsJSChannel::EvaluateScript (this=0x3f2dae00) at /Users/jwalden/moz/trunk/mozilla/dom/src/jsurl/nsJSProtocolHandler.cpp:722
#35 0x15f09249 in nsRunnableMethod<nsJSChannel>::Run (this=0x400606b0) at ../../../dist/include/xpcom/nsThreadUtils.h:261
#36 0x01728c8f in nsThread::ProcessNextEvent (this=0x23136d0, mayWait=0, result=0xbfffe274) at /Users/jwalden/moz/trunk/mozilla/xpcom/threads/nsThread.cpp:510
#37 0x016cc711 in NS_ProcessPendingEvents_P (thread=0x23136d0, timeout=20) at nsThreadUtils.cpp:180
#38 0x34898217 in nsBaseAppShell::NativeEventCallback (this=0x367d6180) at /Users/jwalden/moz/trunk/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:121
#39 0x348623a3 in nsAppShell::ProcessGeckoEvents (aInfo=0x367d6180) at /Users/jwalden/moz/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:302
#40 0x9082cefa in CFRunLoopRunSpecific ()
#41 0x9082ca36 in CFRunLoopRunInMode ()
#42 0x92df5878 in RunCurrentEventLoopInMode ()
#43 0x92df4eb9 in ReceiveNextEventCommon ()
#44 0x92df4dd9 in BlockUntilNextEventMatchingListInMode ()
#45 0x9329c0e5 in _DPSNextEvent ()
#46 0x9329bcd7 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#47 0x93295a64 in -[NSApplication run] ()
#48 0x34860e5f in nsAppShell::Run (this=0x367d6180) at /Users/jwalden/moz/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:591
#49 0x33a9bf37 in nsAppStartup::Run (this=0x2344500) at /Users/jwalden/moz/trunk/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:181
#50 0x00210c10 in XRE_main (argc=4, argv=0xbffff750, aAppData=0x230c090) at /Users/jwalden/moz/trunk/mozilla/toolkit/xre/nsAppRunner.cpp:3170
#51 0x00002798 in main (argc=6, argv=0xbffff750) at /Users/jwalden/moz/trunk/mozilla/browser/app/nsBrowserApp.cpp:158

Comment 5

[timeless]

Attachment: cave in the tunnel

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 318097 [details] [diff] [review]
cave in the tunnel

>Index: mozilla/parser/htmlparser/src/nsScanner.cpp
>+PRBool nsScanner::AppendToBuffer(nsScannerString::Buffer* aBuf,
>+                                 nsIRequest *aRequest)
> {
>   if (nsParser::sParserDataListeners && mParser &&
>       NS_FAILED(mParser->DataAdded(Substring(aBuf->DataStart(),
>                                              aBuf->DataEnd()), aRequest))) {
>     // Don't actually append on failure.
> 
>-    return;
>+    return PR_TRUE;

Perhaps this should return mSlidingBuffer != nsnull?

Comment 7

[Brian Crowder]

Did this get fixed via some other route?  Is there a reason the patch here has r+ but no a?

Comment 8

[Brian Crowder]

timeless:  What's the status here?

Comment 10

[Daniel Veditz [:dveditz]]

Since this is prompted from a public discussion and testcase this ought to be public to catch dupes.

-> mrbkap to answer comment 7 and check-in if still wanted. Looks like this ought to be relevant to Firefox 2 as well.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Updated to my comments

This is updated to my comments (and to compile). jst, what do you think?

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 328673 [details] [diff] [review]
Updated to my comments

sr=jst

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Pushed for timeless as changeset 39aeb2ccb067.

Comment 14

[sibbl]

i'm using 2008071504 where this bug should be fixed and it's still crashing?!? vista sp0 ...

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Do you have a stack trace or breakpad report?

Comment 16

[sibbl]

nope sorry ... i just click "crash" in the testcase and then i see the increasing memory usage of firefox.exe. If I don't kill the process with task manager my hole vista is going to be slow and maybe everything crashes ...

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 328673 [details] [diff] [review]
Updated to my comments

This applies cleanly to 1.9.0; we should just take this.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: Backport to 1.8 branch

The only changes here are basically updating the code that maintains mTotalRead conditionally based on whether the append succeeded.

Comment 19

[Samuel Sidler (old account; do not CC)]

Comment on attachment 328673 [details] [diff] [review]
Updated to my comments

1.9.0.4 is code frozen, pending regressions. (That's why we have the flags still active.)

Comment 20

[Boris Zbarsky [:bzbarsky]]

All good.  I think taking this in 1.9.0.5 is perfectly reasonable

Comment 21

[Daniel Veditz [:dveditz]]

Comment on attachment 328673 [details] [diff] [review]
Updated to my comments

approved for 1.9.0.5 and 1.8.1.19, a=dveditz for release-drivers

(please watch tinderbox for the tree to open before landing)

Comment 23

[Boris Zbarsky [:bzbarsky]]

Fixed on both branches.

Comment 24

[Al Billings [:abillings - ex-MoCo]]

For 1.8.1.19, I'm not seeing any real difference in behavior versus 1.8.1.18. Both prompt to stop the script or continue. If you tell it to continue, it will prompt you again in a minute. If you tell it to stop, it will eat up a bunch of RAM. 1.8.1.18 gets up to 460 MB and 1.8.1.19 gets up to 278 MB on my XP VM. I never see a crach in 1.8.1.18.

For 1.9.0.4, it crashes cleanly. Unfortunately, the 1.9.0 nightly that I have with the fix ALSO crashes cleanly. This is build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008112505 GranParadiso/3.0.5pr with Crash ID: bp-9671acdc-47b7-48e9-93be-6d6bd2081125.

So, this appears unfixed in 1.9.0.5 and not reproducible in 1.8.1.19.

Comment 25

[Al Billings [:abillings - ex-MoCo]]

The latest nightly for 1.9.0.5 is still crashing with the reduced testcase here. This is build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008120105 GranParadiso/3.0.5pre.

Comment 26

[Al Billings [:abillings - ex-MoCo]]

The Crash ID for comment 25 is: bp-f0e057dd-1135-4367-a9e9-6dcb62081201

Comment 27

[timeless]

UUID	f0e057dd-1135-4367-a9e9-6dcb62081201
0  	kernel32.dll  	kernel32.dll@0x12aeb  	
1 	mozcrt19.dll 	_CxxThrowException 	throw.cpp:159
2 	mozcrt19.dll 	operator new 	new.cpp:57
3 	xul.dll 	gfxTextRun::operator new 	mozilla/gfx/thebes/src/gfxFont.cpp:1072
4 	xul.dll 	gfxTextRun::Create 	mozilla/gfx/thebes/src/gfxFont.cpp:1059
5 	xul.dll 	TextRunWordCache::MakeTextRun 	mozilla/gfx/thebes/src/gfxTextRunWordCache.cpp:618

kernel32.dll  	5.1.2600.5512  	34560E80F5C54175B208848EF863C5BD2  	kernel32.pdb

that's expected, it's a "safe" crash.

We don't have any handling for that, and adding handling for it requires making sure *all* error handling is correct, otherwise we open the door for exploits as described in the oom exploit elsewhere.

btw, can you please talk to ted about getting symbols for kernel32 and friends from your system uploaded?

Comment 28

[Al Billings [:abillings - ex-MoCo]]

timesless, so I should mark this as verified for 1.9.0.5 because this newer crash is expected.

As to symbols, I'm using a standard QA virtual machine for xp here. If there are symbols that I can easily add to the VM to make the crash data more useful, I'm happy to do so.

Comment 29

[Al Billings [:abillings - ex-MoCo]]

timeless, do you have a response to my remark in comment 24 about 1.8.1.18 versus 1.8.1.19?

Comment 30

[timeless]

dunno about comment 24, there are probably some failure characteristics which enable to to die safely, in fact, i'd expect there are quite a few, although it's odd that your memory use doesn't go near 1gb.

re symbols, contact ted. and yeah, the other crash is expected and shouldn't prevent a verification.

Comment 31

[Alexander Sack]

Comment on attachment 345533 [details] [diff] [review]
Backport to 1.8 branch

a=asac for 1.8.0 branch