Closed wmertens closed 4 years ago
Apologies, but I don't see how csurf applies the double-submit technique as is being described in this merge request.
Quoted from OWASP:
Double Submit Cookie If maintaining the state for CSRF token at server side is problematic, an alternative defense is to use the double submit cookie technique. This technique is easy to implement and is stateless. In this technique, we send a random value in both a cookie and as a request parameter, with the server verifying if the cookie value and request value match.
I fail to see where csurf is comparing the contents of the _csrf
cookie, and that of another request parameter? Either I am looking at the wrong things, or csurf only validates either one of them. Thus doesn't implement the Double Submit Cookie technique.
Hi @wesselvdv sorry if the code is hard to follow. You're welcome to use a debugger to walk through the code to follow the flow if it help you verify, but it does indeed compare them. You can try it yourself by not submitting the value or altering the cookie value and then submitting the same request value, etc.
The comparison between the two values is occurring right here:
https://github.com/expressjs/csurf/blob/248112a42f36fc9a84a71b0f5d383a1e03813f54/index.js#L111
The secret
variable is the value from the cookie and the value(req)
function call gets the value from the request to compare against.
Hi @wesselvdv sorry if the code is hard to follow. You're welcome to use a debugger to walk through the code to follow the flow if it help you verify, but it does indeed compare them. You can try it yourself by not submitting the value or altering the cookie value and then submitting the same request value, etc.
The comparison between the two values is occurring right here:
https://github.com/expressjs/csurf/blob/248112a42f36fc9a84a71b0f5d383a1e03813f54/index.js#L111
The
secret
variable is the value from the cookie and thevalue(req)
function call gets the value from the request to compare against.
Awesome! I am glad it means I didn't understand it. Ignore my previous comment/rant!
This explanation would have prevented me from opening #195