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

Question on /csrf-token endpoint #49

Open
justin0108 opened this issue Jun 24, 2024 · 9 comments
Open

Question on /csrf-token endpoint #49

justin0108 opened this issue Jun 24, 2024 · 9 comments
Labels
discussion Keeping this open for future reference

Comments

@justin0108
Copy link

justin0108 commented Jun 24, 2024

Hi,
I am quite new to CSRF and was using your library for NextJS 14.
Can I check with regards to this portion of code fetch("/csrf-token"):

const handleClick2 = async () => {
    const csrfResp = await fetch("/csrf-token");
    const { csrfToken } = await csrfResp.json();
    ...

and middleware.ts

  // return token (for use in static-optimized-example)
  if (request.nextUrl.pathname === '/csrf-token') {
    return NextResponse.json({ csrfToken: response.headers.get('X-CSRF-Token') || 'missing' });
  }

is this endpoint /csrf-token suppose to be there in production as well? or this is just an example?

I am reading article which indicate CSRF token should not be accessed with AJAX which confuses me with your example.
Thanks

@amorey
Copy link
Member

amorey commented Jun 24, 2024

Typically the recommended way to share a csrf-token with the client is by it placing it in a <meta> tag in the html response but tokens transmitted in this way can be accessed from client-side javascript as easily as from an explicit /csrf-token endpoint so I don't see what the difference is:

const resp = await fetch('/page-with-csrf-token-in-meta-tag');
const html = await resp.text();
// use regex to parse `<meta name="csrf-token" content="xxx">` here...

One possible attack that they're trying to protect against is that if the /csrf-token endpoint's response can be read from a third-party website (e.g. CORS-enabled) then it can be used in an attack. But this is also true of any page that contains CSRF tokens.

CSRF protection is a deep field so I don't want to say with 100% certainty that you can disregard that comment but I can't see how the specific /csrf-token endpoint in the Next.js example is insecure. Of course, I'd love for a security researcher with more knowledge than me to give us their opinion in the comments.

Incidentally, if you want to move the CSRF token to the <meta> tag in the example you can modify the layout file like this:

// app/layout.tsx

import { Metadata } from 'next';
import { headers } from 'next/headers';

export async function generateMetadata(): Promise<Metadata> {
  const csrfToken = headers().get('X-CSRF-Token') || 'missing';
  return {
    title: 'edge-csrf examples',
    other: {
      'csrf-token': csrfToken,
    },
  };
}

export default function Layout({
  children,
}: {
  children: React.ReactNode;
}) {
  return (
    <html lang="en">
      <body>
        {children}
      </body>
    </html>
  );
}

And the handleSubmit() method like this:

const handleSubmit = async (ev: React.FormEvent<HTMLFormElement>) => {
  // prevent default form submission
  ev.preventDefault();

  // get form values
  const data = new FormData(ev.currentTarget);

  // get token from <meta> tag
  const metaTag = document.querySelector('meta[name="csrf-token"]');
  const csrfToken = metaTag ? metaTag.getAttribute('content') : 'missing';

  // build fetch args
  const fetchArgs = { method: 'POST', headers: {}, body: JSON.stringify(data) };
  if (csrfToken) fetchArgs.headers = { 'X-CSRF-Token': csrfToken };

  // send to backend
  const response = await fetch('/form-handler', fetchArgs);

  // show response
  // eslint-disable-next-line no-alert
  alert(response.statusText);
};

@justin0108
Copy link
Author

justin0108 commented Jun 24, 2024

Thanks for your detail explanation.
The OWASP link is very useful to help me better understand CSRF.

I was looking at their recommendation to get the token "Signed Double-Submit Cookie".
It seems that it is "safer" way to get the token via the cookie?
I am no expert in the security field either. Any guidance/comments in this area is much appreciated.

With CORS disabled in your example, I don't see how an attacker can gain access to the csrf token either.
So to me it seems both method is equally "safe"?

Method-1:

  • CORS disabled
  • client retrieve csrf token via GET
  • attacker cannot access token since CORS is disabled

Method-2:

  • server set csrf token in cookie (without HttpOnly)
  • client retrieve csrf token from cookie
  • attacker cannot access token since its different domain, will not be able to see the cookie

Is this understanding flawed?

@amorey
Copy link
Member

amorey commented Jun 24, 2024

This library implements the "signed double-submit cookie pattern" with a modification that instead of using a shared server-side secret, it uses a unique per-session secret (stored in a cookie). The advantage of this is that it doesn't require the developer to define an environment secret. The token is signed with the secret and both the secret (via cookie) and the token (via payload) must be present in the request and pass validation as described in the "signed double-submit cookie" pattern. Here are the relevant lines in the source code for reference:
https://github.com/kubetail-org/edge-csrf/blob/main/shared/src/protect.ts#L123-L146

With regards to "Method-1", if CORS is disabled then a GET /csrf route cannot be used by an attacker to forge a request from a third-party site (at least not by any method I can think of).

With regards to "Method-2", it sounds like you're describing a pattern that retrieves the token from the initial HTTP response. If so, then that will also be safe but you should keep in mind that typically cookies in the "double-submit cookie pattern" have HttpOnly enabled.

Going back to your original question, the main advantage of using a GET /csrf route in the Next.js example is that it allows the GET / route to be statically optimized at build time because the response HTML doesn't change. If this isn't important to you then you can use this example to include the csrf token in a <meta> tag in the response HTML:
https://github.com/kubetail-org/edge-csrf/tree/main/examples/next14-approuter-js-submission-dynamic

@justin0108
Copy link
Author

thanks @amorey for your prompt response.
Appreciate your effort in developing this library for NextJS

@amorey amorey added discussion Keeping this open for future reference labels Jun 24, 2024
@Eichner
Copy link

Eichner commented Feb 4, 2025

Hi @amorey

I just stumbled upon this question and was just wondering if the "signed double-submit cookie pattern"-CSRF protection with the server-secret being in stored at the client couldn't be bypassed.
So, if an attacker is able to set cookies for the client (e.g. XSS in a subdomain), he would be able to also choose the per-session secret that is used to generate a valid CSRF-Token for that user. Since he would then be able to generate valid CSRF-Token, he would also be able to perform a CSRF attack. Provided that the CSRF token is transmitted as a request parameter in the body.
Or am I missing something?

@amorey
Copy link
Member

amorey commented Feb 4, 2025

You're right to be concerned. The signed double-submit cookie pattern relies on the assumption that an attacker cannot set arbitrary cookies for the target domain. To prevent a compromised subdomain from setting a cookie for the parent domain, you should:

  • Make sure the cookie Domain attribute is set properly (Edge-CSRF default:'')

  • Use HttpOnly to prevent JavaScript from reading/modifying the cookie altogether (Edge-CSRF default: true)

You can modify Edge-CSRF cookie properties here. By default Edge-CSRF leaves Domain blank which tells browsers to restrict access to the host that the cookie was originally set on (e.g. only the parent domain).

For reference here are the general scoping rules for Domain and Path attributes:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#define_where_cookies_are_sent.

And here's an overview of HttpOnly:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#httponly.

@Eichner
Copy link

Eichner commented Feb 5, 2025

The possibility to set cookies via an XSS in a subdomain was a mere example. DOM-based cookie manipulation, HTTP Response Splitting / Header Injection or other weird coding errors might also lead to being able to set cookies.

But what I would rather like to get at is that storing the server-secret at the client renders the signed double-submit cookie pattern just as secure as the naive double-submit cookie pattern which OWASP advises against using.
I think it is a bit risky to rely on an attacker not being able to set cookies (or that the user changes defaults to secure values) in a library that is supposed to protect against CSRF.

If I'm not mistaken the library supports the CSRF-Token to be transported in the request body and in a custom header.
If only the custom header option were supported, a vulnerability that allows an attacker to set the csrf-secret-cookie would still not enable CSRF. This is because correctly configured CORS would prevent a CSRF request from being sent from an attacker's domain to the target domain.

@amorey
Copy link
Member

amorey commented Feb 6, 2025

Thanks, I have to think about it some more but you might be right that storing the secret client-side is effectively an implementation of the naive double-submit cookie pattern. However, I don't think that just moving the secret server-side is enough to properly implement the signed double-submit cookie pattern because that still leaves the client open to an attacker injecting a valid cookie + token obtained elsewhere. I think that to properly implement the recommended pattern, the csrf token needs to be tied to the user session. Is that correct or am I missing something?

With regards to your suggestion, requiring the client to submit the token via a header (or any other method that effectively rejects simple requests) would prevent form submission via <form> tags in general so it would eliminate an entire class of attacks but it would also prevent developers from using those common development patterns.

@Eichner
Copy link

Eichner commented Feb 6, 2025

I totally agree. Tying the CSRF-Token-Value to the user session would be the more secure implementation!

Make sense. I only had the security inspect in mind. But your suggested pattern server-side-secret + tying the value to the user session should also allow for the csrf-token to be sent in the request body.

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

No branches or pull requests

3 participants