Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid calling 'unsign' in case if the token from cookie is 'undefined… #34

Merged
merged 1 commit into from
Dec 13, 2021

Conversation

vla-dev
Copy link
Contributor

@vla-dev vla-dev commented Dec 7, 2021

Avoid calling 'unsign' in case if the token from cookie is 'undefined'. If the first argument is different from 'string', the unsign function will always throw "Signed cookie string must be provided."

src/cookies/getCookie.ts

function getCookie(req: NextApiRequest, name: string): string {
  if (req.headers.cookie != null) {
    const parsedCookie = parse(req.headers.cookie);
    return parsedCookie[name];
  }

  return "";
}

This function checks if headers.cookie !== null and is trying to get the token from parsed cookie, but the cookie could be present but different from XSRF token.

Let's say I'm also using google analytics (gtag) that is making its own cookies such us: _ga=GA1.1.1798070841.1638877244;

parsedCookie will be:

parsedCookie = {
  "_ga"="GA1.1.1798070841.1638877244"
}

and the return statement parsedCookie[name] where name is tokenKey (by default XSRF-TOKEN) will be undefined

then...

src/middleware/csrf.ts

const tokenFromCookie = getCookie(req, tokenKey);
const tokenFromCookieUnsigned = unsign(tokenFromCookie, secret); 

since tokenFromCookie is undefined, and the first argument must be typeof string, this function will always throw "Signed cookie string must be provided." and the request will fail with status 500

…'. If the first argument is different from 'string', the unsign function will always throw "Signed cookie string must be provided."
@j0lvera
Copy link
Owner

j0lvera commented Dec 13, 2021

@vla-dev thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants