331658 - When I click "stop script", cancel any setTimeout/setInterval timers associated with the page

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect, P5)

Description

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

It's very easy to DoS moz with a combination of setInterval and bad code in the function called (e.g. infinite loops).  When the slow script dialog comes up and you click "stop script", timers should be killed too.

Comment 1

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

We already do this when the user navigates away from a page or such, right?

Comment 2

[Brendan Eich [:brendan]]

Please use the right component.

/be

Comment 3

[Brendan Eich [:brendan]]

IIRC, mrbkap and others (bclary?) have found setInterval calls that spawn the same setInterval, making for exponential fun.  IE seems to suppress these bogus calls (which probably arose when someone "ported" a setTimeout chain to setInterval).  If so, we should too.

/be

Comment 4

[Bob Clary [:bc] (inactive)]

Attachment: recursive setinterval testcase

IE6 does the same as us and will hang on this test case.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It was me who ran into this issue when spidering random pages. There are two issues here:

1. We might want to cancel all currently pending timers for the page when the
   you click "stop script".

2. We need to deal better with pages spawning too many timers to avoid the
   timers from filling up the eventqueue.


The second issue is sort of tricky. I talked a bit with stuart about putting a cap on how many timers a page is allowed to have the other day and he made several good points.

Just putting a cap on how many timers a page is allowed to have is not a good solution since those timers might be firing once a day or once an hour (think a calender or a todo list).
And putting a cap on how many timers are allowed to fire a second is not enough either. If the handler for an timer takes half a second and the timer fires 10 times a second we're still going to create a big backlog on the event queue.

What we could limit is how many timer events a page is allowed to have pending on the event queue. Once a single page has more then N timers waiting on the event queue we could pause all timers and intervals until all pending timers has fired.

N could probably be fairly small, like 10, since we don't actually cancel any timers, we just delay them.

I'm not sure how doable is with the current timer code though.

Comment 6

[Jesse Ruderman]

The last few paragraphs of Sicking's comment probably belong in bug 261633.

Comment 7

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Could someone provide info on what code would need to change to kill timers when the "stop script" button is clicked?

Comment 9

[Jim Michaels]

*** Bug 529237 is sort of a duplicate of this bug *** , however, there is a difference.  my runaway recursive script uses no timers and renders FF useless.  you must kill FF with task mgr. can't debug script, can't stop script.  it does this by recursing into child nodes while inserting IMG elements into .innerHTML under certain conditions, albeit wrong conditions.  it is a UL+LI menu tree.

Comment 12

[u528881]

I now have a script generated with Emscripten where I need to press "Stop script" 3 times to actually stop it. This happens in both Nightly and version 35, both in 64 bit Linux. I'm guessing this bug is responsible, so I'm not going to report a new bug.

If I instead press "Debug script" in version 35, the script actually stops. That's not a perfect workaround though, because opening the debugger for a large script takes some time.

Comment 13

[sworddragon2]

Attachment: Infinite loop that starts over and over if stopped

I have found a similar way that would require to stop all scripts of a site (or in my opinion even better: Providing an additional button in the dialog to stop all scripts).

The testcase uses only 1 timer without recursion. Firefox will ask to stop the script after a short time but doing so will result that the script gets started immediately again.

Comment 14

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.

Comment 15

[Andrei Purice]

Hey sworddragon,
Can you still reproduce this or should we close it? I used your script and after Firefox asks to stop and I press the stop button the script doesn't start again. Is this fixed by any chance?

Comment 16

[sworddragon2]

That is actually quite an unexpected question after all those years as I have no clue anymore what this issue actually was about especially in detail. Unfortunately I have to pass here as I probably could give not a reliable answer now. But I guess things can be kept simple: If the issue reported here can't be reproduced anymore and all seems fine this ticket can be simply closed.

Comment 17

[Andrei Purice]

Marking this as Resolved > Worksforme based on the last comment.
If anyone can still reproduce it on any of the latest Firefox versions please re-open or file a new issue.