635673 - Content inside xul:iframe with type=content is always able to resize top chrome window

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Alexandre Poirot [:ochameau]]

There is a common issue when you are writing firefox extension and we faced it in Jetpack too.
There is multiple usecases where you want to display a remote content that you doesn't necessary trust. For example: sidebars, panels, widgets, ... 
So you use XUL Iframe with type attribute set to "content" in order to have a secured iframe, like the browser do with websites. 
But as websites are able to move and resize browser windows, your extension components (sidebar, widgets,...) is going to resize the browser window too.
And we really don't want that a sidebar resize the browser window!

We would really apprieciate having a new attribute on docShell, like allowWindowControl that would allow to block all move and resize commands coming from iframe document!
https://developer.mozilla.org/en/nsIDocShell

Here is a piece of code that highlight this behavior:
var win = window.top.opener;
var doc = win.document;
var iframe = doc.createElement("iframe");
iframe.setAttribute("type","content");
iframe.setAttribute("src","data:text/html,<script>window.resizeTo(200,200);</script>");
doc.documentElement.appendChild(iframe);

You may simply paste this code in a freshly opened JS console (not an automatically opened one with -jsconsole argument)

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Boris: is this feasible (and is this the right Bugzilla component for this bug)?

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is definitely feasible.  The simplest approach is probably to change nsGlobalWindow::CanMoveResizeWindows to not be static and have it check a property on the docshell.  The IsFrame() check that always goes hand in hand with it could go into that method too, or even into the docshell thing (so force subframes to not allow this on the docshell level).

As for component... there are several pieces here.  One is adding the docshell API; the other is exposing it to XUL authors.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Hmm, I suspect the docshell API by itself would be sufficient for Jetpack's needs, even without extra exposure through XUL (f.e. via a "bad-content-no-donut" iframe type), since the Add-on SDK has access to the docshells of the iframes it creates.

*Randomly guessing that jst is the guy to suggest someone to take that on.*

jst: can you recommend someone to do the work bz described in comment 2?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Fwiw, if we don't have someone else who obviously needs a small project, I could just do that.  Lemme know.

Comment 5

[Jan Varga [:janv]]

Boris, I'll take this bug, if you don't mind.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Go for it!

Comment 7

[Jan Varga [:janv]]

Attachment: patch

Comment 8

[Jan Varga [:janv]]

Here is modified JS for testing:
var win = window.top.opener;
var doc = win.document;
var iframe = doc.createElement("iframe");
iframe.setAttribute("type","content");
iframe.setAttribute("src","data:text/html,<button onclick='window.resizeTo(200,250)'>Resize me</button>");
doc.documentElement.appendChild(iframe);
iframe.docShell.allowWindowControl = false;

I didn't add a check for iframes (subframes) for now.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Jan: is your patch ready for review?

Comment 10

[Jan Varga [:janv]]

myk: yes, I think so

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 515735 [details] [diff] [review]
patch

>+  nsIDocShell* docShell = nsContentUtils::GetDocShellFromCaller();

Why is that the right docshell to use, as opposed to mDocShell?

Comment 12

[[e-mail]]

Surely it would be easiest to drop an mPrimary check into nsContentTreeOwner::SetSize and any related methods? This would also stop background tabs from inadvertently changing the window size.

Comment 13

[Jan Varga [:janv]]

Attachment: updated patch

Comment 14

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 536554 [details] [diff] [review]
updated patch

Erm, it looks like there's a patch sitting around waiting for review again.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 536554 [details] [diff] [review]
updated patch

r=me

Comment 16

[Jan Varga [:janv]]

I will land it and thanks for the review

Comment 17

[Jan Varga [:janv]]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/babf3b1dfdbc
http://hg.mozilla.org/mozilla-central/rev/3878e0b60ff2

Comment 18

[Alexandre Poirot [:ochameau]]

I'm now using this feature in jetpack and it works like a charm,
Thanks Jan!

Comment 19

[Eric Shepherd [:sheppy]]

Added the new attribute to the reference documentation:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDocShell#Attributes

And mentioned on Firefox 7 for developers.