Closed Bug 430574 Opened 17 years ago Closed 16 years ago

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

Categories

(Core :: General, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: timeless)

References

()

Details

(5 keywords, Whiteboard: [sg:nse dos])

Attachments

(3 files, 1 obsolete file)

Attached file 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...(?)
I see the hang & crash on Ubuntu Linux 8.04 as well, btw.
Severity: normal → critical
Keywords: crash, hang, testcase
Summary: Testcase hangs firefox, fills memory, & crashes → Infinite document.write() loop hangs, fills memory, and crashes
(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
Attached patch cave in the tunnel (obsolete) — Splinter Review
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+
Did this get fixed via some other route? Is there a reason the patch here has r+ but no a?
timeless: What's the status here?
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]
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+
Assignee: mrbkap → timeless
Pushed for timeless as changeset 39aeb2ccb067.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
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?
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+
Blocks: 401814
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?
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?
All good. I think taking this in 1.9.0.5 is perfectly reasonable
Attachment #345533 - Flags: review?(mrbkap) → review+
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+
Fixed on both branches.
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.
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?
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 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+
Flags: blocking1.8.0.next+
Flags: wanted1.9.1?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: