How to handle detached iframes

2,902 views
Skip to first unread message

Kentaro Hara

unread,
Jun 22, 2016, 4:45:37 AM6/22/16
to platform-architecture-dev, Jochen Eisinger, Daniel Cheng, Yuki Shiino, dom...@chromium.org
Hi

Detached iframes have been causing many stability issues (e.g., P1 bug, P1 bug). Interop is broken in many ways. The problem is that we detach various Blink/V8 things when an iframe gets detached but we support executing scripts on the iframe. I won't be surprised whatever happens there...

There're basically two approaches to fix the problem:

a) Detach everything (especially V8's global object; jochen's CL) when the iframe gets detached. Don't support script execution afterwards.

b) Support full script execution on detached iframes.

Historically we were going toward the approach a), but I begin to think that b) would be the right approach for the following reasons:

- The spec requires b). We asked a question to the spec author and confirmed that b) is a correct behavior. (However, it might be possible to change the spec because behaviors of other browsers around detached iframes are already broken in many ways.)

- a) is hard to implement. It's possible to detach V8's global object, but there is no easy way to forbid all script executions on detached iframes. For example, if a main frame holds a reference to iframe.contentDocument and then the iframe gets detached, the main frame can still call document.createElement for the document of the detached iframe. To forbid all the script executions, we need to insert checks to all C++ bindings.

So yukishiino@ and I are now thinking that b) would be a right way to go, but any thoughts?


--
Kentaro Hara, Tokyo, Japan

Ojan Vafai

unread,
Jun 22, 2016, 4:54:06 AM6/22/16
to Kentaro Hara, platform-architecture-dev, Jochen Eisinger, Daniel Cheng, Yuki Shiino, Domenic Denicola, kin...@chromium.org
I know historically, b has been very hard. I believe this is why we removed magic iframes before. Someone with more inside knowledge than me should clarify though.

If we did b, I believe it would also help ship visibilitychange events on iframes when they are detached. This is currently the blocking api change to killing unload events. +Kinuko fyi.

So, I defer to others on whether b is possible, but we'd get some extra value from it aside from stability.


--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jw2U9W_G%3Daq759eadQaxT253cx-hah0aS3FQedOE91ZpA%40mail.gmail.com.

Kentaro Hara

unread,
Jun 22, 2016, 5:01:29 AM6/22/16
to Ojan Vafai, platform-architecture-dev, Jochen Eisinger, Daniel Cheng, Yuki Shiino, Domenic Denicola, Kinuko Yasuda
Thanks, I welcome more inputs.

From the binding perspective, b) has been hard for two reasons:

1) Running scripts on detached iframes can cause cross-origin security exploits.

2) Not detaching V8's context can cause memory leak.

Regarding 1), we've improved the cross-origin security handling and we have a plan to guarantee that all scripts run on correct contexts. I hope it will mitigate 1). Regarding 2), I think it's solvable.

Daniel Cheng

unread,
Jun 22, 2016, 8:20:52 AM6/22/16
to Kentaro Hara, Ojan Vafai, platform-architecture-dev, Jochen Eisinger, Yuki Shiino, Domenic Denicola, Kinuko Yasuda

The last I checked, Edge and Firefox don't allow closures created by a detached iframe to run (they throw an exception): only Blink and WebKit support this.

Using this for visibilitychange events is interesting, but there are some things that would be confusing: for example, what does window.document work if the frame's been navigated? Do we end up directly exposing the global object to JS once the context is detached instead of the global proxy?

Daniel

Yuki Shiino

unread,
Jun 22, 2016, 8:51:38 AM6/22/16
to Daniel Cheng, Kentaro Hara, Ojan Vafai, platform-architecture-dev, Jochen Eisinger, Yuki Shiino, Domenic Denicola, Kinuko Yasuda
Do we end up directly exposing the global object to JS once the context is detached instead of the global proxy?

For this specific point, I think we should always return the global proxy object to user script, regardless of whether the frame is detached or not.  And I think attached frames require us to be more careful than detached frames for this point because detached frames never navigate to another page.  IIUC, detached frame's global proxy object always points to the same underlying global object unless the frame gets attached (then a navigation occurs).

I think the problem of "global object or global proxy object" is orthogonal to the problem of "detached or attached".

Cheers,
Yuki Shiino

Elliott Sprehn

unread,
Jun 22, 2016, 8:57:06 AM6/22/16
to Daniel Cheng, kin...@chromium.org, Jochen Eisinger, Domenic Denicola, platform-architecture-dev, Kentaro Hara, Yuki Shiino, Ojan Vafai

Firefox has proxies between the contexts as I understand it. We can't really do that, nor do I think we should build the infrastructure for it. Could we instead disable the various platform apis and navigate the document to a unique origin or something of that nature?

In general we probably need to get together with the various browsers and the spec authors and come up with something that's not possible and matches some reality. I don't think anyone follows the spec fiction today.

Yuki Shiino

unread,
Jun 22, 2016, 9:50:39 AM6/22/16
to Elliott Sprehn, Daniel Cheng, Kinuko Yasuda, Jochen Eisinger, Domenic Denicola, platform-architecture-dev, Kentaro Hara, Yuki Shiino, Ojan Vafai
Firefox has proxies between the contexts as I understand it. We can't really do that, nor do I think we should build the infrastructure for it.

V8 supports the global proxy object and global object (and hidden prototype between them), and we're using that proxy system.  The global proxy object corresponds to WindowProxy and the global object corresponds to Window.  So we're following the spec's model so far, I think.

Also, we're sometimes writing frame()->domWindow() in Blink's codebase, and this is what WindowProxy requires.  In this case, frame() corresponds to WindowProxy and frame()->domWindow() corresponds to Window.  IIUC, this is the reason why we need to write obj->frame()->domWindow() instead of obj->domWindow().


Could we instead disable the various platform apis and navigate the document to a unique origin or something of that nature?

I don't understand how this helps.

    iframe = document.createElement('iframe');
    document.body.appendChild(iframe);  // Attach the iframe.
    w = iframe.contentWindow;  // Save the WindowProxy object of the iframe's browsing context.
    d = iframe.contentDocument;  // Save the Document of |w|.
    F = w.Function;  // Save Function, too.

    iframe.remove();  // Detach the iframe.

    iframe.contentWindow;  // Returns null, but it does NOT mean |w| is null.
    w;  // |w| is the same non-null WindowProxy object even if it pointed to another Window.
    d;  // |d| is also a non-null Document.
    F;  // You can use F, too, and run any scripts in that context.

Even if we create a new Window with a new Document with a unique origin and make |w| point to that new Window, you can still access to |w|, |d|, and |F| above.  As long as |F| is alive, the context must exists, and you can run any code in the creation context of |F|.

This is the reason why haraken@ wrote in his first mail of this thread that it's hard to forbid all the script execution.  If we're going to forbid user scripts to access to DOM objects on detached frames, we have to check if the context is detached or not for all the DOM attributes and DOM operations, because user scripts can save any DOM objects for later use.
    // |d| is the saved Document above.
    d.attribute  // Needs to check if |d|'s context is detached or not
    d.operation()  // Needs to check if |d|'s context is detached or not

As long as user scripts can save arbitrary objects (including DOM objects) for later use before detaching, I don't have a good idea to forbid user scripts to access to the DOM objects on detached frames.

Cheers,
Yuki Shiino

Elliott Sprehn

unread,
Jun 22, 2016, 9:59:19 AM6/22/16
to Yuki Shiino, Jochen Eisinger, kin...@chromium.org, Daniel Cheng, Kentaro Hara, platform-architecture-dev, Domenic Denicola, Ojan Vafai


On Jun 22, 2016 3:50 PM, "Yuki Shiino" <yukis...@chromium.org> wrote:
>>
>> Firefox has proxies between the contexts as I understand it. We can't really do that, nor do I think we should build the infrastructure for it.
>
>
> V8 supports the global proxy object and global object (and hidden prototype between them), and we're using that proxy system.  The global proxy object corresponds to WindowProxy and the global object corresponds to Window.  So we're following the spec's model so far, I think.
>
> Also, we're sometimes writing frame()->domWindow() in Blink's codebase, and this is what WindowProxy requires.  In this case, frame() corresponds to WindowProxy and frame()->domWindow() corresponds to Window.  IIUC, this is the reason why we need to write obj->frame()->domWindow() instead of obj->domWindow().

Yup, Firefox has a membrane between the two contexts for all objects though, not just the window. It lets them do some fancier things. We don't have anything like that.

>
>
>> Could we instead disable the various platform apis and navigate the document to a unique origin or something of that nature?
>
>
> I don't understand how this helps.
>
>     iframe = document.createElement('iframe');
>     document.body.appendChild(iframe);  // Attach the iframe.
>     w = iframe.contentWindow;  // Save the WindowProxy object of the iframe's browsing context.
>     d = iframe.contentDocument;  // Save the Document of |w|.
>     F = w.Function;  // Save Function, too.
>
>     iframe.remove();  // Detach the iframe.
>
>     iframe.contentWindow;  // Returns null, but it does NOT mean |w| is null.
>     w;  // |w| is the same non-null WindowProxy object even if it pointed to another Window.
>     d;  // |d| is also a non-null Document.
>     F;  // You can use F, too, and run any scripts in that context.
>
> Even if we create a new Window with a new Document with a unique origin and make |w| point to that new Window, you can still access to |w|, |d|, and |F| above.  As long as |F| is alive, the context must exists, and you can run any code in the creation context of |F|.
>
> This is the reason why haraken@ wrote in his first mail of this thread that it's hard to forbid all the script execution.  If we're going to forbid user scripts to access to DOM objects on detached frames, we have to check if the context is detached or not for all the DOM attributes and DOM operations, because user scripts can save any DOM objects for later use.
>     // |d| is the saved Document above.
>     d.attribute  // Needs to check if |d|'s context is detached or not
>     d.operation()  // Needs to check if |d|'s context is detached or not

That doesn't really make sense. You need a proxy at all JS functions, it doesn't make sense that appendChild would throw but Array forEach wouldn't.

>
> As long as user scripts can save arbitrary objects (including DOM objects) for later use before detaching, I don't have a good idea to forbid user scripts to access to the DOM objects on detached frames.
>

We don't need to forbid all script access, nor should we. The issue is around security origins of the content and apis generated from the frames. Oilpan solves much of the strange lifetime problems, the rest can be solved by disabling the api layer below the web platform features (ex. Fetching) and by forcing the document to take on a unique origin so the frame looses access to any resources across the frame tree.

In either case blink and WebKit already disagree on various things here since we've diverged the code so much. Firefox also had different behavior from edge. I'm pretty sure the spec disagrees with everyone. We need to figure out what's possible in everyone's engine and spec that.

- E

Yuki Shiino

unread,
Jun 22, 2016, 10:11:52 AM6/22/16
to Elliott Sprehn, Yuki Shiino, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, Kentaro Hara, platform-architecture-dev, Domenic Denicola, Ojan Vafai
The issue is around security origins of the content and apis generated from the frames.

I don't understand this point and what you mean.  If the iframe is a cross-origin, you cannot access to any object other than Window and Location regardless of whether it's detached or not.  "detached or not" has nothing to do with the origins.  While we're talking about how to treat detached frames, why origins matter?

Kentaro Hara

unread,
Jun 22, 2016, 10:44:37 PM6/22/16
to Yuki Shiino, Elliott Sprehn, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, platform-architecture-dev, Domenic Denicola, Ojan Vafai
Overall I think:

- It would be possible to change the spec because all browsers behave differently already. We should discuss this with other browsers and spec authors.

- No matter how the spec is improved (e.g., forbid resource fetching on detached frames etc), I don't think we should forbid script execution on detached iframes (there is no easy way to forbid it in reality). We should improve our binding infrastructure so that it can support full script execution without crashing. If we go in this direction, we don't need to (per the current spec, we should not) detach global objects when an iframe gets detached.



Elliott Sprehn

unread,
Jun 23, 2016, 5:47:27 PM6/23/16
to Yuki Shiino, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, Kentaro Hara, platform-architecture-dev, Domenic Denicola, Ojan Vafai
On Wed, Jun 22, 2016 at 4:11 PM, Yuki Shiino <yukis...@chromium.org> wrote:
The issue is around security origins of the content and apis generated from the frames.

I don't understand this point and what you mean.  If the iframe is a cross-origin, you cannot access to any object other than Window and Location regardless of whether it's detached or not.  "detached or not" has nothing to do with the origins.  While we're talking about how to treat detached frames, why origins matter?



This isn't about cross origin frames, this is about frames in general. 

var frame = document.createElement("iframe");
frame.src = "...";
document.body.appendChild(frame);
var contentWindow = frame.contentWindow;
frame.remove();

Now you can do contentWindow.setTimeout(), contentWindow.fetch(), new contentWindow.AudioContext(), and much more. The question is what should all of that do? What's the security context of the iframe now that it's no longer inside the frame tree?

Historically there's been two issues:

1. Lifetime of objects causing security issues and crashes. Oilpan fixed a lot of this.
2. A frame that isn't in the frame tree has all kinds of weird security state.

I was suggesting that if we switch the detached frame to have a unique origin that might make #2 easier. Perhaps there's some other ways too. We should go through the recent security issues with detached frames and figure out what a simpler system would look like that handles those cases.

It's not actually possible to stop all scripting without a membrane like Firefox.

- E

Nick Carter

unread,
Jun 23, 2016, 6:09:42 PM6/23/16
to Elliott Sprehn, Yuki Shiino, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, Kentaro Hara, platform-architecture-dev, Domenic Denicola, Ojan Vafai, Charlie Reis
+creis

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

Charlie Reis

unread,
Jun 23, 2016, 6:35:18 PM6/23/16
to Nick Carter, Elliott Sprehn, Yuki Shiino, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, Kentaro Hara, platform-architecture-dev, Domenic Denicola, Ojan Vafai
Yeah, this seems like all kinds of fun.  Nick and I just found that this applies equally well to top-level windows.

Suppose one tab opens a second tab, and the second tab grabs a reference to window.opener.  If you close the first tab, the reference stays valid, and you can reach in and run script (if you're same origin).  That means the renderer process can't clean up anything about the old tab if some JavaScript reference is keeping it alive, which sounds pretty bad for memory usage (in addition to all these other corner cases it's exposing).

Repro steps if you want to try it:
2) Click "Open named about:blank window."
3) In the new window, type "o = window.opener" in DevTools.
4) Close the original window.
5) In the new window, type "o.document.body.innerHTML" and see that it's still around.  You can run script in the old window, too, despite the fact that its whole WebContents is gone.

Maybe this is well known, but it certainly seems like an unexplored case with plenty of bugs possible.

Charlie



Yuki Shiino

unread,
Jun 24, 2016, 5:00:49 AM6/24/16
to Charlie Reis, Nick Carter, Elliott Sprehn, Yuki Shiino, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, Kentaro Hara, platform-architecture-dev, Domenic Denicola, Ojan Vafai
Now you can do contentWindow.setTimeout(), contentWindow.fetch(), new contentWindow.AudioContext(), and much more. The question is what should all of that do? What's the security context of the iframe now that it's no longer inside the frame tree?

Let me answer the last question first.

Q. What security context we should use if the iframe is detached?
A. window->document()'s security context should be used for a window detached from a frame, while window->frame()->domWindow()->document()'s security context is used for a window inside the frame tree.
I think this problem should be divided into two.
1) Determine a Window object given a WindowProxy object.
2) Determine a security context given a Window object.
2) is easy.  If |window| is the window to be used, then window->document() holds the security context to be used.  1) is slightly complicated than 2), but not so much.  A WindowProxy points to a different Window object as the page navigates, so we need to be careful to extract a correct Window object through a WindowProxy.  However, if no navigation occurs, a WindowProxy keeps pointing to the same Window object, and never changes unless it navigates.  Since detached iframes never navigate, we can simply use |window| itself instead of window->frame()->domWindow().

In short, if a window is detached from a frame, |window| itself should be used as a Window object, otherwise window->frame()->domWindow() should be used as a Window object extracted through a WindowProxy object.  I think this conforms to the spec.


Q. What should we do for window.setTimeout, window.fetch, window.AudioContext, etc.
A. As long as user script holds a reference to the window, that window never gets GC'ed, so technically we can do anything that we can do for attached windows, unless specific APIs depend on the existence of a frame, e.g. window.alert().  I agree that it's better to get consensus among browser vendors what should be supported and what shouldn't.  However, I think these are NOT a binding's problem.  These are API specific issues.

Although the spec doesn't have a concept directly corresponding to "frame", there are similar limitations.  For example, navigation occurs only when iframe is attached to an active document, or a DOM element needs to be added to a DOM tree before some kinds of operations (otherwise, they're noop).

As a binding team, I think what we can do is:
a) support detached frames as same as non-detached ones, and
b) let each API knows the current status correctly and let APIs do what they want/need to do.

I'm interested in supporting detached frames as a system of binding's layer, and I'm okay that specific APIs do not support specific things in case of detached frames.  I think it's better to separate what specific APIs support from what binding layer supports as a binding system.

Cheers,
Yuki Shiino

Kentaro Hara

unread,
Jun 24, 2016, 5:08:21 AM6/24/16
to Yuki Shiino, Charlie Reis, Nick Carter, Elliott Sprehn, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, platform-architecture-dev, Domenic Denicola, Ojan Vafai
I'm interested in supporting detached frames as a system of binding's layer, and I'm okay that specific APIs do not support specific things in case of detached frames.  I think it's better to separate what specific APIs support from what binding layer supports as a binding system.

+1. This sounds like a reasonable way to go. It's important to support script execution on detached frames to fix a lot of random crashes on trunk.

Yuki Shiino

unread,
Aug 5, 2016, 7:05:41 AM8/5/16
to Kentaro Hara, Yuki Shiino, Charlie Reis, Nick Carter, Elliott Sprehn, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, platform-architecture-dev, Domenic Denicola, Ojan Vafai
Some updates here.

Domenic kindly connected us to Travis from Microsoft Edge team, and it turned out that Edge is trying to support scripting on detached frames although Edge currently does not support it and throws exceptions in many cases.  Having said that, we agreed that we may need to disable not few web platform features on detached frames, although there is no clear list of such web platform features.

There is a rough plan to make some changes in the binding layer to better support detached frames.
0. Crashes are happening mostly when accessing to a ScriptState which already discarded the V8PerContextData.  ScriptState is passed to the DOM implementation if [CallWith=ScriptState] or [CallWith=ExecutionContext] extended attribute is specified.  This is the place we have to pay our attention.
1. For those attributes/operations which have [CallWith=XXX], the binding code generator will insert a new check to see if ScriptState is still valid or not.  If not, it returns undefined or throws an exception (TBD).  This should be better than continuing running the code and crashing.
2. Make ScriptState keep the correct V8PerContextData as long as it's alive.  At this point, technically Blink is capable to run user scripts on detached frames, but user scripts are not yet allowed to access to DOM attributes/operations with [CallWith=XXX] on detached frames.
3. As we confirm that we can safely support specific attributes/operations with [CallWith=XXX], we'll make them not return undefined nor throw an exception and we'll allow them to run the actual DOM implementation, i.e. we'll support those attributes/operations on detached frames.  We'll do this step by step.

In short,
a) Make DOM APIs return undefined or throw instead of crashing if detached frames.
b) Internally prepare support of detached frames.
c) Allow user scripts to access to detached frames step by step (API-by-API) as we confirm its safety.

This is a very rough plan and there are a lot of TBD, however, we think we can better support detached frames with very low risk with this approach.

Cheers,
Yuki Shiino


+creis

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

Yutaka Hirano

unread,
Aug 8, 2016, 4:45:08 AM8/8/16
to Yuki Shiino, Kentaro Hara, Charlie Reis, Nick Carter, Elliott Sprehn, Jochen Eisinger, Kinuko Yasuda, Daniel Cheng, platform-architecture-dev, Domenic Denicola, Ojan Vafai
> 1. For those attributes/operations which have [CallWith=XXX], the binding code generator will insert a new check to see if ScriptState is still valid or not.  If not, it returns undefined or throws an exception (TBD).  This should be better than continuing running the code and crashing.

I may be good to return a rejected promise if that attribute/operation is specified as returning a promise.

+creis

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.
--
Kentaro Hara, Tokyo, Japan

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
Reply all
Reply to author
Forward
0 new messages