Custom CSRF protection in Express / Node

Question

Please review my code rather than telling me to use a trusted NPM package like csurf.

What are some potential weaknesses with the following implementation?

'use strict'

const   express = require('express'),
        router = express.Router(),
        crypt = require('../services/crypt')


router.use((req, res, next) => {
    if (req.session.csrfToken === undefined) {
        req.session.csrfToken = crypt.randHex(64) // this uses Crypto.randomBytes(); gives a hex of length 64
        // alt: res.locals.csrfToken = crypt.hash('sha256', req.session.csrfToken + req.headers.host) // also using Crypto
    }

    res.locals.csrfToken = req.session.csrfToken
}

module.exports = router

I put the res.locals.csrfToken in all forms with method="post", and this middleware is always used:

'use strict'

const   express = require('express'),
        router = express.Router()


router.post('*', (req, res, next) => {
    if (req.session.csrfToken !== req.body.csrfToken) {
    // alt: if (crypt.hash('sha256', req.session.csrfToken + req.headers.host) !== req.body.csrfToken) {
        let err = new Error('Invalid CSRF Token!')
        err.status = 403

        return next(err)
    }

    return next()
})

module.exports = router

I am using HTTPS, and my express-session options look something like this:

{
    name: 'sessid',
    resave: false,
    saveUninitialized: false,
    secret: 'djt84nq7y4tnvwosufvgsudih3rve783vdk'
}

Will the alternative (alt) implementation help any?

Answer 1

Not a node expert, but it looks like you cover the basics. CSRF protection via token is standard practice and very reliable. You are generating your token via a cryptographically secure random number generator. You are leaving it in the session and making sure it is present on all post requests. The only way an attacker can break it is via XSS attack, but XSS attacks are able to defeat any CSRF protection, so that isn't your fault (just make sure that you are practicing defense in depth), nor will CSRF vulnerability be your biggest problem if you have an XSS vulnerability. Your weaknesses are the same as any CSRF implementation:

1. Failure to actually implement your CSRF protection on all pages. Most common cause of CSRF vulnerabilities is either people trying non-standard protection methods (which you are not doing), or inadvertently leaving actions out of CSRF protection.
2. You can accidentally leave action URLs unprotected if you somehow build a URL that doesn't use your CSRF middleware or if you inadvertently allow GET requests to execute actions, in which case the CSRF protection will be explicitly ignored by your code. So the biggest weakness is the same weakness that everyone has: you have to follow best security practices everywhere else in your code. Forget it one place and your CSRF protection disappears. If you have a valuable enough website someone will find the vulnerability.

In terms of improvements, the only reasonable addition would be to change your CSRF token after every request. Many systems implement this additional layer of security. It is not clear to me if this additional security step has any practical benefits, but if it isn't difficult to implement it certainly won't hurt to include it. In my own implementations in the past I've used it for additional UI help: I keep track of past CSRF tokens and if I see a duplicate come in (which is usually caused by the user hitting "submit" twice on a form), I ignore the second request and warn the user of their error.

You could also consider a dual-protection using referrer and origin headers, but those have mixed results due to incomplete browser support. Most frameworks I have seen stick with a token-only protection.

I don't think your alternate implementation adds anything. Throwing in a sha256 hash certainly doesn't change anything, because hashing doesn't introduce any additional randomness. It just makes things look more random. The host header doesn't inherently add additional security, IMO. This is very similar to an attempt to add protection via the ORIGIN or REFERRER headers. In this particular case, I don't think a more complicated algorithm is worth the (dubious) increase in security. If you did want to also verify such a header then I would ditch the hash: just store it in the session in plain text and check it on the way back up. However, you probably don't have to store it in the session either: most of the time you should know what the host, origin, or referrer should be. No need to store it to check it later.