Skip to content

Commit

Permalink
Fixing CSRF by reverting the (well meaning) changes made in PR 3756 (#…
Browse files Browse the repository at this point in the history
…3756). I don't know if this will resurface issue 3420 (#3420), which the PR apparently addressed.
  • Loading branch information
molomby committed Oct 16, 2017
1 parent 9a1f9a1 commit 358857e
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/security/csrf.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,13 @@ exports.requestToken = function (req) {
return req.headers[exports.XSRF_HEADER_KEY];
} else if (req.headers && req.headers[exports.CSRF_HEADER_KEY]) {
return req.headers[exports.CSRF_HEADER_KEY];
} else if (req.cookies && req.cookies[exports.XSRF_COOKIE_KEY]) {
return req.cookies[exports.XSRF_COOKIE_KEY];
}
// JM: If you think we should be checking the req.cookie here you don't understand CSRF.
// On pages loaded from this app (on the same origin) JS will have access to the cookie and should add the CSRF value as one of the headers above.
// Other pages, like those created by an attacker, can still create requests to this app (to which the browser will add cookie information) but,
// since the calling page itself can't access the cookie, it will be unable to add the CSRF header, body or query param to the request.
// The fact that we *don't* check the CSRF value that comes in with the cookie is what makes this CSRF implementation work.
// See.. https://en.wikipedia.org/wiki/Cross-site_request_forgery#Cookie-to-header_token
return '';
};

Expand Down

0 comments on commit 358857e

Please sign in to comment.