Bug 26784 – Enable XSSAuditor by default

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Adam Barth]

We should try enabling the XSSAuditor by default in the nightly to get a sense for the false positive rate.  Sam said we should do this once we have decent test coverage, and we now have 29 tests.

Please CC me and Dan on any regressions / false positives we find.  If we get a bunch of them, we can turn off the auditor again while we think about how to reduce them.

We still have one known false negative (HTML entities), but we can work on fixing that in parallel.  Also, we should support the "turn off XSS filtering" header that IE8 supports, but I'll file a separate bug about that.

[Adam Barth]

Created attachment 31993 [details]
patch

[Daniel Bates]

Another known false negative is HTTP header injection.

(In reply to comment #0)
> We should try enabling the XSSAuditor by default in the nightly to get a sense
> for the false positive rate.  Sam said we should do this once we have decent
> test coverage, and we now have 29 tests.
> 
> Please CC me and Dan on any regressions / false positives we find.  If we get a
> bunch of them, we can turn off the auditor again while we think about how to
> reduce them.
> 
> We still have one known false negative (HTML entities), but we can work on
> fixing that in parallel.  Also, we should support the "turn off XSS filtering"
> header that IE8 supports, but I'll file a separate bug about that.
> 

[Adam Barth]

(In reply to comment #2)
> Another known false negative is HTTP header injection.

True.  I suppose we could try to stop that too.  I was thinking of that as more of a non-goal for the first version.

[Eric Seidel (no email)]

I would think it would be annoying that this would break plexode.com and hixie's live dom viewer in the nightlies.  Not a deal breaker.  But definitely important that we give sites a way to work around this.

[Eric Seidel (no email)]

FYI, neither plexode.com or Hixie's dom viewer send the IE8 header at this time:

% curl -I http://www.plexode.com/cgi-bin/main.py
HTTP/1.1 200 OK
Date: Mon, 29 Jun 2009 07:03:40 GMT
Server: Apache/2.2.8 (Ubuntu) DAV/2 SVN/1.4.6 mod_fastcgi/2.4.6 PHP/5.2.4-2ubuntu5.6 with Suhosin-Patch mod_ssl/2.2.8 OpenSSL/0.9.8g
Content-Type: text/html

% curl -I http://software.hixie.ch/utilities/js/live-dom-viewer/
HTTP/1.1 200 OK
Date: Mon, 29 Jun 2009 07:04:41 GMT
Server: Apache/2.0.63 (Unix) PHP/5.2.6 mod_ssl/2.0.63 OpenSSL/0.9.7e mod_fastcgi/2.4.2 DAV/2 SVN/1.4.2
Accept-Ranges: bytes
Vary: Accept-Encoding
X-Pingback: http://tracking.damowmow.com/
Content-Language: en-GB-x-Hixie
Content-Length: 13791
Content-Type: text/html; charset=utf-8

[Eric Seidel (no email)]

I emailed Aaron Whyte (plexode) and Hixie to notify them of this pending change.  BTW, I'm totally in favor of this change.  Although I would prefer we added HTTP Header opt-in support before.... or at least really really soon after making this change.

[Ian 'Hixie' Hickson]

I'm not adding anything that's explicitly asking the browsers to follow the specs.

[Adam Barth]

Eric, do you have examples of where plexode.com and hixie's live dom viewer do the wrong thing with the auditor turned on?  I played with them for a few minutes and they seemed to work fine.

(You can test the auditor by enabling the pref in the above patch or by grabbing a trunk build of Chromium and passing the --enable-xss-auditor command line flag.)

[Adam Barth]

We need to fix Bug 26807 first.

[Eric Seidel (no email)]

Comment on attachment 31993 [details]
patch

No need for this to be in the review queue while blocked by a crasher.  Removing r=? for now.

[Adam Barth]

Comment on attachment 31993 [details]
patch

Marking for review again now that this is actionable again.

[Sam Weinig]

Comment on attachment 31993 [details]
patch

By the power of Grayskull!  r=me

[Adam Barth]

Comment on attachment 31993 [details]
patch

Clearing the review flag.  We're going to hold off landing this for a few days.

[Adam Barth]

Sending        WebKit/mac/ChangeLog
Sending        WebKit/mac/WebView/WebPreferences.mm
Sending        WebKit/win/ChangeLog
Sending        WebKit/win/WebPreferences.cpp
Transmitting file data ....
Committed revision 45740.