Open Bug 1045949 Opened 10 years ago Updated 2 years ago

Popup/reload interaction vulnerability

Categories

(Core :: DOM: Navigation, defect, P5)

Product:
Core
Component:
DOM: Navigation
Type:
defect

Tracking

()

Tracking Flags:
Tracking Status
firefox33 + wontfix
firefox34 --- affected
firefox35 --- affected
fennec + ---

People

(Reporter: rbsoulhunter17, Unassigned)

Details

(Keywords: csectype-dos, regression, reproducible) 

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.125 Safari/537.36

Steps to reproduce:

Firefox for android was prone to a pop up recursion vulnerability and hence resulting in a DOS as there is no way to for a victim to escape the popup recursions and return to the original browser window. 

Here is the proof of concept:

<html>
<head></head>
<title> Firefox Pop Up Recursion DOS</title>
<body>
<script>
window = window.open(location.reload('http://rafayhackingarticles.net'));
window.alert(window)
</script>
</body>
</html>



Actual results:


Firefox for android recursively opens up new window on pressing the ok button. This keeps on continuing until the browser cannot open up more tabs and is crashed. 


Expected results:

Firefox should have opened a new window with single alert box.
Sure, I guess. I'm able to reproduce. We have the 'prevent this from showing again' prompt, but a new window replaces it. I'm fairly sure this used to work properly back when the checkbox was added a long time ago.
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
Ever confirmed: true
Keywords: regression, regressionwindow-wanted, reproducible
OS: Windows 7 → Android
Hardware: x86_64 → ARM
Yes.As for chrome the window does not replaces it and hence we are able to close it up
Richard - Thoughts on how to stop this?
Assignee: nobody → rnewman
tracking-fennec: ? → +
I don't see the same thing as the reporter.

I see:

* A doorhanger (rapidly pushed into the shadows) saying "Fennec has prevented this site from opening a pop-up window".
* An ever-growing stream of alert overlays.
* Occasionally a dialog in the stream that offers to prevent more pop-ups.

Still a DOS, but it doesn't wait for a click.

Observations:

* location.reload() doesn't take an argument, so that shouldn't matter.
* It also doesn't return anything, so we're really calling window.open(undefined).

That is, the POC should be equivalent to:

  location.reload();
  window.open().alert("");

The content of the opened windows, then, isn't relevant.

Here's what I think happens:

* The page reload gets triggered. JS execution continues.
* A window.open call occurs. This triggers the various popup-blocking protections, such as the initial doorhanger. A value is returned immediately.
* window.alert is called.
* The alert overlays, preventing interaction with the page until dismissed.
* The reload completes, triggering another reload and another alert.
* The alerts stack, obscuring access to dialogs and controls. The alerts can stack because each one comes from a fresh window object.

Potential mitigations:

* window.open should block for the current window and all child windows until the pop-up doorhanger is dismissed. (This sounds like Chrome's approach.)
* Potentially: window.alert should queue or reject for child windows, such that each family tree can open one alert at a time.

I haven't yet tested whether the window.open is necessary for this attack; it might be that reloading invalidates some state on the window object such that it can be used for another alert, with the looping being provided by the reload.

In that case we'd need to ensure that alert state was maintained across reloads.
The same applies to desktop, and even in its most minimal form:

    location.reload();
    window.alert(window);

causes endless looping and reloading. You can click the alert as often as you want.

An alternative form:

    location.reload();
    window.open().alert(window);

causes endless looping of the yellow pop-up blocker bar, which is hilarious.

The original, the alternate form, and the minimal form are respectively:

  http://twinql.com/vuln/p.html
  http://twinql.com/vuln/p2.html
  http://twinql.com/vuln/p3.html
Component: General → Document Navigation
OS: Android → All
Product: Firefox for Android → Core
Hardware: ARM → All
Summary: Firefox Android Popup Recursion Vulnerability → Popup/reload interaction vulnerability
This is slightly less serious on desktop platforms with non-modal alerts, because of course you can always close the tab.
When I tried this on desktop I could not close the tab -- the navigation cleared the in-page alert dialog so it never paused for a click. Instead the looping seemed to lock up the browser chrome.
Keywords: csectype-dos
I tested it on Desktop browsers and it proved to be less serious, since i was able to close the tab by opening up a new tab as soon as the pop up occurred and then closing the previous one.
(In reply to Rafay Baloch from comment #8)
> I tested it on Desktop browsers and it proved to be less serious, since i
> was able to close the tab by opening up a new tab as soon as the pop up
> occurred and then closing the previous one.

The precise behavior probably differs between platforms, but the main conclusion from this is that this is something we should fix in the Gecko core, rather than in Fennec.
Makes sense.
Unassigning, 'cos this is far from my area of expertise.

Marking as tracking and backlog in the hope that someone else will work on it.


Logspam while this happens, in case that helps:

08-01 10:38:14.134 E/GeckoConsole(10344): [JavaScript Error: "too much recursion"]
08-01 10:38:14.464 E/GeckoConsole(10344): [JavaScript Error: "too much recursion"]
08-01 10:38:14.474 E/GeckoConsole(10344): [JavaScript Error: "too much recursion" {file: "chrome://browser/content/browser.js" line: 4202}]

08-01 10:38:14.084: E/GeckoConsole(10344): [JavaScript Error: "this.blockedPopups is null" {file: "chrome://global/content/bindings/browser.xml" line: 840}]
Assignee: rnewman → nobody
tracking-firefox33: --- → ?
Flags: firefox-backlog?
We attempt to avoid part of this in the (desktop) modal prompts, by tearing down / aborting any existing prompts when the document goes away...

toolkit/components/prompts/src/nsPrompter.js

399     domWin.addEventListener("pagehide", pagehide);
400     function pagehide() {
401         domWin.removeEventListener("pagehide", pagehide);
402 
403         if (newPrompt) {
404             newPrompt.abortPrompt();
405         }
406     }

I'm not sure how/if the old window-modal prompt code dealt with this.

But page-spanning tricks have been a pain before, too. (eg bug 636374)
(Not a Firefox bug, so doesn't belong in the Firefox backlog. Happy to put front-end mitigation bugs in the backlog, but it sounds like that's not something we want to pursue here.)
Flags: firefox-backlog?
I can poke at it this week and see how I get on.
Flags: needinfo?(gijskruitbosch+bugs)
Regression + dos, tracking
status-firefox33: --- → affected
status-firefox34: --- → affected
tracking-firefox33: ? → +
tracking-firefox34: --- → +
Apologies for the delay here.

(In reply to Daniel Veditz [:dveditz] from comment #7)
> When I tried this on desktop I could not close the tab -- the navigation
> cleared the in-page alert dialog so it never paused for a click. Instead the
> looping seemed to lock up the browser chrome.

I can't reproduce this on OS X. As rnewman said in comment 6, I can just close the tab because the alert dialogs are tab-modal. What platform did you test on?


(In reply to (Away Aug 14-24) Richard Newman [:rnewman] from comment #5)
> The same applies to desktop, and even in its most minimal form:
> 
>     location.reload();
>     window.alert(window);
> 
> causes endless looping and reloading. You can click the alert as often as
> you want.

Is this version (without window.open) an actual DoS issue for mobile? Because your suggested mitigations were:

(In reply to (Away Aug 14-24) Richard Newman [:rnewman] from comment #4)
> * window.open should block for the current window and all child windows
> until the pop-up doorhanger is dismissed. (This sounds like Chrome's
> approach.)
> * Potentially: window.alert should queue or reject for child windows, such
> that each family tree can open one alert at a time.

And both of these are dependent on the window.open factor, if I understand correctly. Which you anticipated, perhaps:

> I haven't yet tested whether the window.open is necessary for this attack;
> it might be that reloading invalidates some state on the window object such
> that it can be used for another alert, with the looping being provided by
> the reload.
> 
> In that case we'd need to ensure that alert state was maintained across
> reloads.

But as dolske noted in comment #12, page spanning attacks like this are tricky. With tab modal alerts and a simple: data:text/html,<script>alert('hi')</script>, I can interact with the location bar. If I then load a different page, I would expect (as a user) that the alert just goes away, not that it persists.

In fact, even if I had a page that, say, created an alert because I clicked a button, if I (as a user) then clicked the reload button, I would also expect the alert to go away. (of course, we might be able to treat a user-triggered page unload differently, but I expect that may be tricky)

Worse still, I suspect you can probably do the same (on mobile) if you have two pages which use location.href = "otherpage.html" to redirect to each other, plus the alert.

Which makes me think that the reload() thing is not the issue here per se (we could do all manner of similar things in roundabout ways with iframes and whatnot, probably). It's the alerts that are the cause of the DoS. So as far as I can tell, we should ensure in all our UAs that:

1) alerts do not block chrome interaction (ie are content-modal)

and one of

2a) only one alert can occur simultaneously for a single open tab (which might be what your second suggestion meant?)
or
2b) alerts should block navigation in their topmost-content docshell

Desktop on OS X, at least, seems to satisfy (1) and (2a). If we don't do this on all of desktop, it would be good to know where exactly this fails (hence needinfo dveditz).

Finally, needinfo'ing dolske in lieu of rnewman (who's on PTO) to sanity-check my thoughts above. :-)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(dveditz)
Flags: needinfo?(dolske)
I tested on windows, using the original testcase as a data: url

data:text/html,<html><head></head><title>%20Firefox%20Pop%20Up%20Recursion%20DOS</title><body><script>window=window.open(location.reload('http://rafayhackingarticles.net'));window.alert(window)</script></body></html>

I could, eventually, stop the loop by focusing in the addressbar and hitting ESC repeatedly.
Flags: needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #17)
> I tested on windows, using the original testcase as a data: url
> 
> data:text/html,<html><head></
> head><title>%20Firefox%20Pop%20Up%20Recursion%20DOS</
> title><body><script>window=window.open(location.reload('http://
> rafayhackingarticles.net'));window.alert(window)</script></body></html>
> 
> I could, eventually, stop the loop by focusing in the addressbar and hitting
> ESC repeatedly.

Odd. ctrl-w worked immediately for me (testing on 32 beta, Windows 8).

Redirecting the other needinfo to rnewman who should be back by now - see comment 16. :-)
Flags: needinfo?(dolske) → needinfo?(rnewman)
Mark, what is your opinion on this bug? I am not planning to block the 33 release for this bug but I would be happy to take a patch. Thanks
Flags: needinfo?(mark.finkle)
(In reply to Sylvestre Ledru [:sylvestre] from comment #19)
> Mark, what is your opinion on this bug? I am not planning to block the 33
> release for this bug but I would be happy to take a patch. Thanks

I agree with this approach.
Flags: needinfo?(mark.finkle)
I'm unlikely to have time in the near future to do a detailed analysis of Gijs's Comment 16 (and, indeed, I'm not the best person to do so -- I was just trying to make sure the ball didn't stop moving!).

If this needs to be fixed urgently, someone with more DOMish chops than me should pick this up.
Flags: needinfo?(rnewman)
beta 8 go to build is today, I guess it is too late for a fix => wontfix for 33.
status-firefox33: affected → wontfix
status-firefox35: --- → affected
tracking-firefox35: --- → +
(In reply to Richard Newman [:rnewman] from comment #21)
> I'm unlikely to have time in the near future to do a detailed analysis of
> Gijs's Comment 16 (and, indeed, I'm not the best person to do so -- I was
> just trying to make sure the ball didn't stop moving!).
> 
> If this needs to be fixed urgently, someone with more DOMish chops than me
> should pick this up.

jst - Can someone on your team help here?
Flags: needinfo?(jst)
filter on [mass-p5]
Priority: -- → P5
I reviewed this bug with jst and overholt. They suggest that smaug is the right person to investigate but that this is a lower priority than other work that is already in queue. I am dropping tracking as both the DOM and Android teams have set this as low priority.
tracking-firefox34: + → ---
tracking-firefox35: + → ---
Flags: needinfo?(bugs)
(In reply to Richard Newman [:rnewman] from comment #4)
> * The page reload gets triggered. JS execution continues.
> * A window.open call occurs. This triggers the various popup-blocking
> protections, such as the initial doorhanger. A value is returned immediately.
> * window.alert is called.
> * The alert overlays, preventing interaction with the page until dismissed.
> * The reload completes, triggering another reload and another alert.
> * The alerts stack, obscuring access to dialogs and controls. The alerts can
> stack because each one comes from a fresh window object.
This is on Mobile?
On linux the page just reloads, shows an alert, the alert goes away, new document is loaded, alert shown, and it goes away...
And one can just close the problematic tab

> 
> Potential mitigations:
> 
> * window.open should block for the current window and all child windows
> until the pop-up doorhanger is dismissed. (This sounds like Chrome's
> approach.)
Not really doable, at least not before e10s. We need to spin event loop for alert().


So is this mostly Mobile FF issue? Does it not close modal dialogs when the relevant page goes away?
Flags: needinfo?(bugs)
Flags: needinfo?(jst)
Still pretty effectively trashes Fennec, but I don't think a regression window is going to help much here at this point given the later discussion in the bug about how about to fix it.
Keywords: regressionwindow-wanted
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.