Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x-xss-protection should default to 0 (not 1; mode=block) #4327

Closed
davewichers opened this issue Jan 3, 2022 · 4 comments
Closed

x-xss-protection should default to 0 (not 1; mode=block) #4327

davewichers opened this issue Jan 3, 2022 · 4 comments
Labels
breaking changes Change that can breaking existing code security Issue with security impact

Comments

@davewichers
Copy link

davewichers commented Jan 3, 2022

This issue was raised long ago in #1770 and ignored. I'm raising it again.

If you look at a few modern discussions:
https://security.stackexchange.com/questions/253924/is-it-better-to-disable-x-xss-protection-header-or-set-the-header-as-x-xss-prote
https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#x-xss-protection-header

They both recommend disabling this header by default (i.e., setting it to 0). Can I ask you to revisit this decision and make this recommended change this time?

And when this is done, it should set the header to: x-xss-protection: 0 (rather than simply dropping the header entirely).

@davewichers davewichers added the support Questions, discussions, and general support label Jan 3, 2022
@devinivy devinivy added the breaking changes Change that can breaking existing code label Feb 17, 2022
@devinivy
Copy link
Member

We'll take this under consideration for the next major version of hapi 👍

@devinivy devinivy added security Issue with security impact and removed support Questions, discussions, and general support labels Apr 29, 2022
@devinivy
Copy link
Member

I have made a PR for this, and your review is welcomed on it @davewichers: #4352.

@davewichers
Copy link
Author

Looks good to me. But I'm not much of a JavaScript expert. You might want to add a comment near the default is '0' explanation to say 'as recommended by OWASP (with link)', or whatever, to provide a bit of rationale in the code.

@devinivy
Copy link
Member

devinivy commented Nov 7, 2022

Resolved with v21 #4386

@devinivy devinivy closed this as completed Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Change that can breaking existing code security Issue with security impact
Projects
None yet
Development

No branches or pull requests

2 participants