Last Comment Bug 430574 - Infinite document.write() loop hangs, fills memory, and crashes
: Infinite document.write() loop hangs, fills memory, and crashes
Status: RESOLVED FIXED
[sg:nse dos]
: crash, fixed1.8.1.19, hang, testcase, verified1.9.0.5
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All All
: -- critical (vote)
: ---
Assigned To: timeless
:
Mentors:
http://es.geocities.com/jplopezy/prue...
: 401814 441631 441670 (view as bug list)
Depends on:
Blocks: 401814
  Show dependency treegraph
 
Reported: 2008-04-23 16:08 PDT by Daniel Holbert [:dholbert] (largely AFK until June 28)
Modified: 2009-09-14 20:31 PDT (History)
19 users (show)
samuel.sidler+old: wanted1.9.0.x+
dveditz: wanted1.8.1.x?
asac: blocking1.8.0.next+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
reduced testcase 1 (1.18 KB, text/html)
2008-04-23 16:08 PDT, Daniel Holbert [:dholbert] (largely AFK until June 28)
no flags Details
cave in the tunnel (11.72 KB, patch)
2008-04-27 23:28 PDT, timeless
mrbkap: review+
Details | Diff | Review
Updated to my comments (3.35 KB, patch)
2008-07-09 07:31 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
mrbkap: review+
jst: superreview+
dveditz: approval1.9.0.5+
Details | Diff | Review
Backport to 1.8 branch (5.61 KB, patch)
2008-10-30 11:51 PDT, Boris Zbarsky [:bz] (Out June 25-July 6)
mrbkap: review+
dveditz: approval1.8.1.19+
asac: approval1.8.0.next+
Details | Diff | Review

Description Daniel Holbert [:dholbert] (largely AFK until June 28) 2008-04-23 16:08:17 PDT
Created attachment 317421 [details]
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] (largely AFK until June 28) 2008-04-23 16:11:22 PDT
I see the hang & crash on Ubuntu Linux 8.04 as well, btw.
Comment 2 Daniel Holbert [:dholbert] (largely AFK until June 28) 2008-04-23 16:27:27 PDT
(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] (remove +bmo to email) 2008-04-23 17:41:14 PDT
#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] (remove +bmo to email) 2008-04-23 19:55:43 PDT
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 2008-04-27 23:28:44 PDT
Created attachment 318097 [details] [diff] [review]
cave in the tunnel
Comment 6 Blake Kaplan (:mrbkap) (please use needinfo!) 2008-04-30 16:35:45 PDT
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 2008-05-01 15:13:00 PDT
Did this get fixed via some other route?  Is there a reason the patch here has r+ but no a?
Comment 8 Brian Crowder 2008-05-29 13:46:42 PDT
timeless:  What's the status here?
Comment 9 Martijn Wargers [:mwargers] (not working for Mozilla) 2008-06-24 14:47:40 PDT
*** Bug 441631 has been marked as a duplicate of this bug. ***
Comment 10 Daniel Veditz [:dveditz] 2008-07-09 01:20:56 PDT
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) (please use needinfo!) 2008-07-09 07:31:38 PDT
Created attachment 328673 [details] [diff] [review]
Updated to my comments

This is updated to my comments (and to compile). jst, what do you think?
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2008-07-10 16:51:18 PDT
Comment on attachment 328673 [details] [diff] [review]
Updated to my comments

sr=jst
Comment 13 Blake Kaplan (:mrbkap) (please use needinfo!) 2008-07-14 06:08:07 PDT
Pushed for timeless as changeset 39aeb2ccb067.
Comment 14 sibbl 2008-07-15 10:30:15 PDT
i'm using 2008071504 where this bug should be fixed and it's still crashing?!? vista sp0 ...
Comment 15 Blake Kaplan (:mrbkap) (please use needinfo!) 2008-07-15 10:41:43 PDT
Do you have a stack trace or breakpad report?
Comment 16 sibbl 2008-07-15 11:18:47 PDT
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 [:bz] (Out June 25-July 6) 2008-10-30 11:37:53 PDT
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 [:bz] (Out June 25-July 6) 2008-10-30 11:51:40 PDT
Created attachment 345533 [details] [diff] [review]
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) 2008-10-30 13:42:35 PDT
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 [:bz] (Out June 25-July 6) 2008-10-30 13:47:42 PDT
All good.  I think taking this in 1.9.0.5 is perfectly reasonable
Comment 21 Daniel Veditz [:dveditz] 2008-11-03 11:30:41 PST
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 22 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-11-04 14:46:27 PST
*** Bug 441670 has been marked as a duplicate of this bug. ***
Comment 23 Boris Zbarsky [:bz] (Out June 25-July 6) 2008-11-06 11:49:56 PST
Fixed on both branches.
Comment 24 [On PTO until 6/29] 2008-11-25 15:09:18 PST
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 [On PTO until 6/29] 2008-12-01 12:03:46 PST
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 [On PTO until 6/29] 2008-12-01 18:10:21 PST
The Crash ID for comment 25 is: bp-f0e057dd-1135-4367-a9e9-6dcb62081201
Comment 27 timeless 2008-12-01 23:51:23 PST
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 [On PTO until 6/29] 2008-12-02 14:55:56 PST
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 [On PTO until 6/29] 2008-12-02 15:45:37 PST
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 2008-12-02 20:32:00 PST
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 2008-12-16 00:45:32 PST
Comment on attachment 345533 [details] [diff] [review]
Backport to 1.8 branch

a=asac for 1.8.0 branch
Comment 32 Jesse Ruderman on Windows 2009-09-14 20:31:15 PDT
*** Bug 401814 has been marked as a duplicate of this bug. ***

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