-
-
Notifications
You must be signed in to change notification settings - Fork 119
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
What's the attack vector on /csrf? #6
Comments
/csrf
?
+1 I don't quite understand the reasoning behind that as well. |
+1 |
+1 |
if anyone wants to maintain this, i can add you as a collab. haven't had time to revise this yet. |
I'm not a security expert, but I can contribute to discussion, and help keep this repo alive if needed. |
Best way to start is with PRs, of course : |
@marfire @jonathanong @bradleyflood @frutality @SEAPUNK when we decided to add csrf support to Sails core, we exposed a Here's an attempt to summarize that word salad above into a more straightforward format:
Happy to dig into this more if it would help anybody out (if I don't respond, just remind me on twitter). |
(One other major caveat is that nasty browser plugins and/or compromised native software running on a computer completely circumvent everything I discussed above) |
JSON hijacking is one of the possible attack vectors. See this stackoverflow answer. If you are careful to follow the OWASP recommendation and always return an object such as |
JSON hijacking is also possible using a charset-based attacks. I just learned about it, but here's my attempt at an accurate summary. Attack:A malicious site loads your AJAX endpoint via <script> tag with a charset attribute. This causes the JSON to instead appear like JS accessing a global variable. The Mitigation:You must declare a charset in the "Content-Type" header returned by your server. This means the <script> tag's charset will be ignored. The AJAX endpoint that returns CSRF tokens must always declare a response header "Content-Type: application/json; charset=utf-8" Details:http://blog.portswigger.net/2016/11/json-hijacking-for-modern-web.html |
@cspotcode and this is by default turned on if we are using popular web servers, right? I mean, I checked my sites behind |
@frutality I'm not sure, but I think it would be good for this document to explain both those points: that charset is required on If nginx is reverse-proxying to an express server, are there common ways of returning JSON from a route that don't include a charset by default? Does express have a middleware to enforce charsets on all responses, throwing an error if it's absent? |
@cspotcode: The attack described in that article is dependent on bugs in the browser: "Conclusion: That's interesting and important information, but it doesn't seem to fit with the purpose of this page. As far as I can tell it's designed to present best practices based on the specified behavior of the various actors and protocols involved. If it's going to give advice based on browser bugs it will have to be much more involved and edited much more frequently, since the landscape is always changing. For example, since the article was written the bug in Chrome has been fixed, and presumably the others soon will be as well. |
Today still no clear answer in my mind: can we expose such rout for CSRF tokens for (e.g.) a |
In a couple places this document emphasizes the following point: Make sure CSRF tokens can not be accessed with AJAX! Don't create a /csrf route just to grab a token.
I don't see the attack vector here. Such an endpoint would be no more or less secure than the usual practice of embedding the token in a hidden form field. In both cases the Same Origin policy will prevent a foreign script from reading the value.
That is: in both cases a foreign script can send a
GET
to that URL; in both cases the user's authentication cookie will be included, causing the server to return a valid CSRF token; and in both cases the browser's Same Origin policy will prevent the foreign script from reading the response to theGET
, which keeps the token secure.The text was updated successfully, but these errors were encountered: