Closed Bug 31041 Opened 25 years ago Closed 7 years ago

Add quota on script-opened windows (DOS prevention)

Categories

(Core :: Security, defect, P3)

Product:
Core
Component:
Security
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME
Milestone:
Future

People

(Reporter: junruh, Assigned: jst)

References

(
URL
)

Details

(Keywords: hang, Whiteboard: [sg:dos][expired?])

Attachments

(1 file, 1 obsolete file)

Endless loop testcases
(deleted), text/html
Details
1) With javascript enabled, visit http://junruh/crash.html. What happens: Windows - an endless number of main windows opens. Linux - an endless loop occurs of opening the same window. To view the source, visit that site with javascript disabled.
I don't get this endless loop (except on 4.7 which is a different story), instead an assert fires: ###!!! ASSERTION: null ptr: 'nsnull != aContext', file F:\client\mozilla\view\src\nsViewManager2.cpp, line 248 ###!!! Break: at file F:\client\mozilla\view\src\nsViewManager2.cpp, line 248 With this stack trace: NTDLL! 77f7629c() nsDebug::Assertion(const char * 0x024a11fc, const char * 0x024a11e8, const char * 0x024a11b8, int 0x000000f8) line 189 + 13 bytes nsDebug::PreCondition(const char * 0x024a11fc, const char * 0x024a11e8, const char * 0x024a11b8, int 0x000000f8) line 282 + 21 bytes nsViewManager2::Init(nsViewManager2 * const 0x0351e2b0, nsIDeviceContext * 0x00000000) line 248 + 32 bytes DocumentViewerImpl::MakeWindow(void * 0x00000000, const nsRect & {...}, nsScrollPreference nsScrollPreference_kAuto) line 887 + 41 bytes DocumentViewerImpl::Init(DocumentViewerImpl * const 0x03546e80, void * 0x00000000, nsIDeviceContext * 0x00000000, const nsRect & {...}, nsScrollPreference nsScrollPreference_kAuto) line 470 + 20 bytes nsDocShell::SetupNewViewer(nsDocShell * const 0x03487d80, nsIContentViewer * 0x03546e80) line 2041 + 87 bytes nsWebShell::SetupNewViewer(nsWebShell * const 0x03487d80, nsIContentViewer * 0x03546e80) line 813 + 13 bytes nsDocShell::CreateContentViewer(nsDocShell * const 0x03487d80, const char * 0x0012ac38, int 0x00000000, nsIChannel * 0x035301c0, nsIStreamListener * * 0x0012ac78) line 1911 + 21 bytes nsWebShell::DoContent(nsWebShell * const 0x03487e74, const char * 0x0012ac38, int 0x00000000, const char * 0x10085580 gCommonEmptyBuffer, nsIChannel * 0x035301c0, nsIStreamListener * * 0x0012ac78, int * 0x0012ac1c) line 1643 + 38 bytes nsDocumentOpenInfo::DispatchContent(nsIChannel * 0x035301c0, nsISupports * 0x00000000) line 389 + 109 bytes nsDocumentOpenInfo::OnStartRequest(nsDocumentOpenInfo * const 0x03531e40, nsIChannel * 0x035301c0, nsISupports * 0x00000000) line 252 + 16 bytes nsCachedChromeChannel::HandleStartLoadEvent(PLEvent * 0x03531df0) line 403 PL_HandleEvent(PLEvent * 0x03531df0) line 556 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x032d1ef0) line 501 + 9 bytes _md_EventReceiverProc(HWND__ * 0x6d5f0106, unsigned int 0x0000c0c1, unsigned int 0x00000000, long 0x032d1ef0) line 1011 + 9 bytes USER32! 77e71820() 032d1ef0()
Assignee: rogerl → kmcclusk
Component: Javascript Engine → Views
QA Contact: rginda → petersen
It is going into a endless loop with 4/7/2000 build on WINNT. Here is the document: <!doctype html public "-//w3c//dtd html 4.0 transitional//en"> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <meta name="GENERATOR" content="Mozilla/4.73b1 [en] (WinNT; U) [Netscape]"> <meta name="Author" content="John Unruh"> </head> <body onLoad="startclock()"> &nbsp; <br>&nbsp; <p><script><!-- var secsleft = 100 var timerID = null var timerRunning = false function Bomb() { var iCounter = 1 // dummy counter while (true) { window.open("http://home.netscape.com","CRASHING" + iCounter,"width=1,height=1,resizable=no") iCounter++ } } function stopclock(){ if(timerRunning) clearTimeout(timerID) timerRunning = false } function startclock(){ // Make sure the clock is stopped stopclock() showtime(3) } function showtime(){ var timeValue = " " + secsleft document.clock.face.value = timeValue secsleft -= 100000 if(secsleft>=0) { timerID = setTimeout("showtime()",10000000000000000000000000000000000000000000000000000000 000000000000000000000000000000000000000000000000000) timerRunning = true } else Bomb() } //--></script> <center> <h1> <font size=+4></font></h1></center> <pre><form NAME="clock" onSubmit="0"><font size=+4>&nbsp;&nbsp; <input TYPE="text" NAME="face" SIZE=3 VALUE =""></font><b><font size=+1>&nbsp; &nbsp;<font color="#FFFFFF">----------</font></font></b></pre> <pre><b><font color="#FFFFFF"><font size=+2>--------- </font></font><font size=+1>&nbsp;</font></b> </form></pre> </body> </html> It goes into a endless loop creating windows in function Bomb(). Not a view problem. Is JavaScript suppose to detect these tight loops and prevent them from locking up the browser? Reassigning to JavaScript Engine module owner
Assignee: kmcclusk → rogerl
Component: Views → Javascript Engine
QA Contact: petersen → pschwartau
I checked with the JS Engine team on this. The JS Engine provides an API at the bottom of any loop. Specifically, jsapi.h has JS_SetBranchCallback for setting the branch callback function. It is up to the embedding to check it and decide whether or not to terminate the loop...it's not an Engine issue. In any case, Mozilla no longer seems to be creating this endless loop when I go to the above URL (http://junruh/crash.html). So I'm going to mark it as "WorksForMe". I tried this both on WinNT and Linux. Using: Mozilla debug Linux and Windows builds made on 06/14/00. OS: Linux Red Hat 2.2.12-20smp XTerm = XFree86 3.3.3.1b(88b) OS: WinNT 4.0 (SP5) Mitch, would you like to review this from a Security perspective? Thanks -
Assignee: rogerl → mstoltz
Let's keep this one open as a placeholder for trivial denial-of-service such as opening windows in an infinite loop, or with infinite recursion, etc. I know these attacks exist, and I know they're tricky to fix, but they need to be addressed at some point. Marking M18 for post-Beta2.
Status: NEW → ASSIGNED
Component: Javascript Engine → Security: General
Target Milestone: --- → M18
*** Bug 38983 has been marked as a duplicate of this bug. ***
Future.
Target Milestone: M18 → Future
Group: netscapeconfidential?
Blocks: 30942
Keywords: hang
Setting default QA -
QA Contact: pschwartau → bsharma
Classic placed a quota on the number of windows that could be opened from JS, which was 100 based on testing with native front-ends, but which for XUL windows should probably be 20 or something like that. See http://lxr.mozilla.org/classic/ident?i=lm_window_count. Jst, you interested in hacking this quota in soon? /be
Attached file Endless loop testcases (deleted) —
I don't know if this is the right place for this testcase. This is definately not something that a professional piece of software should crash on. The last one rapidly increases the memory to a crazy amount.
Who said we were professionals? There are a near-infinite number of DoS attacks. Please don't attach any more. The right bug to help get fixed (by testing the patch, improving it, etc.) for infinite loop policing is bug 13350. /be
Brendan: When someone is working on their web page and accidentally create an endless loop that isn't a denial of service attack though a denial of service attack could use endless loops, it is not the only occasion when this is a problem. That is more what I am worried about. The average javascript writer that accidentally makes a mistake and has to reload the browser because it has crashed.
Yes, accidents are not attacks. Still, the problem of distinguishing a runaway from a long but converging script is hard. I'm pretty sure our conversations in this bug are not helping. I don't have a better suggestion than to help jst dust off that patch in bug 13350 for mozilla1.0 -- why not help do that? /be
Sure, do you want me to test the patch and find situations it doesn't handle?
*** Bug 149558 has been marked as a duplicate of this bug. ***
*** Bug 182870 has been marked as a duplicate of this bug. ***
OS: Windows NT → All
*** Bug 235469 has been marked as a duplicate of this bug. ***
nallen fixed up and landed bug 13350's patch. If it doesn't help, then this bug must be asking for a window.open quota. If so, it should be resummarized. In any event, it should be reassigned. /be
Assignee: security-bugs → nobody
Status: ASSIGNED → NEW
Comment 14 and comment 15 seem to be talking about different bugs than the lack of a window quota bug. Why were they dup'd? Duping based on a vague, general idea of a symptom, or a bag of symptoms, is not a good idea. Bugs should describe individual problems in the code. Reporting symptoms is fine, but eventually the causes become clear, and metabugs tracking separate bugs, one per problem in the code, should be filed. The original bug could morph into a metabug. I'm not sure we need one here, though. /be
As I test Mozilla 1.7 alpha, I've remarked that *somehow* the popup manager got "more intelligent" and starting blocking some kinds of recursive popups. Still, I think some progress could be made. I know this is a very basic hacking problem, and its solution is very probably the popup blocker. Perhaps recursive sites could be detected by the popup blocker and get "blocked" until you want them back. Later on, it the site re-starts creating this kind of popups, the popup blocker could start blocking it again... etc.
danm-moz lives for this kind of bugs! /be
Assignee: nobody → danm-moz
*** Bug 261575 has been marked as a duplicate of this bug. ***
Assignee: danm.moz → nobody
Given that javascript cpu/memory exhaustion will hang/crash the browser, why is this bug not Severity: Critical? Also, what is the purpose of this bug? Is it a Tracking bug (if not I would like to start one), or only a specific case, or should all known JS loop bugs be Duped here? p.s. the latest IE exploit loop causes Firefox to hang in OSX and crash in WinXP. http://computerterrorism.com/research/ie/poc.htm
Assignee: nobody → dveditz
QA Contact: bsharma → toolkit
> p.s. the latest IE exploit loop causes Firefox to hang > http://computerterrorism.com/research/ie/poc.htm Bug 317334, completely unrelated to javascript loops. As currently summarized this is a uselessly overgeneralized bug. As suggested by Brendan in comment 17 I'm morphing back toward the original testcase, add window.open() quotas. We should include modal alert() etc in the quota, "while (true) alert('ha ha ha');" is far too easy. Our current unresponsive script detection misses that case (we're perfectly responsive, just stuck), and you lose the window you're working on. Other windows remain responsive and you can open new windows from the OS, but if you were working through a large tab-set you've just lost them all.
Assignee: dveditz → jst
Summary: Javascript can create an endless loop → Add quota on script-opened windows (DOS prevention)
Whiteboard: [sg:dos]
if you have venkman or jsconsole you can kill the script (although i'd rather not try using jsconsole to do it).
There are various sites that try to prevent you from shutting down the browser by popping windows faster than you can close them. This should be fixed either by a quota or by a velocity limit, e.g. JS is not allowed to create more than 5 windows in any 15 second period.
A per-click quota is one way to fix this, but other there are other possible solutions. Firefox could show the "slow script" dialog, or it could show a similar dialog specific to opening lots of windows: "A script on this page is attempting to open a zillion windows. Do you want to open them all? [Stop script] [Open a zillion windows]"
What's left here? All three tests in the attached testcase are stopped by the stop script dialog (besides the third which results in memory allocation error - no crash or freeze). Popups are capped at 20, even if they're allowed for the site. Bug 61098 should cover prompts, anything else I'm missing? Note: I'm not 100% sure what this bug is about, seeing as the url in the description of this bug has expired and without it there really isn't much else to go on...
Whiteboard: [sg:dos] → [sg:dos][expired?]
natch: expired isn't precisely accurate, it was an mcom local domain host.
Attachment #8757575 - Attachment is obsolete: true
Attachment #8757575 - Attachment is patch: false
Flags: needinfo?(qlwlgfgioq)
We should file new bugs with working testcases for new symptoms. We have many such bugs filed so this one isn't helping anymore.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: