CSRF Query
gabeio opened this issue · 18 comments
Why does lusca not try and get the csrf token(usually in body) from req.query also? There is no difference in the delivery of the correct csrf token as the cookie is the other half of the security.
Hmmm, proxies and web servers might log the URL. I am not sure if exposing the csrf tokens like that is a good idea. -1 from me.
The csrf cookie can only be set by the origin domain which is needed to validate a good token.
I can think of a couple of reasons.
The biggest one would be that query params are passed in referer headers. You'd be exposing the token to 3rd parties. Exploiting that would be pretty simple, particularly if you accepted user-generated content (links).
Next, accidental replay or 3rd party replay. The user can accidentally retrigger an action by using their back button. 3rd parties can retrigger by using window.history.back.
Additionally, as @thefourtheye mentioned, server logs and browser history. You'd still be safe in the sense that you're only giving up one of two factors, but I'd rather give up zero.
The user can accidentally re-trigger an action by using their back button.
The token should be invalid by then. Unless this library doesn't invalidate the token after it's been used.
Proxy server logs can record whatever they wish including headers and bodies so that's here nor there.
The token should be invalid by then. Unless this library doesn't invalidate the token after it's been used.
We do not. Tokens are valid for the life of the session. This behavior abides by the OWASP recommendation and mimics the behavior of csurf.
As a side note for chrome at least when users hit the back button (if it's
a post request) asks the user if they wish to resend the request (though I
might be wrong this might be refresh). So either way the user can do the
replay.
On Mon, Aug 10, 2015 at 13:42 Jean-Charles Sisk notifications@github.com
wrote:
We do not. Tokens are valid for the life of the session. This behavior
abides by the OWASP recommendation
https://www.owasp.org/index.php/CSRF_Prevention_Cheat_Sheet and mimics
the behavior of csurf
https://github.com/pillarjs/csrf/blob/fa5f9877abb2991dfaf34ab9d7eb75a0fc9aabf8/index.js#L120-L139
.—
Reply to this email directly or view it on GitHub
#61 (comment).
This can be summarized fairly succinctly with this:
OWASP recommends against it. They have several paragraphs dedicated to the subject.
The ideal solution is to only include the CSRF token in POST requests and modify server-side actions that have state changing affect to only respond to POST requests. This is in fact what the RFC 2616 requires for GET requests. If sensitive server-side actions are guaranteed to only ever respond to POST requests, then there is no need to include the token in GET requests.
I am not inclined to violate their recommendation.
Okay I was just wondering. I've been helping around multer and a question came up about how to handle csrf now that multer needs to be mounted at route level and lusca users stated that my suggestion to try query variable for csrf wasn't working though csurf was. How would you recommend using csrf with multer then? expressjs/multer#195 & expressjs/multer#184 . Thank you for the owasp references & reasoning.
Ah. Gotcha. Thanks for framing the problem for me.
Let me give it some thought today and get back to you. I'm certain we can come up with something.
I also wanted to see why lusca didn't, and csurf did allow query-string. It seemed strange that one would allow it and the other wouldn't...
Leaving this issue open for now but continuing the conversation in expressjs/multer#195 until I can provide a functional solution.
@jasisk I was wondering if there was any update regarding this issue. I'm using lusca together with multer and have the same problem with 'CSRF token missing' in multipart/form-data forms. I could potentially switch to csurf and use the workaround with getting the token from the query string, or split my forms into two (one for data, one for files) and disable lusca for the file upload form route, but neither is ideal given the current state of my project.
Any suggestions/help would be greatly appreciated. Thank you very much in advance.
The best solution is still to ensure multer parses the body before the csrf check. There's an example of that in the multer thread. If using kraken, multipart parsing (by means of formidable) is one of the preconfigured middleware so there should be no additional need for multipart-parsing middlewares (and you can always disable the one that ships with kraken though you'd be advised to reuse the priority number of 80 from the preconfigured one).
Many thanks for the prompt response. If I go down the route proposed in the multer thread and it's a mixed form (text inputs + file), does it mean that the form will still be protected from CSRF? Or does it basically skip the lusca CSRF check? (My apologies if this is a silly question, I'm new to this.) I'm not using entire kraken suite, just lusca.
Not a silly question. 😀
Yes, it will still be protected if the body is being parsed in the right order (multipart parsing followed by csrf checking).
Accord to the multer readme:
Multer adds a
bodyobject and afileorfilesobject to therequestobject. Thebodyobject contains the values of the text fields of the form, thefileorfilesobject contains the files uploaded via the form.
In other words, after multer does its work, the non-file fields (including the _csrf input you submit) will be on req.body which is the what lusca expects to find. So you're good. :)
That's awesome. Thank you so much, I really appreciate your help!
My pleasure. 😀