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

Update: [CSRF] Reintroduce an overview of Double Submit Cookie with HMAC #1110

Closed
advename opened this issue Apr 2, 2023 · 6 comments
Closed
Labels
ACK_WAITING Issue waiting acknowledgement from core team before to start the work to fix it. HELP_WANTED Issue for which help is wanted to do the job. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet.

Comments

@advename
Copy link
Contributor

advename commented Apr 2, 2023

What is missing or needs to be updated?

The Double Submit Cookie is a very popular technique. Searching the web for CSRF mitigation techniques, nearly all sources points to the OWASP CSRF Cheatsheet page. Unfortunately, unless the Double Submit Cookie technique uses signed and session bound tokens, with e.g. HMAC, it is vulnerable (to my knowledge) to:

  • Session Hijacking, a type of MIT attack
  • Vulnerable subdomains or sibling domains (XSS or HTML injection)

(Reference: 1, 2)

However, this has led over the past years to a range of different (mis-)implementations of the HMAC Double Submit Cookie technique, due to the fact of the lack of knowledge of what value exists for what reason. I've commonly seen that the CSRF Token is generated with HMAC using a mix of the following values:

  1. A time-to-live (TTL) timestamp, creating short-lived tokens
  2. A session ID, binding the token to the session
  3. A static unique user value, such as the user id or email address
  4. A nonce, to add additional randomness to the token

For example, this GO CSRF package uses a combination of 1 & 3, but lacks the important 2.

I would imagine that these mis-implementation originated from an old version of the OWASP CSRF page, recommending to use "user's ID, a timestamp value and a nonce".

After extensive research, I've come to the conclusion that to effectively prevent the above-mentioned vulnerabilities, the HMAC should be built with at least:

Note
It's a common misconception to include timestamps as a value to specify the CSRF token Time-to-live (TTL). A CSRF Token is not an access token. They are used to verify the authenticity of requests throughout a session, using session information. A new session should generate a new token.
Source: https://stackoverflow.com/a/30539335

How should this be resolved?

I'd like to suggest to re-introduce the overview of the Double Submit Cookie with HMAC with implementation details, without being platform-specific.

@advename advename added ACK_WAITING Issue waiting acknowledgement from core team before to start the work to fix it. HELP_WANTED Issue for which help is wanted to do the job. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet. labels Apr 2, 2023
@mackowski
Copy link
Collaborator

@advename awesome issue, thanks for it. It looks good for me after the first read but I need to dive deeper into this.
@jmanico what do you think about this change?

@kwwall
Copy link
Collaborator

kwwall commented Apr 8, 2023 via email

@advename
Copy link
Contributor Author

@kwwall

Alright - I'll try to keep in mind what "nonce" is used for in the future :)

That said, I'd rather go with the CSPRNG in this case, as I'm trying to remove the idea of timestamps entirely and thereby prevent confusion.

And i hope it was a great night, that made you stay up till 2AM ;)

@kwwall
Copy link
Collaborator

kwwall commented Apr 15, 2023

@advename - I'd prefer to recommend CSPRNG as well. You seldom will go wrong that way.

Regarding 2am nights, those ended on 4/13 when I finally got ESAPI 2.5.2.0 out the door.

@advename
Copy link
Contributor Author

advename commented May 26, 2023

@mackowski, I believe this issue and #1111 are related. Should we solve both within this issue?

@advename
Copy link
Contributor Author

advename commented May 26, 2023

Moreover, I thought about introducing the term "Naive Double Submit Cookie" to make the patterns more distinctive:

  • "Double Submit Cookie"
    • "Naive Double Submit Cookie" (Double submit cookie without any signing or session-dependent value)
    • "Session-dependent signed Double Submit Cookie"
      • "HMAC CSRF Token" (Explain the correct concept and show a pseudocode example)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACK_WAITING Issue waiting acknowledgement from core team before to start the work to fix it. HELP_WANTED Issue for which help is wanted to do the job. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet.
Projects
None yet
Development

No branches or pull requests

4 participants
@kwwall @advename @mackowski and others