Bug 162075 – top.location.replace() and top.location.assign() should be blocked in iframe with sandbox "allow-same-origin allow-scripts"

• Component: Page Loading
• Status: NEW

[Oliver Fernandez]

Created attachment 289077 [details]
demo showing how bypass iframe sandbox security

I have sandboxed IFRAME, with a sandbox that doesn't contain the property allow-top-navigation. 

The content of this IFRAME has been loaded using the property srcdoc (or directly writing the content using iframe.contentWindow.document.write method)

The IFRAME document tries to change top window location using top.location.replace('http://www.webkit.org').

The expected result should be that I get a Sandbox security error, but the actual result is that I get redirected. The same is happening with top.location.assign method. However if I use top.location.href = 'http://www.webkit.org' I get the correct behaviour. 

I've attached a complete example to reproduce the problem.

Chrome is showing the expected result.

[Radar WebKit Bug Importer]

<rdar://problem/28340114>

[Darin Adler]

The original implementation of the feature is here <https://trac.webkit.org/changeset/56591> and includes regression tests. I guess we don’t have replace and assign coverage in those tests!

[Oliver Fernandez]

Do you know if there's any work around I can use to prevent this? 

I've detected at least 1 Ad Network that is using this hole to redirect users to "malicious" advertising. That's why I think this is critical.

[Darin Adler]

(In reply to comment #3)
> Do you know if there's any work around I can use to prevent this? 

I assume you are specifically talking about needing a workaround for Safari on iOS.

This doesn’t seem like the type of thing that would have a workaround.

> I've detected at least 1 Ad Network that is using this hole to redirect
> users to "malicious" advertising. That's why I think this is critical.

Yes, I agree that it is critical and I believe the relevant people at Apple are treating it as critical, too.

[Daniel Bates]

activeDOMWindow() in jsLocationInstanceFunctionReplace() and jsLocationInstanceFunctionAssign() are returning the DOM window associated with the main frame. But they should return the DOM window associated with the frame (<iframe>) whose document called location.replace(), location.assign(), respectively.

[Daniel Bates]

(In reply to comment #5)
> activeDOMWindow() in jsLocationInstanceFunctionReplace() and
> jsLocationInstanceFunctionAssign() are returning the DOM window associated
> with the main frame. But they should return the DOM window associated with
> the frame (<iframe>) whose document called location.replace(),
> location.assign(), respectively.

s/are/is
s/they/it

"...called location.replace(), location.assign(), respectively" => "...called location.replace() and location.assign(), respectively"

[Darin Adler]

That sounds like callerDOMWindow, then, which is a different concept than activeDOMWindow.

[Daniel Bates]

The bypass only occurs when a site embeds a same-origin page (*) in an HTML iframe element that allows same-origin and script access. Although we should prevent top navigation from HTML iframes that embed a same-origin page, allow scripts, allow same-origin access, and disallow top navigation (e.g. sandbox="allow-scripts allow-same-origin") for consistency with the HTML standard among other reasons, this is a minor inconvenience to a malicious Ad Network at best. It will not prevent top navigation; pages loaded in such iframes can still navigate the top-most frame by first removing their sandbox restrictions and reloading themselves. To prevent this, the site that embeds the iframe must load the ad content in its own origin. One way to do this is to define a sandbox policy for the iframe that does not include the allow-same-origin flag.

(*) A srcdoc page has the same origin as the page that embedded it.

[Daniel Bates]

(In reply to comment #7)
> That sounds like callerDOMWindow, then, which is a different concept than
> activeDOMWindow.

Yes, we want to use callerDOMWindow(), but we will need to make callerDOMWindow() smarter with respect to allow-same-origin iframes that load same-origin pages as the current implementation of callerDOMWindow() [1] is insufficient. Consider the page with the following markup:

<!DOCTYPE html>
<html>
<body>
<iframe sandbox="allow-scripts allow-same-origin" srcdoc="<script>['http://www.webkit.org'].map(window.top.location.replace.bind(window.top.location))</script>"></iframe>
</body>
</html>

Both callerDOMWindow() and activeDOMWindow() return the DOM window associated with the main frame when location.replace() is invoked.

[1] <https://trac.webkit.org/browser/trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp?rev=207473#L783>

[Daniel Bates]

Created attachment 291960 [details]
Layout tests

[Daniel Bates]

(In reply to comment #8)
> ... this is a minor inconvenience to a malicious Ad Network at best.

*ad network

> It will not prevent top navigation; pages loaded in such iframes can still navigate the top-most frame by first removing their...

*same-origin pages

[Daniel Bates]

@Oliver Fernandez: Can you elaborate on how this bug represents a security threat?

As far as I can tell this bug is a correctness issue and we should fix it. It does not seem to represent a security threat anymore than either <iframe srcdoc="..."> or <iframe src="U">, where U is the URL for a document that has the same origin as the document that embedded the iframe. I mean, an iframe with sandbox "allow-scripts allow-same-origins" with a srcdoc (or loads a page with the same origin as its containing document) has sufficient permission to remove the sandbox attribute from itself. The HTML standard explicitly warns against this combination in <https://html.spec.whatwg.org/multipage/embedded-content.html#attr-iframe-sandbox>.

[Daniel Bates]

Oliver Fernandez has not replied to my question in comment #12 in over two weeks. I reached out to him in a private email last Thursday, 11/03, and have not heard back from him.

Making this bug public as it does not represent a security threat any more than an iframe without a sandbox attribute. See my remark in comment #8 for more details.

[Oliver Fernandez]

You are right, if you are using an IFRAME with src=about:blank and srcdoc='' it's because you are trusting the iframe's content. But let me explain why we are using this iframe src=about:blank

When working with advertisement, sometimes it's just impossible to avoid some malicious advertisers redirecting the user without user interaction. This happens from time to time with all major Ad Networks: Ad Exchange, Rubicon, Criteo, ...

We found a solution to 100% avoid this kind of advertising, and always protect the user: put the Ad Network client side code inside an iframe with src=about:blank Here are the reasons

1. Since it is not a cross-origin IFRAME, Ad Network Javascript code can access top location and cookies, so it can properly target the user, for good Ad quality
2. Since it's inside an IFRAME, we can use the sandox attribute, so we can prevent these malicious advertisers to replace the top frame location.

The last point is not true in Safari, because window.location.replace is not protected.

This is a problem that any website sustained by advertisement faces. We work with hundreds of publishers, and all the complains we receive about this kind of advertisement comes from Safari users (specially iOS), since this technique to safely load advertisement doesn't work in iPhone. This mean we cannot offer this solution to everyone and widespread its usage.

[Oliver Fernandez]

I just realized what Daniel were mentioning: since it a same origin IFRAME, you can do something like: window.frameElement.removeAttribute('sandbox'), do a location reload, and then the sandbox is gone :-(

So yeah, even though the window.location.replace is fixed, having a sandbox in a same-origin IFRAME doesn't guarantee 100% protection.

[Daniel Bates]

Created attachment 304310 [details]
Patch and layout tests

This patch depends on the patch for bug #163412.

[Geoffrey Garen]

I'm worried that "CallerWindow" in .idl and callerDOMWindow() in JSDOMWindowBase.cpp are anti-patterns.

(1) Restricting access based on caller rather than callee is suspicious because caller had access to callee somehow, which implies maybe a security problem.

(2) The implementation of callerDOMWindow() allows its to bail out in lots of fun ways, which suggests that it wasn't designed with security in mind.

Why do we want our caller in this case? Is it because these Location functions have the special behavior that foreign callers get unique copies of Location functions, with restricted privileges? If so, I wonder if we can arrange for those unique copies to have a lexicalGlobalObject equal to the caller's lexical global object.

[Build Bot]

Comment on attachment 304310 [details]
Patch and layout tests

Attachment 304310 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3311003

New failing tests:
http/tests/security/frameNavigation/context-for-location-assign.html
fast/frames/sandboxed-iframe-allow-same-origin-navigation-top-indirect-replace-denied.html
fast/frames/sandboxed-iframe-navigation-parent.html
fast/frames/sandboxed-iframe-allow-same-origin-navigation-top-indirect-assign-denied.html

[Build Bot]

Created attachment 304318 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 304310 [details]
Patch and layout tests

Attachment 304310 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3311054

New failing tests:
http/tests/security/frameNavigation/context-for-location-assign.html
fast/frames/sandboxed-iframe-allow-same-origin-navigation-top-indirect-replace-denied.html
fast/frames/sandboxed-iframe-navigation-parent.html
fast/frames/sandboxed-iframe-allow-same-origin-navigation-top-indirect-assign-denied.html

[Build Bot]

Created attachment 304320 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 304310 [details]
Patch and layout tests

Attachment 304310 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3311052

New failing tests:
http/tests/security/frameNavigation/context-for-location-assign.html
fast/frames/sandboxed-iframe-allow-same-origin-navigation-top-indirect-replace-denied.html
fast/frames/sandboxed-iframe-navigation-parent.html
fast/frames/sandboxed-iframe-allow-same-origin-navigation-top-indirect-assign-denied.html

[Build Bot]

Created attachment 304323 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 304310 [details]
Patch and layout tests

Attachment 304310 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3317132

New failing tests:
http/tests/security/frameNavigation/context-for-location-assign.html
fast/frames/sandboxed-iframe-allow-same-origin-navigation-top-indirect-assign-denied.html
fast/frames/sandboxed-iframe-navigation-parent.html
fast/frames/sandboxed-iframe-allow-same-origin-navigation-top-indirect-replace-denied.html

[Build Bot]

Created attachment 304357 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6