1208950 - single click allows infinity pop ups

Status: RESOLVED DUPLICATE of bug 675574

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

Description

[Jun (Request needinfo for comment, I ignore other updates)]

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

Steps to reproduce:

Go to http://szabist.web.fc2.com/crash.html and click the link.


Actual results:

10000 pop ups


Expected results:

only 1 pop ups and ignore after 2nd.

Comment 1

[Jun (Request needinfo for comment, I ignore other updates)]

data:text/html;charset=utf-8,<a href%3D"%23" onclick%3D"go()">click<%2Fa>%0D%0A<script>%0D%0Afunction go(){%0D%0Afor(var a%3D0%3Ba<3%3Ba%2B%2B){%0D%0Awindow.open("https%3A%2F%2Fwww.google.com%2F")%3B%0D%0A}%0D%0A}%0D%0A<%2Fscript>

Maybe above it more easy one to identify. If you open it and click using Chrome, it will open only 1 and block other 2 pop ups. But Firefox opens all 3 pop ups.

This is advantage for an attacker who wants to exploit XSS or SOME(Same Origin Method Execution) using opener. If attacker can generate multiple pop ups by 1 click, they can manipulate multiple actions of users. FYI(http://files.benhayak.com/Same_Origin_Method_Execution__paper.pdf)

Comment 2

[Jun (Request needinfo for comment, I ignore other updates)]

Here we go the super hang which needs Task manager to close Firefox.

data:text/html;charset=utf-8,<script>while(1){window.open("https://www.google.com/");}</script>

Comment 3

[Al Billings [:abillings - ex-MoCo]]

[Tracking Requested - why for this release]:

Comment 4

[Jun (Request needinfo for comment, I ignore other updates)]

Hi, :dveditz

Can you explain me why is it sec-low? Definition of sec-moderate says "temporary DoS of the user's system, requiring reboot". Please check below.

data:text/html;charset=utf-8,<a href%3D"%23" onclick%3D"go()">click<%2Fa>%0D%0A<script>%0D%0Afunction go(){%0D%0Awhile(1){%0D%0Awindow.open("https%3A%2F%2Fwww.google.com%2F")%3B%0D%0A}%0D%0A}%0D%0A<%2Fscript>

Comment 5

[Jun (Request needinfo for comment, I ignore other updates)]

And please check comment 1.

In Same Origin Method Execution, number of window means number of user action can be simulated. As Firefox allow infinity popups by single click, it gives huge advantage on exploiting SOME. Which I think "Vulnerabilities which can provide an attacker positioning that could be used in combination with other vulnerabilities".

Comment 6

[:Gijs (he/him)]

(In reply to s.h.h.n.j.k from comment #4)
> Hi, :dveditz
> 
> Can you explain me why is it sec-low? Definition of sec-moderate says
> "temporary DoS of the user's system, requiring reboot". Please check below.
> 
> data:text/html;charset=utf-8,<a href%3D"%23"
> onclick%3D"go()">click<%2Fa>%0D%0A<script>%0D%0Afunction
> go(){%0D%0Awhile(1){%0D%0Awindow.open("https%3A%2F%2Fwww.google.
> com%2F")%3B%0D%0A}%0D%0A}%0D%0A<%2Fscript>

I am not dveditz, but the reboot for sec-severity purposes clearly refers to a system (not just Firefox) reboot, and "indefinite" denial of service, which is not the case here. Things like "prevent the user from starting Firefox", which this doesn't do.

I also don't actually see how this is substantially different from bug 685828 which is public.

We might be able to fix this specifically in the popup blocker code, as Chrome has clearly done, but we'd still be left with bug 685828.

Comment 7

[Jun (Request needinfo for comment, I ignore other updates)]

:Gijs

I understood about DoS part but what about combination with SOME?

Regarding bug 685828, it's different then this. I think problem in bug 685828 is that somehow it goes inside infinity loop before Firefox blocks it by popup blocker. As you can see below, it does block popup with out user click.

data:text/html;charset=utf-8,<script>window.open("https%3A%2F%2Fwww.google.com%2F")<%2Fscript>

But problem in this bug is that Firefox does not have limit of popups with user click where chrome decided to allow only 1.

Comment 8

[Daniel Veditz [:dveditz]]

Yes, the problem in this bug is that Firefox does not limit the number of popups that can be opened with a single qualifying event (e.g. click). That leads to user annoyance or a DoS which is sec-low.

The phrase you quoted about positioning for other vulnerabilities meant things like bugs that leak internal memory addresses, which are not interesting on their own but could be combined with some kinds of memory corruption bugs to create a successful exploit that was not possible with either bug alone.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> does not limit the number of popups that can be opened with a single qualifying event 

I thought we had such a limit, controlled by the "dom.popup_maximum" preference.

Comment 10

[Jun (Request needinfo for comment, I ignore other updates)]

Great info!

"The number of pop-ups to allow from a single non-click event. (Default: 20)"
(http://kb.mozillazine.org/Dom.popup_maximum)

Looks like there is maximum popups set to 20 for "non-click event" but this bug uses a click.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Ah, we should consider enforcing a similar limit for click.

Comment 12

[Jesse Ruderman]

Yeah, if we aren't going to reduce it to 1 window per click (bug 675574), we should at least reduce it to 20 windows per click.

Comment 13

[:Gijs (he/him)]

Can we just remove the specialcasing for click and keypresses that use enter/space from the code in the popupblocker and always use openControlled? What'd break? (ie https://dxr.mozilla.org/mozilla-central/source/dom/events/Event.cpp#831 )

Comment 14

[Boris Zbarsky [:bzbarsky]]

> What'd break?

The problem, last I checked, is that the 20 limit is global.

That is, once you had 20 popups open from 20 separate clicks, the 21st click would fail to open anything at all, no?

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Gijs, Bz: This bug is tracked for 44 and hasn't had any activity for ~6 weeks. Can you please help find an owner? This also has a sec rating so a fix would be nice.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Why is this tracked for 44?  In my opinion it shouldn't be.  It's not a new problem, the fix will be very regression-prone (in the sense of some sites no longer working, pretty much no matter what we change about our popup blocker), and it's not even clear what the "right" behavior here is.

This could use an owner, for sure, and targeting 46 might be reasonable.

As for who should own this, that's a question for jst or someone else who knows who might have time on their hands....

Comment 17

[:Gijs (he/him)]

(In reply to Ritu Kothari (:ritu) from comment #15)
> Gijs, Bz: This bug is tracked for 44 and hasn't had any activity for ~6
> weeks. Can you please help find an owner? This also has a sec rating so a
> fix would be nice.

It's sec-low and not a regression. While I would like to see this fixed, I am busy with a number of other higher-prio things, and it's unlikely I will have time for it in the near future. I concur with bz's comment #16.

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Boris Zbarsky [:bz] from comment #16)
> Why is this tracked for 44?  In my opinion it shouldn't be.  It's not a new
> problem, the fix will be very regression-prone (in the sense of some sites
> no longer working, pretty much no matter what we change about our popup
> blocker), and it's not even clear what the "right" behavior here is.
> 
> This could use an owner, for sure, and targeting 46 might be reasonable.
> 
> As for who should own this, that's a question for jst or someone else who
> knows who might have time on their hands....

Fair enough. I'll set this to wontfix for FF44. I hope the security team is ok with this. Please let me know if there are any concerns.

Comment 19

[Johann Hofmann [:johannh]]

This is a dupe of bug 675574. Can someone unhide this? :)

Comment 20

[Boris Zbarsky [:bzbarsky]]

> This is a dupe of bug 675574.

It is?  Our design behavior right now is that 20 popups be allowed.  Changing that to "1" is bug 675574.  If 10000 popups are opening, that's a different bug that says our check for "20" is being bypassed, no?  And the check for "1" would be bypassed the same way, I'd think.

In any case, there is a non-public problem here, which is that more than 20 popups are being opened.  Someone needs to actually debug this testcase to see why.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Though it's possible that things coming from "click" are not treated as a popup at all, hence not subject to the "20" limit.

Comment 22

[Boris Zbarsky [:bzbarsky]]

And maybe a good start would be to treat the 2nd and later open from a click as "popup".

Comment 23

[Johann Hofmann [:johannh]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #20)
> > This is a dupe of bug 675574.
> 
> It is?  Our design behavior right now is that 20 popups be allowed. 
> Changing that to "1" is bug 675574.  If 10000 popups are opening, that's a
> different bug that says our check for "20" is being bypassed, no?  And the
> check for "1" would be bypassed the same way, I'd think.
> 
> In any case, there is a non-public problem here, which is that more than 20
> popups are being opened.  Someone needs to actually debug this testcase to
> see why.

Ah, apologies for not reading this bug correctly, but the testcase in bug 675574 easily allows me to open more than 20 popups just by opening them in a loop, without a bypass, which made me think it's the same problem.

https://bug675574.bmoattachments.org/attachment.cgi?id=549722

I don't understand how dom.popup_maximum works yet, but it seems like it never protected against opening popups in a loop and/or from event handlers (or that broke a long time ago). I'm going to work my way through this code now, but do you know against what it's supposed to protect, off-hand?

Comment 24

[Johann Hofmann [:johannh]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #14)
> > What'd break?
> 
> The problem, last I checked, is that the 20 limit is global.
> 
> That is, once you had 20 popups open from 20 separate clicks, the 21st click
> would fail to open anything at all, no?

Ok, my answer is here.

Comment 25

[Boris Zbarsky [:bzbarsky]]

> but it seems like it never protected against opening popups in a loop

It certainly does in general (but possibly not for a left-click event).

Note that for popup purposes left-click is very special compared to all other events.