Throw when blocking top-level navigation.

Throw when blocking top-level navigation.

In a sandboxed <iframe>, setting 'top.location' currently blocks the
resulting navigation, but does not throw. This doesn't match the HTML
spec (see https://html.spec.whatwg.org/#location-object-navigate and
https://html.spec.whatwg.org/#navigating-across-documents:allowed-to-navigate),
nor does it match Firefox's behavior.

This patch changes our implementation to throw a SecurityError in
these cases.

BUG=650232
Committed: https://crrev.com/ce8199558bd1cc8737c4349fc0937860b380da3c
Cr-Commit-Position: refs/heads/master@{#421807}

[Mike West, 2016-09-27 14:16:56]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 14:17:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2016-09-27 14:17:34]

mkwst@chromium.org changed reviewers:
+ elawrence@chromium.org, jochen@chromium.org

[Mike West, 2016-09-27 14:17:34]

Jochen, Eric, mind taking a look at this?

Thanks!

-mike

[commit-bot: I haz the power, 2016-09-27 14:44:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 14:44:24]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[jochen (gone - plz use gerrit), 2016-09-27 14:46:42]

can you verify that the exception that is thrown is not cross-origin?

[Mike West, 2016-09-27 14:55:30]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 14:55:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 16:09:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 16:09:58]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Mike West, 2016-09-27 16:28:47]

On 2016/09/27 at 14:46:42, jochen wrote:
> can you verify that the exception that is thrown is not cross-origin?

I'm pretty sure it's created in the same context in which it's thrown. As I'm
lazy, is there a test I can copy/paste? :)

Looks like a few tests are failing; I'll look into them tomorrow. Perhaps
they're relevant. ;)

[jochen (gone - plz use gerrit), 2016-09-27 19:32:33]

On 2016/09/27 at 16:28:47, mkwst wrote:
> On 2016/09/27 at 14:46:42, jochen wrote:
> > can you verify that the exception that is thrown is not cross-origin?
> 
> I'm pretty sure it's created in the same context in which it's thrown. As I'm
lazy, is there a test I can copy/paste? :)
> 
> Looks like a few tests are failing; I'll look into them tomorrow. Perhaps
they're relevant. ;)

hum, you could try to catch the exception (e) and do
e.constructor.constructor("console.log(location.href)")(); Ideally, it's not the
location of the outer frame :)

[jochen (gone - plz use gerrit), 2016-09-27 19:32:33]

On 2016/09/27 at 16:28:47, mkwst wrote:
> On 2016/09/27 at 14:46:42, jochen wrote:
> > can you verify that the exception that is thrown is not cross-origin?
> 
> I'm pretty sure it's created in the same context in which it's thrown. As I'm
lazy, is there a test I can copy/paste? :)
> 
> Looks like a few tests are failing; I'll look into them tomorrow. Perhaps
they're relevant. ;)

hum, you could try to catch the exception (e) and do
e.constructor.constructor("console.log(location.href)")(); Ideally, it's not the
location of the outer frame :)

[Mike West, 2016-09-28 08:39:41]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-28 08:39:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-28 09:55:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-28 09:55:56]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Mike West, 2016-09-28 12:24:19]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-28 12:24:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mike West, 2016-09-28 12:26:13]

https://codereview.chromium.org/2371993003/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/W3CImportExpectations (right):

https://codereview.chromium.org/2371993003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/W3CImportExpectations:368:
imported/wpt/html/browsers/history/the-location-interface/security_location_0.sub.htm
[ Skip ]
This test now fails because we're not running WPT, so the URL substitutions
don't happen. Our old behavior was disguising this failure, as the URL failure
would transparently fail. We now match Firefox's behavior for this test (when
loaded as a `file:` and not through `wptserve`.

https://codereview.chromium.org/2371993003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/security/sandbox-iframe-blocks-top-navigation.html
(right):

https://codereview.chromium.org/2371993003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/security/sandbox-iframe-blocks-top-navigation.html:20:
'location': e.constructor.constructor('return location.href')()
Jochen: Does this (and the assertion on #8) address your concern?

[commit-bot: I haz the power, 2016-09-28 13:26:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-28 13:27:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike West, 2016-09-29 09:28:57]

The CQ bit was checked by mkwst@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 09:29:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 10:36:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-29 10:36:54]

Dry run: This issue passed the CQ dry run.

[Mike West, 2016-09-29 11:37:26]

https://codereview.chromium.org/2371993003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/imported/wpt/html/browsers/history/the-location-interface/security_location_0.sub-expected.txt
(right):

https://codereview.chromium.org/2371993003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/imported/wpt/html/browsers/history/the-location-interface/security_location_0.sub-expected.txt:2:
FAIL Accessing location object from different origins doesn't raise SECURITY_ERR
exception Failed to set the 'location' property on 'Window':
'http://www1.{{host}}:{{ports[http][0]}}/' is not a valid URL.
Adding a failing expectation file, as per the comments in
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/TestExpec....
We don't support the host/port substitution, so... Still waiting for wptserve.

[jochen (gone - plz use gerrit), 2016-09-29 12:20:54]

lgtm

[Mike West, 2016-09-29 13:50:09]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-09-29 13:50:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 13:56:14]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-29 13:58:08]

Description was changed from

==========
Throw when blocking top-level navigation.

In a sandboxed <iframe>, setting 'top.location' currently blocks the
resulting navigation, but does not throw. This doesn't match the HTML
spec (see https://html.spec.whatwg.org/#location-object-navigate and
https://html.spec.whatwg.org/#navigating-across-documents:allowed-to-navigate),
nor does it match Firefox's behavior.

This patch changes our implementation to throw a SecurityError in
these cases.

BUG=650232
==========

to

==========
Throw when blocking top-level navigation.

In a sandboxed <iframe>, setting 'top.location' currently blocks the
resulting navigation, but does not throw. This doesn't match the HTML
spec (see https://html.spec.whatwg.org/#location-object-navigate and
https://html.spec.whatwg.org/#navigating-across-documents:allowed-to-navigate),
nor does it match Firefox's behavior.

This patch changes our implementation to throw a SecurityError in
these cases.

BUG=650232

Committed: https://crrev.com/ce8199558bd1cc8737c4349fc0937860b380da3c
Cr-Commit-Position: refs/heads/master@{#421807}
==========

[commit-bot: I haz the power, 2016-09-29 13:58:09]

Patchset 5 (id:??) landed as
https://crrev.com/ce8199558bd1cc8737c4349fc0937860b380da3c
Cr-Commit-Position: refs/heads/master@{#421807}