Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED

Status

()

Core
General
--
critical
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: dholbert, Assigned: timeless)

Tracking

(5 keywords)

Trunk
crash, fixed1.8.1.19, hang, testcase, verified1.9.0.5
Points:
---
Bug Flags:
wanted1.9.0.x +
wanted1.8.1.x ?
blocking1.8.0.next +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse dos], URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

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

Comment 1

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

Updated

9 years ago
Severity: normal → critical
Keywords: crash, hang, testcase
Summary: Testcase hangs firefox, fills memory, & crashes → Infinite document.write() loop hangs, fills memory, and crashes
(Reporter)

Comment 2

9 years ago
(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.
#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.
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
(Assignee)

Comment 5

9 years ago
Created attachment 318097 [details] [diff] [review]
cave in the tunnel
Attachment #318097 - Flags: review?(mrbkap)
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?
Attachment #318097 - Flags: review?(mrbkap) → review+

Comment 7

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

Comment 8

9 years ago
timeless:  What's the status here?

Updated

9 years ago
Duplicate of this bug: 441631
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.
Assignee: nobody → mrbkap
Group: security
Flags: wanted1.9.1?
Flags: wanted1.9.0.x?
Flags: wanted1.8.1.x?
Whiteboard: [sg:nse dos]
Created attachment 328673 [details] [diff] [review]
Updated to my comments

This is updated to my comments (and to compile). jst, what do you think?
Attachment #318097 - Attachment is obsolete: true
Attachment #328673 - Flags: superreview?(jst)
Attachment #328673 - Flags: review+
Comment on attachment 328673 [details] [diff] [review]
Updated to my comments

sr=jst
Attachment #328673 - Flags: superreview?(jst) → superreview+

Updated

9 years ago
Assignee: mrbkap → timeless
Pushed for timeless as changeset 39aeb2ccb067.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 14

9 years ago
i'm using 2008071504 where this bug should be fixed and it's still crashing?!? vista sp0 ...
Do you have a stack trace or breakpad report?

Comment 16

9 years ago
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 ...
Flags: wanted1.9.0.x? → wanted1.9.0.x+

Updated

9 years ago
Blocks: 401814

Comment 17

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

This applies cleanly to 1.9.0; we should just take this.
Attachment #328673 - Flags: approval1.9.0.5?
Attachment #328673 - Flags: approval1.9.0.4?

Comment 18

9 years ago
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.
Attachment #345533 - Flags: review?(mrbkap)
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.)
Attachment #328673 - Flags: approval1.9.0.4?

Comment 20

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

Updated

9 years ago
Attachment #345533 - Flags: review?(mrbkap) → review+

Updated

9 years ago
Attachment #345533 - Flags: approval1.8.1.19?
Attachment #328673 - Flags: approval1.9.0.5? → approval1.9.0.5+
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)
Attachment #345533 - Flags: approval1.8.1.19? → approval1.8.1.19+

Updated

9 years ago
Duplicate of this bug: 441670

Comment 23

9 years ago
Fixed on both branches.
Keywords: fixed1.8.1.19, fixed1.9.0.5
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.
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.
The Crash ID for comment 25 is: bp-f0e057dd-1135-4367-a9e9-6dcb62081201
(Assignee)

Comment 27

9 years ago
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?
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.
timeless, do you have a response to my remark in comment 24 about 1.8.1.18 versus 1.8.1.19?
(Assignee)

Comment 30

9 years ago
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.
Keywords: fixed1.9.0.5 → verified1.9.0.5

Comment 31

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

a=asac for 1.8.0 branch
Attachment #345533 - Flags: approval1.8.0.next+

Updated

9 years ago
Flags: blocking1.8.0.next+
Flags: wanted1.9.1?
Duplicate of this bug: 401814
You need to log in before you can comment on or make changes to this bug.