If the csrf cookie is changed manually after it is set, the application crashes even with get request and the error does not go to the handler

[Sertturk16]

ForbiddenError: invalid csrf token
at doubleCsrf (/Users/omersertturk/Desktop/repositories/id/node_modules/csrf-csrf/lib/cjs/index.cjs:17:61)

[psibean]

Going to need more details for this one.

If the csrf cookie is changed manually

Changed how? Can you provide some code examples? Typically you shouldn't be manually changing the cookie in the first place. If you need to change some of the properties on the cookie, the feature to support doing that will likely be provided via fixing issue #46

the application crashes even with get request and the error does not go to the handler

Exactly which line of code are you seeing cause the crash? If you look at the middleware:

  const doubleCsrfProtection: doubleCsrfProtection = (req, res, next) => {
    req.csrfToken = (overwrite?: boolean, validateOnReuse?: boolean) =>
      generateToken(req, res, overwrite, validateOnReuse);
    if (ignoredMethodsSet.has(req.method as RequestMethod)) {
      next();
    } else if (validateRequest(req)) {
      next();
    } else {
      next(invalidCsrfTokenError);
    }
  };

You'll see that for a get request (by default), the middleware is entirely skipped, there is no access being made to the cookie. The only way the cookie is used on a get request is if you have overridden the ignoredMethods, or you're using the validateRequest on GET requests yourself.

ForbiddenError: invalid csrf token

You've posted this error. How are you getting that error if it isn't going to the handler? Depending on the order of your middleware registration, it's possible the error is being passed to the default handler instead of your own.

It's really difficult to have any insight here without additional details.

Ideally a fully reproducible example

[Sertturk16]

Here is my config:

export const { generateToken, doubleCsrfProtection } = doubleCsrf({
  cookieName: '_csrf',
  cookieOptions: {
    secure: true,
    httpOnly: true
  },
  getSecret: () => env.CSRF_SECRET,
  getTokenFromRequest: (req) => req.body._csrf
})

And here is my route:

export const getLogin = [
  rateLimiter('login.get'),
  saveUTMTagsToSession,
  setTheme,
  async (req, res, next) => {
    if (!req.user) {
      return res.render('login', {
        returnTo: req.session.returnTo,
        csrfToken: generateToken(req, res)
      })
    }

    next()
  },
  emailVerificationMiddleware,
  twoFactorAuth(ENABLED_TWO_FA_METHODS.LOGIN),
  kycMiddleware
]

after getting login page, if I change _csrf cookie manually, app immediately crashes even if I don't use doubleCsrf middleware just because of "generateToken". Its not prevented with error handling.

[Sertturk16]

export const getLogin = [
  ...
  async (req, res, next) => {
    if (!req.user) {
      return res.render('login', {
        returnTo: req.session.returnTo,
        csrfToken: generateToken(req, res)
      })
    }

    next()
  },
  (err, req, res, next) => {
    console.log(err)
    next(err)
  },
  ...
]

I can't even console the error here

[Sertturk16]

1- Using like "generateToken(req, res, true)" fixes the issue since cookie will always be recreated.
2- After cleaning cookies manually fixes the issue.

[psibean]

Edit: See below, since you're in a promise based context, the error not bubbling or being caught is the native behavior of express v4. It's documented in their error handling docs.

Hey @Sertturk16 based on the example code you've provided so far, it's evident where the error is occurring.

As part of the generateTokenAndHash call that happens when you call generateToken there is this code:

    // If ovewrite is true, always generate a new token.
    // If overwrite is false and there is no existing token, generate a new token.
    // If overwrite is false and there is an existin token then validate the token and hash pair
    // the existing cookie and reuse it if it is valid. If it isn't valid, then either throw or
    // generate a new token based on validateOnReuse.
    if (typeof csrfCookie === "string" && !overwrite) {
      const [csrfToken, csrfTokenHash] = csrfCookie.split("|");
      if (validateTokenAndHashPair(csrfToken, csrfTokenHash, possibleSecrets)) {
        // If the pair is valid, reuse it
        return { csrfToken, csrfTokenHash };
      } else if (validateOnReuse) {
        // If the pair is invalid, but we want to validate on generation, throw an error
        // only if the option is set
        throw invalidCsrfTokenError;
      }
    }

So you should be able to do this: generateToken(req, false, false) and this will work without overwriting the token, and it won't throw the error. Instead, it will ignore the fact that your existing token pair is invalid due to being manually changed, and it will then overwrite the manually changed cookie with the correct token from the resolved session state.

This part is expected behaviour.

I asked the following questions, which remain unanswered:

1. How are you changing the token manually, and to what? Can you share a code example.
2. Why and for what reason? The middleware essentially keeps track of the cookie value via the provided session state resolver, tinkering it is always going to be problematic.

So, the only issue I'm seeing here is that for some reason the error being thrown from generateToken isn't bubbling up to the error handler for some reason, without a 100% reproducible repo I'll have to put something together myself to figure that part out.

Edit: Hey I just noticed your running generateToken in an async function. Are you using express v5? Express v4 does not support error handling in async middleware. If you're using express v4, errors inside promises don't bubble to the error handler. This is native express v4 behavior. Please see the official docs on express and async error handling

  async (req, res, next) => {
    if (!req.user) {
      return res.render('login', {
        returnTo: req.session.returnTo,
        csrfToken: generateToken(req, res)
      })
    }

    next()
  },

Why are you using async here? If you remove async, it will work, otherwise you need to handle the error yourself, as per the official express docs. You would have to do this:

  async (req, res, next) => {
    try {
        if (!req.user) {
            return res.render('login', {
                returnTo: req.session.returnTo,
                csrfToken: generateToken(req, res)
            })
        }
        next()
    } catch (error) {
        next(error);
    }
  },

This isn't a csrf-csrf bug. This is the intended / expected behavior of express.

[psibean]

As there have been no replies and the issue seems to be the expected behavior, I'm closing this issue for now.