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

Fix when WhiteListedAllowFromStrategy returns "DENY" #4559

Closed

Conversation

nathanCFRA
Copy link
Contributor

Originally, if the return from getAllowFromValue(request) is DENY,
then the X-Frame-Options header's value will proceed to be written as
ALLOW FROM DENY - an invalid value.

This commit adds a condition in the if clause that checks whether
allowFromValue is DENY. This way, the X-Frame-Options header will be
written as ALLOW FROM [origin] or DENY.

If XFrameOptionsHeaderWriter is used with the strategy WhiteListedAllowFromStrategy, then any supplied origin that is not a direct match on the provided white list should result in DENY being written. But stepping through the code will show that if there is no match, getAllowFromValue() will return DENY, then will proceed to be concatenated with ALLOW FROM, resulting in ALLOW FROM DENY.

If the supplied origin has a match on the white list, then result is as expected.

Originally, if the return from getAllowFromValue(request) is "DENY",
then the X-Frame-Options header's value will proceed to be written as
"ALLOW FROM DENY" - an invalid value.

This commit adds a condition in the if clause that checks whether
allowFromValue is "DENY". This way, the X-Frame-Options header will be
written as "ALLOW FROM origin" or "DENY".
@pivotal-issuemaster
Copy link

@nathanCFRA Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@nathanCFRA Thank you for signing the Contributor License Agreement!

@rwinch rwinch self-assigned this Oct 30, 2017
@rwinch rwinch added type: bug A general bug in: web An issue in web modules (web, webmvc) labels Oct 30, 2017
@rwinch rwinch added this to the 5.0.0.RC1 milestone Oct 30, 2017
rwinch added a commit that referenced this pull request Oct 30, 2017
@rwinch
Copy link
Member

rwinch commented Oct 30, 2017

Thanks for the PR @nathanCFRA! I merged this in via 02a78b1 and polished it via 93ac706

@rwinch rwinch closed this Oct 30, 2017
@rwinch rwinch changed the title Add check to see if return value is DENY Fix when WhiteListedAllowFromStrategy returns "DENY" Oct 30, 2017
thomasdarimont pushed a commit to thomasdarimont/spring-security that referenced this pull request Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants