52526 - Site goes into loop opening infinite number of windows

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P2)

Description

[Alistair Vining]

- Browse to http://www.nationalreview.com/ with cookies set to "ask".
- Say no when the site asks to set a cookie and remember this decision.

Either a) The browser keeps putting up more requests to set cookies
or b) it keeps opening more and more windows.

Result perhaps depends on whether the pop-up has appeared when the cookies are
refused.

There's some cookie-handling javascript on the page which is probably connected,
but I'm pleading ignorance and laziness.

Browser version 2000-09-13 08:00

Comment 1

[Decklin Foster]

Seeing this w/ linux CVS from 091321. If cookies are disallowed (including using
the dialog for the first visit), I get an infinite number of popups. If they are
allowed, I see only one.

I also get 3 modal "allow cookie" dialogs on the first visit, all at once
(that's probably a seprate issue and I'm unsure if it has been filed or not.)

Comment 2

[Stephen P. Morse]

I don't think this is a cookie problem.  I'm not seeing an infinite number of 
requests to set cookies -- I see only about three such requests each from 
different hosts so that's understandable.  I am seeing the browser opening more 
and more windows which appear to be related to the content.  Furthermore, I see 
the same problem if I restore the cookie-warning pref to its default value (no 
cookie warning box).

Comment 3

[Alistair Vining]

Yes, there could be two separate issues here.  The multiple 'accept cookies'
dialogs I can also see at:
http://www.linuxworld.com/linuxworld/lw-2000-09/lw-09-vcontrol_2.html
but only sporadically (3 from itworld.com).  I admit, I haven't tested the
various combinations of preferences systematically.

Comment 4

[Stephen P. Morse]

I am unable to reproduce the multiple-cookie problem at site 
http://www.linuxworld.com/linuxworld/lw-2000-09/lw-09-vcontrol_2.html.

I suggest that we separate this out into two bugs.  This bug is about the 
opening of infinite number of content windows which has nothing to do with 
cookies and is reproducible.  Open a new report on the repeated cookie-requests 
and indicate that it is intermittent (I have not been able to reproduce it).

Possibly the repeated cookie warning boxes has to do with the fact that the 
cookie warning box is not always modal (bug 51338) which I am currently working 
on.  If it were always modal, the browser would "freeze" until you dismissed the 
warning box so it would not be possible to get multiple simultaneous warning 
boxes.

Updating summary to reflect that this current box has nothing to do with 
cookies.

Comment 5

[Decklin Foster]

I am not so sure this has nothing to do with cookies. As I commented, If I edit
the cookie permissions so that this site is allowed, I only get one popup
window. Can anyone else reproduce that?

Comment 6

[Stephen P. Morse]

Yes, you are correct.  Setting the cookie pref to accept-all-cookies does not 
have the problem whereas rejecting all cookies does have the problem.

That doesn't necessarily mean that it is a cookie problem, but at least we know 
that somehow cookies are involved.  I'll investigate further.

Comment 7

[Daniel Veditz [:dveditz]]

Need info on what this is about and how severe, and if it really is a side 
effect of bug 51338 as postulated by Steve.  Infinite windows sounds like a P1, 
not a P3, or maybe P2 if it's only some people some of the time.

Comment 8

[Stephen P. Morse]

No, this bug has nothing to do with 51338.  I mentioned that as a possible 
explanation of the repeated cookie warning box.  But I have not been able to 
reproduce the repeated cookie warning boxes and, furthermore, this bug does not 
cover that situation (if that is still a problem, it should be opened as a 
separate bug).

As far as the severity goes, this is basically a denial-of-service attack except 
that the attack is coming from the browser itself rather than from a website.  
Or, to be more specific, this particular website has something in it that 
exploits a bug in the browser to generate the denial-of-service.  I'm 
investigating to find out what is unique about this site that it is resulting in 
this DOS.

Comment 9

[Alistair Vining]

I can verify the multiple-cookie-dialog part (and have a screenshot), but
noticed that there was a checkin to fix 51338 today, so I'll wait for tomorrow's
builds before filing another bug.

The more-and-more-windows part always seems to crash in necko.dll.  No idea if
that is significant.

Comment 10

[Stephen P. Morse]

Point of crash is not significant -- it's just indicative of a low-memory 
situation.  In fact, here's the stack trace.  Obviously some bug in our 
memory-allocation/deallocation when low-memory is encountered.

__sbh_free_block(tagHeader * 0x113e5b84, void * 0x166c2ea0) line 350 + 6 bytes
_realloc_base(void * 0x166c2ea0, unsigned int 84) line 101 + 13 bytes
realloc_help(void * 0x166c2ec0, unsigned int 48, int 1, const char * 0x00000000, 
int 0, int 1) line 636 + 16 bytes
_realloc_dbg(void * 0x166c2ec0, unsigned int 48, int 1, const char * 0x00000000, 
int 0) line 806 + 27 bytes
realloc(void * 0x166c2ec0, unsigned int 48) line 755 + 19 bytes
JS_realloc(JSContext * 0x166226f0, void * 0x166c2ec0, unsigned int 48) line 1366 
+ 14 bytes
js_AllocSlot(JSContext * 0x166226f0, JSObject * 0x11982ad8, unsigned long * 
0x0012b6a0) line 1627 + 20 bytes
js_NewScopeProperty(JSContext * 0x166226f0, JSScope * 0x166c1a50, long 37582864, 
int (JSContext *, JSObject *, long, long *)* 0x00281253 _JS_PropertyStub, int 
(JSContext *, JSObject *, long, long *)* 0x00281253 _JS_PropertyStub, unsigned 
int 1) line 477 + 20 bytes
js_SetProperty(JSContext * 0x166226f0, JSObject * 0x11982ad8, long 37582864, 
long * 0x0012b818) line 2221 + 29 bytes
nsXPCArbitraryScriptable::SetProperty(nsXPCArbitraryScriptable * const 
0x01a3bba0, JSContext * 0x166226f0, JSObject * 0x11982ad8, long 37582864, long * 
0x0012b818, nsIXPConnectWrappedNative * 0x166c1ce0, nsIXPCScriptable * 
0x00000000, int * 0x0012b814) line 132 + 25 bytes
nsXPCComponents_Classes::CacheDynaProp(JSContext * 0x166226f0, JSObject * 
0x11982ad8, long 37582864, nsIXPConnectWrappedNative * 0x166c1ce0, 
nsIXPCScriptable * 0x01a3bba0) line 548
nsXPCComponents_Classes::GetProperty(nsXPCComponents_Classes * const 0x166c1d84, 
JSContext * 0x166226f0, JSObject * 0x11982ad8, long 37582864, long * 0x0012c0b0, 
nsIXPConnectWrappedNative * 0x166c1ce0, nsIXPCScriptable * 0x01a3bba0, int * 
0x0012b8b4) line 420
WrappedNative_GetProperty(JSContext * 0x166226f0, JSObject * 0x11982ad8, long 
37582864, long * 0x0012c0b0) line 325 + 40 bytes
js_Interpret(JSContext * 0x166226f0, long * 0x0012c234) line 2404 + 1563 bytes
js_Execute(JSContext * 0x166226f0, JSObject * 0x11348060, JSScript * 0x02851b20, 
JSFunction * 0x00000000, JSStackFrame * 0x00000000, unsigned int 0, long * 
0x0012c234) line 903 + 13 bytes
JS_ExecuteScript(JSContext * 0x166226f0, JSObject * 0x11348060, JSScript * 
0x02851b20, long * 0x0012c234) line 3024 + 27 bytes
nsJSContext::ExecuteScript(nsJSContext * const 0x166228a0, void * 0x00d717a8, 
void * 0x11348060, basic_nsAWritableString<unsigned short> * 0x00000000, int * 
0x00000000) line 723 + 42 bytes
nsXULDocument::ExecuteScript(JSObject * 0x00d717a8) line 5631 + 33 bytes
nsXULDocument::LoadScript(nsXULPrototypeScript * 0x023a03b0, int * 0x0012c81c) 
line 5486 + 15 bytes
nsXULDocument::ResumeWalk() line 5269 + 19 bytes
nsXULDocument::CachedChromeStreamListener::OnStopRequest(nsXULDocument::CachedCh
romeStreamListener * const 0x1660ed70, nsIChannel * 0x165fe630, nsISupports * 
0x00000000, unsigned int 0, const unsigned short * 0x00000000) line 6567
nsDocumentOpenInfo::OnStopRequest(nsDocumentOpenInfo * const 0x165fe4a0, 
nsIChannel * 0x165fe630, nsISupports * 0x00000000, unsigned int 0, const 
unsigned short * 0x00000000) line 269
nsCachedChromeChannel::HandleStopLoadEvent(PLEvent * 0x1669ed40) line 535
PL_HandleEvent(PLEvent * 0x1669ed40) line 575 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x165fb480) line 508 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00010f9a, unsigned int 49426, unsigned int 0, 
long 375370880) line 1044 + 9 bytes
USER32! 77e71268()
165fb480()

Comment 11

[Daniel Veditz [:dveditz]]

Approving nsbeta3+, giving P2. Until we find out what about this site causes 
the problem we don't know how exploitable this is.

Has anyone thought to save the site somewhere (like an attachment to this bug) 
just in case they change it on us and "fix" the problem?

Comment 12

[Stephen P. Morse]

Yes, I've already saved the site, modified it, and just finished doing a 
complete analysis.  Here's the story.

First, the bug is not at all in the cookie module.  The reason that the failure 
occured when cookies were disabled is because the site tests for a cookie and if 
it doesn't find one it goes and opens a pop-up window.  So if cookies are 
enabled, the window does not pop up even one time and there are no "infinite" 
windows.

And "infinite" in this example is a slight exageration.  In fact, the site will 
open up 186 of these pop-up windows when cookies are disabled.  Here's why.

The html on the page calls on javascript to open the pop-up window when the page 
finishes loading (by using an onload handler).  The page itself consists of a 
bunch of images, each of which get loaded as well.  The problem is that the 
loading of each image is also triggering the onload handler.  And there are 186 
images on the page so the handler gets called 186 times and opens 186 pop-up 
windows.

The following html demonstrates the problem.  It was obtained by greatly 
simplifying the content on the site (place any gif file in the same directory as 
the html and call it bug.gif).

<html>
  <head>
    <title>Repeated calls to on-load hander</title>
    <script type="text/javascript" language="JavaScript">
      var count = 1;
      function Loaded() {
        alert("count="+count++);
      }
    </script>
  </head>
  <body onload="Loaded()">
    <img src="bug.gif"
         width="20" height="20"
         align="absbottom">
  </body>
</html>

The above example brings up an alert box where the actual site was bringing up a 
pop-up window.  In this example, the alert box is coming up twice, once for the 
page loading and once for the image loading.  And if you add more images into 
the above example, you will get the same amount of more alerts.

Now presumably this is a bug because the onload handler should only be called 
once (when the page itself finishes loading) and not when each image on the page 
finishes loading.  At least that's the way it is in 4.x.  If you run the above 
example in 4.x, you will get the alert only one time.

Comment 13

[Stephen P. Morse]

In rereading my above comment, I see that one paragraph is not very clear.  
Namely:

    The following html demonstrates the problem.  It was obtained by greatly
    simplifying the content on the site (place any gif file in the same
    directory as the html and call it bug.gif).

What I was trying to do was give instructions for running the sample html that I 
included.  Specifically, you need to first obtain a gif file (any old gif will 
do) and put it into the same directory as the sample html file, and call it 
bug.gif.  Then load the html file into the browser and you will see the 
problem.

Comment 14

[Stephen P. Morse]

So who is responsible for the onload handler?  Dan Veditz suggested I try joki.

Comment 15

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This is me. I made image load send load events. Currently load events bubble, 
and you should check event.target to know it was image that loaded, not the 
document. The specs state that load event should not bubble, so I am changing 
our implementation. A slight problem is that currently just about every event 
bubbles in Mozilla.

Comment 16

[Heikki Toivonen (remove -bugzilla when emailing directly)]

*** Bug 52585 has been marked as a duplicate of this bug. ***

Comment 17

[Stephen P. Morse]

I find it amazing as to how similar the two test cases are (the one presented 
in this report and the one in bug 52585), considering that the two tests were 
written independently by different people.

Comment 18

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Fix, do not bubble image load & some cleanup etc.

Comment 19

[Heikki Toivonen (remove -bugzilla when emailing directly)]

joki, can you review? I run into some difficulties with precheckin tests that I 
believe are unrelated... (AIM & view source crash on Linux, no AIM at all on 
NT). I have a pretty old tree so I am updating.

I added a new parameter to nsIPresShell::HandleEventWithTarget so that we can 
pass in flags. The can't bubble flag is set when we call this for the image load 
event. Note that the event still bubbles, we just don't call the event handlers 
for it. This weirdness is because at least at some point in time XUL needed all 
events to bubble.

This seems to fix the site in the URL as well as testcase in the duplicate bug.

In the patch I also fixed GetBubbles and GetCancelable methods of DOM event. And 
I fixed all the cases of wrong event objects/types that I came across (like 
someone using nsMouseEvent with NS_FORM_SELECTED or something). I did not do a 
search for this so there are bound to be more.

Comment 20

[Heikki Toivonen (remove -bugzilla when emailing directly)]

The correct event class and event message combos can be seen in 
nsDOMEvent::SetEventType.

http://lxr.mozilla.org/seamonkey/source/layout/events/src/nsDOMEvent.cpp#910

Comment 21

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fixed.

Comment 22

[Tom Everingham]

verified:
Win NT 2000092110
Linux rh6 2000092008