Skip to content

Html.AntiForgeryToken() adds duplicate X-Frame-Options headers if called more than once #7

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

Closed
Eilon opened this issue Jul 23, 2016 · 2 comments
Assignees

Comments

@Eilon
Copy link
Member

Eilon commented Jul 23, 2016

Original bug: https://aspnetwebstack.codeplex.com/workitem/2057


System.Web.WebPages v.3.0.0.0:
Sometimes, more than one anti-forgery token is needed in a page (when there are multiple forms). Calling Html.AntiForgeryToken() causes the "X-Frame-Options: SAMEORIGIN" header to be emitted once per call. In extreme circumstances, this can lead to webserver/proxy malfunctions.


I encountered this issue on a page that uses the Anti-Forgery token within the row for each item in an index view.

The HTTP header was too large and MSIE aborted the connection when the list view was displaying roughly 500 records (selectable view size). Using Fiddler, I noticed the "X-Frame-Options: SAMEORIGIN" header was output for each call to Html.AntiForgeryToken(). The header wasn't output for like this when using Firefox and Chrome. In fact, it only seemed to be output when using MSIE. I was able to change the User Agent in Firefox and Chrome to identify itself as MSIE which reproduced the issue.

I realize there is more than one way to do things, but rather than rewrite the implementation in that view, I came up with a temporary workaround. For each Html.AntiForgeryToken() call made in the list loop. I removed the header eg:

foreach (var item in collection) {
...
    using (Html.BeginForm("Delete", FormMethod.Post, new { id = item.Id }) {
        @Html.AntiForgeryToken()
        // Temporary fix
        Response.Headers.Remove("X-Frame-Options");
    }
...
}
@Eilon Eilon modified the milestone: 5.2.4 Jul 23, 2016
@dougbu dougbu self-assigned this Sep 21, 2017
@dougbu
Copy link
Member

dougbu commented Sep 21, 2017

Self-assigning because I'll be merging #50.

dougbu pushed a commit that referenced this issue Sep 21, 2017
- #7

* According to RFC7034, only these three values, DENY, SAMEORIGIN and ALLOW FROM are valid values and they are mutually exclusive; that is, the header field must be set to exactly ONE of these three values.   This will prevent the CSRF code from inserting it multiple times as well as duplicating it if it was already set elsewhere (e.g. IIS Header)
* Changed var to const string per request.
* Changed const name to avoid SA130 error
* Changing to correct cost naming per standard
@dougbu
Copy link
Member

dougbu commented Sep 21, 2017

11a9624
Thanks very much @chekm8!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants