search

expressjs / csurf

CSRF token middleware
MIT License
2.3k stars 217 forks source link

Update readme to highlight the danger of ignoring CSRF checks #84

Closed scottymcribs closed 8 years ago

scottymcribs commented 9 years ago

The current advice in the "Ignoring Routes" section of the README is dangerous because it assumes that authenticated sessions are enough to prevent cross-site scripting. I've made a quick change to caution future users against bypassing CSRF for most requests.

Thanks, Rob.

gabeio commented 9 years ago

You removed the part about these calls being fully authenticated and rate limited. Which is the entire point behind allowing users to sent requests without CSRF.

Might want to also BOLD the dangerous part so users really can't ignore it.

scottymcribs commented 9 years ago

Hey Gabe,

For "fully authenticated" do you mean authenticated sessions, or requests that have auth tokens attached to it as well?

CSRF tokens should also be used for all requests from users with only session authentication because web browsers will include all of your website's cookies with any request to your website, regardless of what webpage the user is on. For example, if I'm logged into github and an malicious webpage creates a post requests to github then my github cookies will be added to the request. If github didn't validate the CSRF token on every request, then the malicious webpage would be able to run any request they want.

The rate limiting part is separate from CSRF checks/attacks and didn't seem relevant to the section.

I modified and bolded the warning in the README.

gabeio commented 9 years ago

Most API's (that I've seen) require the auth tokens directly attached for all the requests. Yes I am not disputing your changes so yeah, no web browser should be hitting these routes directly ever (maybe with javascript) but the auth token should still be attached (somehow).

scottymcribs commented 9 years ago

Your right that most servers developed as pure API servers require username/password attached to all request. The original text just worried me because someone developing a webserver with some endpoints exposed for frontend AJAX calls may read the original advice and think that they don't need CSRF tokens on their api endpoints because their users are already authenticated via a login page.

dougwilson commented 8 years ago

Thank you! I can't believe it took this long to get merged, totally lost track of this :) I altered the wording a bit to be less yell-y.