XSS Auditor: Block by default.

XSS Auditor: Block by default.

This patch changes the default behavior of the XSS auditor from "filter"
to "block". It also fixes a bug exposed by this change: blocking a page
in the middle of parsing/processing `document.write()` crashes the
renderer due to a null deref.

The vast majority of this change is changing layout tests to specify
filtering behavior rather than default behavior.

Intent to Ship: https://groups.google.com/a/chromium.org/d/msg/blink-dev/aZsNygF84JM/86EbD_q0CAAJ

BUG=654794
Committed: https://crrev.com/46b2f19290555de613e09226348ae711db179f58
Cr-Commit-Position: refs/heads/master@{#434392}

[Mike West, 2016-11-23 15:31:23]

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

[commit-bot: I haz the power, 2016-11-23 15:31:45]

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-11-23 15:32:23]

mkwst@chromium.org changed reviewers:
+ tsepez@chromium.org

[Mike West, 2016-11-23 15:32:24]

Hi Tom. Sorry in advance, this is a big mess of changes to layout tests.

I'm sure it'll break a few more, and probably some browser/unit tests that rely
on the current default behavior. Assuming I fix those up, WDYT?

[Mike West, 2016-11-23 15:35:42]

https://codereview.chromium.org/2524013002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt
(right):

https://codereview.chromium.org/2524013002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt:1:
CONSOLE ERROR: line 6: The XSS Auditor refused to execute a script in
'http://localhost:8000/security/xssAuditor/resources/echo-dom-write-location.php?#<script>alert(String.fromCharCode(0x58,0x53,0x53))</script>'
because its source code was found within the request. The server sent an
'X-XSS-Protection' header requesting this behavior.
This is the extent of the change for most of the `-expected.txt` files: I
changed the framed data to a PHP file so I could add in `X-XSS-Protection: 1`.
This preserves the behavior the test is testing, and makes this minor change to
the error message.

https://codereview.chromium.org/2524013002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/malformed-xss-protection-header-1-expected.txt
(right):

https://codereview.chromium.org/2524013002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/malformed-xss-protection-header-1-expected.txt:3:
ALERT: URL mismatch: '[Location object access threw exception]' vs.
'http://127.0.0.1:8000/security/xssAuditor/resources/echo-intertag.pl?notifyDone=1&malformed-header=1&q=%3Cscript%3Ealert(String.fromCharCode(0x58,0x53,0x53))%3C/script%3E%3Cp%3EIf%20you%20see%20this%20message%20and%20no%20JavaScript%20alert()%20then%20the%20test%20PASSED.%3C/p%3E'
Because we're now blocking, these tests fail to read the URL from the inner file
when presented with a malformed header. I think keeping this behavior is a
reasonable way to test the new default, but I'll change the error message to
something like "Couldn't read frame's URL: its Document is either cross-origin
or was blocked by the XSS auditor."

WDYT?

[Tom Sepez, 2016-11-23 16:31:29]

BUG= appears to be an unrelated issue.

[Tom Sepez, 2016-11-23 16:36:52]

On 2016/11/23 16:31:29, Tom Sepez wrote:
> BUG= appears to be an unrelated issue.

Also, any relation to
https://bugs.chromium.org/p/chromium/issues/detail?id=421786

[Tom Sepez, 2016-11-23 16:39:59]

Otherwise, looks good.  I only spot-checked the expectations files.

https://codereview.chromium.org/2524013002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt
(right):

https://codereview.chromium.org/2524013002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location-expected.txt:1:
CONSOLE ERROR: line 6: The XSS Auditor refused to execute a script in
'http://localhost:8000/security/xssAuditor/resources/echo-dom-write-location.php?#<script>alert(String.fromCharCode(0x58,0x53,0x53))</script>'
because its source code was found within the request. The server sent an
'X-XSS-Protection' header requesting this behavior.
On 2016/11/23 15:35:42, Mike West (slow) wrote:
> This is the extent of the change for most of the `-expected.txt` files: I
> changed the framed data to a PHP file so I could add in `X-XSS-Protection: 1`.
> This preserves the behavior the test is testing, and makes this minor change
to
> the error message.

Acknowledged.

[commit-bot: I haz the power, 2016-11-23 17:15:44]

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

[commit-bot: I haz the power, 2016-11-23 17:15:46]

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-11-23 19:02:17]

Description was changed from

==========
XSS Auditor: Block by default.

This patch changes the default behavior of the XSS auditor from "filter"
to "block". It also fixes a bug exposed by this change: blocking a page
in the middle of parsing/processing `document.write()` crashes the
renderer due to a null deref.

The vast majority of this change is changing layout tests to specify
filtering behavior rather than default behavior.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/aZsNygF84JM/86EbD_q0...

BUG=654791
==========

to

==========
XSS Auditor: Block by default.

This patch changes the default behavior of the XSS auditor from "filter"
to "block". It also fixes a bug exposed by this change: blocking a page
in the middle of parsing/processing `document.write()` crashes the
renderer due to a null deref.

The vast majority of this change is changing layout tests to specify
filtering behavior rather than default behavior.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/aZsNygF84JM/86EbD_q0...

BUG=654794
==========

[Mike West, 2016-11-23 19:04:53]

On 2016/11/23 at 16:36:52, tsepez wrote:
> On 2016/11/23 16:31:29, Tom Sepez wrote:
> > BUG= appears to be an unrelated issue.

Fixed, thanks for checking that!

> Also, any relation to
https://bugs.chromium.org/p/chromium/issues/detail?id=421786

No, that looks different. The bug fixed here is that blocking the page cancels
the loader which eventually tells the parser to discard it's current token. That
means that when we try to use the token in `constructTreeFromHTMLToken()`,
things go boom.

[Tom Sepez, 2016-11-23 19:13:27]

On 2016/11/23 19:04:53, Mike West (slow) wrote:
> On 2016/11/23 at 16:36:52, tsepez wrote:
> > On 2016/11/23 16:31:29, Tom Sepez wrote:
> > > BUG= appears to be an unrelated issue.
> 
> Fixed, thanks for checking that!
> 
> > Also, any relation to
> https://bugs.chromium.org/p/chromium/issues/detail?id=421786
> 
> No, that looks different. The bug fixed here is that blocking the page cancels
> the loader which eventually tells the parser to discard it's current token.
That
> means that when we try to use the token in `constructTreeFromHTMLToken()`,
> things go boom.

Looks like we still have a failure in win_chromium_rel_ng.  LGTM otherwise.

[Mike West, 2016-11-24 09:49:17]

The CQ bit was checked by mkwst@chromium.org

[Mike West, 2016-11-24 09:49:18]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2524013002/#ps20001
(title: "Rebase+Test")

[commit-bot: I haz the power, 2016-11-24 09:49:42]

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

[Mike West, 2016-11-24 09:51:10]

The CQ bit was unchecked by mkwst@chromium.org

[Mike West, 2016-11-24 10:32:27]

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

[commit-bot: I haz the power, 2016-11-24 10:32:56]

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-11-24 11:30:46]

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

[commit-bot: I haz the power, 2016-11-24 11:30:48]

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-11-24 19:11:47]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-11-24 19:12:01]

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-11-24 19:48:04]

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

[commit-bot: I haz the power, 2016-11-24 19:48:06]

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

[Mike West, 2016-11-24 21:02:29]

The CQ bit was checked by mkwst@chromium.org

[commit-bot: I haz the power, 2016-11-24 21:02:50]

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-11-24 21:48:47]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480021349659030,
"parent_rev": "b1f51377009853f766a3219c319878ec6a4ed499", "commit_rev":
"c42878221dab7d18713eb36eb3f1cf9c687bd441"}

[commit-bot: I haz the power, 2016-11-24 21:49:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-24 21:51:25]

Description was changed from

==========
XSS Auditor: Block by default.

This patch changes the default behavior of the XSS auditor from "filter"
to "block". It also fixes a bug exposed by this change: blocking a page
in the middle of parsing/processing `document.write()` crashes the
renderer due to a null deref.

The vast majority of this change is changing layout tests to specify
filtering behavior rather than default behavior.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/aZsNygF84JM/86EbD_q0...

BUG=654794
==========

to

==========
XSS Auditor: Block by default.

This patch changes the default behavior of the XSS auditor from "filter"
to "block". It also fixes a bug exposed by this change: blocking a page
in the middle of parsing/processing `document.write()` crashes the
renderer due to a null deref.

The vast majority of this change is changing layout tests to specify
filtering behavior rather than default behavior.

Intent to Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/aZsNygF84JM/86EbD_q0...

BUG=654794

Committed: https://crrev.com/46b2f19290555de613e09226348ae711db179f58
Cr-Commit-Position: refs/heads/master@{#434392}
==========

[commit-bot: I haz the power, 2016-11-24 21:51:26]

Patchset 2 (id:??) landed as
https://crrev.com/46b2f19290555de613e09226348ae711db179f58
Cr-Commit-Position: refs/heads/master@{#434392}