Open Bug 1176397 Opened 9 years ago Updated 2 years ago

popups blocked by iframe sandboxing should not trigger popup blocker infobar

Categories

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

41 Branch
defect

Tracking

()

People

(Reporter: bgstandaert, Unassigned)

Details

Attachments

(1 file)

Attached file test.html
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150618030206

Steps to reproduce:

(testcase attached).

1. Enable the popup blocker.
2. Create an iframe with sandbox="allow-scripts", which should block popups from being opened. 
3. Set the srcdoc of the iframe to "<script>window.open('http://example.com');</script>"
2. Run the page.
3. In the yellow bar generated by the popup blocker, click "preferences".


Actual results:

 - The popup blocker message is shown, even though the popup won't open anyway.

 - More concerning, the origin shown in the popup blocker message is for the sandboxed frame's container, and not the frame itself, even though the popup is coming from the frame. If, for example, an iframe embedded in "mysite.com" (with sandboxing) was generating the popup, the popup blocker would show "allow popups for "http://mysite.com". Then, below that it would show the address of the page that the sandboxed iframe was trying to open.

This could potentially trick the user into manually typing in the address of the popup window, believing it has come from the frame's container (example.com) instead of the sandboxed frame. In other words, the user could be tricked into opening a malicious website, bypassing the sandboxing rules on the iframe.


Expected results:

Ideally, the popup blocker window should never open, since the popup will never appear anyway. If that isn't possible, the "allow popups from..." message should make it clear that the popup is coming from the sandboxed frame and not the parent window.
Also, I will be going on vacation tomorrow, and will be gone for a week or so, so I won't be able to reply to this bug until I get back.
Christoph: do you know who works on iframe sandbox these days? It appears srcdoc content may not be fully sandboxed.
Flags: needinfo?(mozilla)
Product: Firefox → Core
(In reply to Daniel Veditz [:dveditz] from comment #2)
> Christoph: do you know who works on iframe sandbox these days? It appears
> srcdoc content may not be fully sandboxed.

I am pretty sure Bob Owen is.
Flags: needinfo?(mozilla)
Bob is working on both kinds of sandbox? I know he's working on the code execution sandbox, but the iframe sandbox is something entirely different.
Flags: needinfo?(mozilla)
Flags: needinfo?(bobowen.code)
(In reply to Daniel Veditz [:dveditz] from comment #4)
> Bob is working on both kinds of sandbox? I know he's working on the code
> execution sandbox, but the iframe sandbox is something entirely different.

iframe sandbox was the first thing I worked on, so I tend to pick these up, as I know the code fairly well.


(In reply to bgstandaert from comment #0)

> Actual results:
> 
>  - The popup blocker message is shown, even though the popup won't open
> anyway.

Looks like the popup blocker runs before the sandboxing checks and is unaware of the sandbox.
I don't know the popup code off hand, but I'll try and look into it.

(This doesn't appear to be srcdoc specific.)
 
>  - More concerning, the origin shown in the popup blocker message is for the
> sandboxed frame's container, and not the frame itself, even though the popup
> is coming from the frame. If, for example, an iframe embedded in
> "mysite.com" (with sandboxing) was generating the popup, the popup blocker
> would show "allow popups for "http://mysite.com". Then, below that it would
> show the address of the page that the sandboxed iframe was trying to open.
> 
> This could potentially trick the user into manually typing in the address of
> the popup window, believing it has come from the frame's container
> (example.com) instead of the sandboxed frame. In other words, the user could
> be tricked into opening a malicious website, bypassing the sandboxing rules
> on the iframe.

This appears to be the way that the popup blocker currently works and I don't think it is anything to do with the sandbox.
It appears to just go on the top-level domain (not origin - I've checked port but not protocol).
I've tested without a sandbox and the same thing happens.
I think that this is more of a UX question.

The popup blocker seems to currently work just off the top level browsing context domain.
(The code I found is in: browser/base/content/browser.js)

It looks like we might be able to make that code aware of the sandboxing, although I'm not sure we have the iframe document at that point to get the sandboxing flags.
Given that the blocker only checks against the top level, would we want the popup blocker to not be triggered when it comes from an iframe without allow-popups?
It is a bit confusing that you add a permission to allow the popup but then it doesn't open.

Even though the popup blocker shows the top level for adding permission, I think the permission is only added for that domain.
So, it would not subsequently allow popups on a normal page from the iframe's domain.

Maybe we should change the popup-blocker to work by origin and to handle (i)frames somehow, but I think that's a separate question to the sandboxing.
Flags: needinfo?(bobowen.code) → needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #4)
> Bob is working on both kinds of sandbox? I know he's working on the code
> execution sandbox, but the iframe sandbox is something entirely different.

It seems Bob is on it. Thanks Bob! Clearing my needinfo!
Flags: needinfo?(mozilla)
Your testcase is a little confusing because it uses srcdoc, which is by definition same origin with the page which creates it. There's nothing wrong or confusing about the message in that case (except your first complaint, that we shouldn't show the message at all because of the sandbox!).

In the case of a cross-origin frame you're right, the popup blocker message does go ahead and use the domain that the user can see in the URL bar, because as far as the user knows that's the only domain involved. If the user says "foo.com" can open popups, that extends to any domain the foo.com author has incorporated into the content she presents to the user as "foo.com". It's not strictly accurate, but it's good enough. That also means that even if the user has allowed popups on nice.com that does not extend if evil.com frames nice.com trying to get popups to show. The user will have to allow popups on evil.com to see them in that context.

The popup block infobar is client UI to let the user decide to open up popups the page wanted to show, but that Firefox blocked because we thought they could be abusive. In this case the page itself has blocked it's own popup and Firefox should not be showing the helpful UI. It's annoying but not a security problem that needs to be hidden.
Group: core-security
Component: Untriaged → DOM: Core & HTML
Flags: needinfo?(dveditz)
Summary: Misleading popup blocker message could trick users into opening malicious websites → popups blocked by iframe sandboxing should not trigger popup blocker infobar
Status: UNCONFIRMED → NEW
Ever confirmed: true
Note that per spec at https://html.spec.whatwg.org/multipage/browsers.html#the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name step 5, the UA is in fact required to do its internal popup blocking before checking the sandbox flags.  Note that the sandbox flags don't mean no new window; UAs are allowed to open one after checking with the user....

But yes, in practical terms until we implement that bit we should just reorder the checks for popup blocking and sandboxing.

I don't get the popup blocker UI in Firefox anymore. I just submitted a PR for the spec to remove even allowing that: https://github.com/whatwg/html/pull/5817

Still, Firefox doesn't do what the spec requires. The spec says to return null, Firefox throws an exception.

Would you like to reuse this bug about the exception, or close this bug and report a new one?

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: