Comments (22)
In fact, the only times I see people asking security-related questions in projects is because they are wondering how to do something that is not secure at all and wondering why they are not being allowed to (or how it is hard to make it insecure).
from csurf.
meh. a lot of methods like GET, HEAD, and DELETE shouldn't even have bodies.
from csurf.
Right, but they can, which may lead to someone sending a body with a GET request and (if the user incorrectly placed method-override after this module) could be interpreted as a POST. I'm proposing the check to be
if (hasbody(req) || ('GET' !== req.method && 'HEAD' !== req.method && 'OPTIONS' !== req.method)) {
checkCSRF()
}
from csurf.
FWIW this is a response to http://blog.nibblesec.org/2014/05/nodejs-connect-csrf-bypass-abusing.html
From email, since yahoo's being stupid and not sending to @jonathanong
(dougwilson)
I would like to draft a security section into the csurf readme, any objections? My thoughts are to outline the general problem of placing the middleware above logic that alters req.method (and specifically calling out method-override).
In addition, what do you think about altering the module such that instead of just ignoring specific methods, we also never ignore requests that have a body?
Just because they shouldn't have bodies doesn't mean that they won't.
from csurf.
my general philosophy is to educate vs. handle all these cases. i'd rather add that link to the readme and tell developers what they should and shouldn't be doing.
but if we add this in and somebody complains, we could always tell them, "stop". haha
from csurf.
Unfortunately when it comes to security, people can shoot themselves in the foot, but we should at least make them try harder to do this. Currently all it takes is
app.use(csrf()) // #1
app.use(methodOverride()) // #2
// explode!
We can always go around chanting "be sure method-override comes before csrf" but that doesn't seem very proactive to me. If all it took was education, those blog articles may not exist.
but if we add this in and somebody complains, we could always tell them, "stop". haha
people never ask questions and just copy-pasta all day long. once one example on the interweb does csrf before method-override, it'll just be everywhere.
from csurf.
yup. what we can do is just 403
if the original method is idempotent and not support request bodies on these methods. if they want to do something insecure, they should do it themselves.
we could also change body-parser to do the same.
from csurf.
what we can do is just 403 if the original method is idempotent
I think I like this better than just checking the CSRF token and carrying on, though if a method is idempotent would need to be a whitelist instead of a blacklist to be safe...
from csurf.
@visionmedia koajs/koa#274 can you move in the methods
lib here so we can do some awesome over engineering?
from csurf.
Once issue I can see, though, is that DELETE
and PUT
is defined to be idempotent, but I'm sure people would like to CSRF token check on those methods; I'm no longer sure that method idempotentency directly maps to what should check the CSRF token.
from csurf.
I feel like I have circled back around to liking my original proposal better again...
from csurf.
lol, I just thought of a crazy new idea: check the CSRF token how it is, but then set req.method
to no longer be a writable property :P
from csurf.
I think I like the idea of the original check returning a 403
the most.
from csurf.
i'm so confused now. haha. i'm down for that too. haha
from csurf.
i'm so confused now. haha. i'm down for that too. haha
lol, sorry. just brainstroming over here :P
I think I like the idea of the original check returning a 403 the most.
@Fishrock123 which check was that (you can always quote it is link to it)?
from csurf.
@dougwilson -- checking for a body.
if (hasbody(req) || ('GET' !== req.method && 'HEAD' !== req.method && 'OPTIONS' !== req.method) {
checkCSRF()
}
from csurf.
So to continue this real quick, if I were to say add the hasbody
check, should there be an option to turn it off? i.m.o no, because people already cannot influence that list of methods. Does this make sense to you guys?
from csurf.
If any option, I'd add an option to still checkCRSF()
or just 403
. I dunno if we should be that giving even that leeway though.
This is CSRF prevention, it's going to be strict.
from csurf.
Fair enough, @Fishrock123 that sounds good to me. csurf
2.0, here we come!
from csurf.
By the way, having these modules not included in connect/express is AWESOME. We can make breaking changes in them, bump the major version, and not care!
from csurf.
If we are talking 2.0, docs needs to be landed first for sure. #7
from csurf.
@Fishrock123 I'll make a milestone in here.
from csurf.
Related Issues (20)
- Feature add 'Encrypted Token Pattern' HOT 3
- Add credentials warning to documentation HOT 7
- A way of getting csrfToken through POST request HOT 3
- Cannot validate CSRF token using the example code HOT 4
- Can docs clarify how cookie mode works? HOT 3
- please document the `signed` config option HOT 4
- Disable CSRF checking during tests HOT 1
- previous token still valid HOT 1
- Token Lifetime HOT 2
- Need docs and examples for working with single page application. HOT 3
- BREACH attack mitigation HOT 2
- No regeneration of secret when a valid token is submitted HOT 2
- A cookie secret is not really secret HOT 1
- Upgrade to [email protected] for SameSite=None support HOT 1
- Best practice for the csrf token and secret (signed? httponly?) HOT 1
- User's CSRF Token is invalid but doesn't look like so HOT 7
- New token secret with every request HOT 3
- Update docs to address situations with mixed protection approaches HOT 1
- How I can validate csrf token one time with only a request
- Failed on validation when using with 2 backends
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from csurf.