search

krakenjs / lusca

Application security for express apps.
Other
1.79k stars 123 forks source link

Add support for X-XSS-Protection header. #14

Closed mstuart closed 10 years ago

mstuart commented 10 years ago

Add support for X-XSS-Protection header.

Although it's only useful for older IE browsers (IE8), it's still an important feature. It's still used in the wild today. You see Twitter, Facebook, and Google have it. We will enable this by default.

MEOWWWW

LOL

jeffharrell commented 10 years ago

Ironically, I had left this out originally because it wasn't supported outside of IE. I just did a second look and it may also be in Chrome (although it's unclear).

I'm not a fan of iexss for the naming though. Can you rename it to something like xssProtection?

mstuart commented 10 years ago

Sure no problem. Thanks!

mstuart commented 10 years ago

Yeah it was a little unclear to me too. It looks like it was introduced many versions back, but no mentions of it since. I think I was heavily inspired by helmet's iexss option :)

Ok, ready to merge now!

mstuart commented 10 years ago

Looks like your refactoring PR #13 may cause a few more changes to be made here. Pull that in first and I can patch this commit up to match the new pattern.

totherik commented 10 years ago

I'm not totally up-to-date on MSFT blog posts from 2008, but should the header be configurable with the current hardcoded value as a sane default?

mstuart commented 10 years ago

Looks like you can also set it to "0" (same thing as not including the header) or "1" which opts you into it in your intranet zone setting in Settings > Internet options. Our hardcoded default is the best option, but I'll update the PR so it's configurable.

Source: http://blogs.msdn.com/b/ieinternals/archive/2011/01/31/controlling-the-internet-explorer-xss-filter-with-the-x-xss-protection-http-header.aspx

mstuart commented 10 years ago

@totherik Ok, made it configurable with a sane default. Ready to merge

lmarkus commented 10 years ago

You need to update the documentation too. It doesn't show that you can pass an optional argument to it.

mstuart commented 10 years ago

Ah good catch. Adding that now

mstuart commented 10 years ago

K! /cc @lmarkus

lmarkus commented 10 years ago

:+1:

jeffharrell commented 10 years ago

If 0 is the same as no header, I'm not sure there's any benefit to including a param for it, but I guess it doesn't hurt. I'm good with this as well.

Mark, I'll merge it and then update it when I rebase my other PR. Thanks for the contribution!