Update: [CSRF] Improving the new Double Submit Cookie sections from #1110

[advename]

What is missing or needs to be updated?

My previous PR to fix #1110 reintroduced an overview of the HMAC CSRF Token and added a Naive Double Submit Cookie and Encrypt Cookie sections.

The PR is merged into main, but there are some improvements left I believe should be addressed.

How should this be resolved?

Improvement 1

Uncertain if the pseudocode should be marked with code. Maybe we should use python since it's close to python syntax, taking advantage of highlighting.

Improvement 2

Personally, the new "Signed Double Submit Cookie" section does still not provide full mitigation against CSRF attacks, since it doesn't involve by default the idea of using session-dependent value. I would suggest instead of the header "Signed Double Submit Cookie", to use "Session-dependent Signed Double Submit Cookie", since a "Session-independent Signed Double Submit Cookie" is only a mitigation technique when combined with Referer validation to protect against:

• Vulnerable subdomains or sibling domains
• Man-in-the-middle (MITM) attack
• Open redirect vulnerability

Actually, the "Double Submit Cookie" & "Signed Double Submit Cookie" pattern without a session-dependent value is only a defense-in-depth (DiD) technique.

Only the following two are true mitigation techniques:

• Signed, session-independent, CSRF Token is combined with Referer validation. This is the CSRF mitigation technique of Django, as they have always utilized a signed, session independent, Double Submit Cookie) which by itself without Referer validation is vulnerable to both previously mentioned attacks.
• Signed, session-dependent, CSRF Token

I'm uncertain how to include easily the idea of "session-dependent" value without overwhelming the less experienced developers.

Furthermore, the "Naive Double Submit Cookie" is a defense-in-depth technique and should probably be moved into the Defense in Depth Techniques section? This would mean we have to restructure the "Double Submit Cookie" to introduce "Signed Double Submit Cookie" as the default from the beginning.

Improvement 3

There is actually a bug I just noticed in the pseudocode. The request.setCookie() should actually be response.setCookie() since we set the CSRF cookie in the response.

[mackowski]

Awesome @advename I agree.
I am not sure how much python will help in Improvement 1 but it is worth checking

[jmanico]

I think the double-submit cookie section is really "over built," and would love to see it simplified.

[advename]

@jmanico, do you have any suggestions?
I mentioned in #1101 about Collapsible Sections?
But how do you see Improvement 2 of my issue? Do you agree that naive Double Submit Cookie should be moved to DiD or what do you suggest?

[kiara-riley]

I like the new double-submit cookie section as someone that is attempting to learn about this. There are a lot of different approaches for different reasons and more information is certainly appreciated.

However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks?

Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

[jmanico]

I think the signed double-cookie submit is very confusing and would like it to go away, I think its very confusing and not necessary.

[jmanico]

I like the new double-submit cookie section as someone that is attempting to learn about this. There are a lot of different approaches for different reasons and more information is certainly appreciated.

However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks?

Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

If you care to submit a PR that removed that section, I'd take it!

[advename]

@jmanico
I think the signed double-cookie submit is very confusing and would like it to go away, I think its very confusing and not necessary.

That's what I outlined in this PR, and am still waiting for feedback on PR 1149. Collapsibles should help us to make it more straightforward and "stow away" the extensive contexts.

---

@kiara-riley
However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks?
Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

I have not yet had the chance to see any XSS vulnerabilities because of the sessionID knowledge. Nevertheless, I agree. My notes, from which I wrote the new sections from, also don't send the sessionID in the CSRF Cookie, but only, as you point out, the random value.
Would you mind opening a new issue or PR that fixes this in the pseudocode?

[jmanico]

Hey @advename can you give me a PR for one small thing at a time, maybe on PR for just the removal of signed double submit for start? It's a lot easier for me to process small changes as opposed to big design changes.

And when your collapsable is ready to go live let me know!

[advename]

@jmanico would love to. But i have still not received a reply to my previous questions.

[jmanico]

Hey @advename what questions are not answered? I don't have any thoughts on collapsable but am happy to answer questions about content. I don't think we need the signed cookie section at all, I'd rather just keep it simple and explain the basic ideas.

If there are pending questions you want me to answer, please ask me again here. I'm sorry to have missed them!

[siradam7th]

I like the new double-submit cookie section as someone that is attempting to learn about this. There are a lot of different approaches for different reasons and more information is certainly appreciated.

However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks?

Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

I was about to open an issue about this, then decided to see if anyone else mentioned it, so here I am.

As you mentioned, this way of generating a csrf token leaks the SessionID since it is recommended to set it to HttpOnly, but doing it this way, it makes such action useless, since it will be accessible by javascript easily from the csrf Cookie, by just spliting the csrf token string.

In summary, including the SessionID in the CSRF token generation by concatenating it defies the protections afforded by the actual SessionID cookie being set to HttpOnly.

[jmanico]

I also agree the sessionId derived token is a bad idea. Would someone like to remove this and submit a PR?

[murshex]

I like the new double-submit cookie section as someone that is attempting to learn about this. There are a lot of different approaches for different reasons and more information is certainly appreciated.
However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks?
Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

I was about to open an issue about this, then decided to see if anyone else mentioned it, so here I am.

As you mentioned, this way of generating a csrf token leaks the SessionID since it is recommended to set it to HttpOnly, but doing it this way, it makes such action useless, since it will be accessible by javascript easily from the csrf Cookie, by just spliting the csrf token string.

In summary, including the SessionID in the CSRF token generation by concatenating it defies the protections afforded by the actual SessionID cookie being set to HttpOnly.

Just saw your post now, and I ended up noticing the same thing and created an issue #1493

The implementation of the "signed double submit cookie" is fundamentally flawed. I'm genuinely surprised this was even approved.

Do you have anything to say for your broken implementation of csrf protection @advename ?

[advename]

@c0nd3v - I think we all agreed in this discussion that the session id should go. However, at the time of when I initially discussed this issue, I was waiting for some updates from the maintainers on collapsible sections for improved structure. Since that moved quite slowly and a crunch-time at work happened, I didn't spare much attention to this since then.

[murshex]

@c0nd3v - I think we all agreed in this discussion that the session id should go. However, at the time of when I initially discussed this issue, I was waiting for some updates from the maintainers on collapsible sections for improved structure. Since that moved quite slowly and a crunch-time at work happened, I didn't spare much attention to this since then.

Eh, its your pull request, isn't it? It's been up for a whole year and some more already. You have some responsibility in association with that. I sure hope nobody actually followed what you recommended in the mean time. Sorry to be harsh, but this is embarrassing and hurts the reputation of OWASP as an organization.

[advename]

No harshness taken!
I felt that I invested quite a lot of time then already and hoped that someone else would be able to remove those lines of code. Anyway, you are right, I'll make a PR ASAP to fix that particular issue.

[advename]

@c0nd3v feel free to chip in: #1513

[mackowski]

Thank you all!

[advename]

@mackowski , this issue is not completed. It mainly discussed the session id issue, but the initial topics are still open.

[goulashsoup]

@advename As I have read from this SO question, the section does not even mention that the created cookie needs to be send via HTTP request header or body (as it can not be read by a possible attacker). At least this is how I understood the answers to the linked question...

So, I suggest adding such a sentence explicitly (otherwise I have no idea what is "double signed" about the cookie if it is just sent by the browser as usual).

I also suggest (in general), first to explain how the pattern works from the logical order it applies in a request-response series from a time point where no session or cookie is established by the client (browser) too when the pattern is "finished":

1. The web app sends first request to target site.
2. The server creates a session cookie in SAID way (e.g. without HttpOnly) sends it with the first response.
3. For the next request, the web app reads the HTTP cookie and sends it in the HTTP header and request body (Also explain if this must be the case for all request kinds)
4. The server checks if the cookie is as expected from both HTTP header and the request body.

Then after an explanation, it can be explained in detail why this mitigates all kinds of attacks, or what attacks are still possible.

Just 1. explain the pattern and then 2. explain why it makes the application secure.

[teohhanhui]

The same pattern is documented here: https://en.wikipedia.org/wiki/Cross-site_request_forgery#Cookie-to-header_token

[goulashsoup]

@teohhanhui The wikipedia is way easier to understand, but it is just a reference to the Cross-Site Request Forgery Prevention Cheat Sheet.

[bencdelaney]

FWIW, I've also bounced from the cheat sheet, through the SO question mentioned above, and now to this issue.

I agree with @goulashsoup's suggestion that being very explicit about the sequence of operations could be helpful. There is already a similar section for synchronizer tokens, but it doesn't directly translate to double-submit patterns.

The current section is also light on request-time details. If I've parsed it correctly, the key difference between a Session-dependent Signed Double Submit Cookie and a Naive Double Submit Cookie is that the former is compared to a freshly-computed HMAC - in addition to the comparison between the cookie and the value from the body - on every request. However, I don't believe that extra comparison is stated explicitly in the cheat sheet.

[mackowski]

@jmanico @advename what do you think about recent comments here? I think we can re-write that to be easier to understand

[jmanico]

Can I please get @kwwall to look at this? He's solid of crypto and would like his eye on this.

[advename]

I agree with the recent comments. It's not easy to find the right balance of introducing the multiple recommended ways without overwhelming the reader with details, but that the token needs to be taken from the cookie and send via HTTP is an important detail which actually is the whole point of the Double Submit cookie pattern!

…

On Thu, 16 Jan 2025, 19.12 Jim Manico, ***@***.***> wrote: Can I please get @kwwall <https://github.com/kwwall> to look at this? He's solid of crypto and would like his eye on this. — Reply to this email directly, view it on GitHub <#1143 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHXU5ASYWUX32LBTYUTRMKT2K7ZCTAVCNFSM6AAAAABL5KEZPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOJWGM4TEMRXGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[goulashsoup]

@advename My first simple suggestion would be to first explain the Naive Double-Submit Cookie Pattern and it's flaws, then explain the Signed Double-Submit Cookie and why this mitigates the flaws of the "Naive" approach.

From there it can be extended with the sequence of operations later.

[advename]

Agree, though my initial concerns in this issue:

I'm uncertain how to include easily the idea of "session-dependent" value without overwhelming the less experienced developers.

The CSRF Cheat sheet is already quite massive. It takes around 30-45min to read its entirety. Many developers use it as a reference and some only want to see the best solution without understanding the reason behind it. Therefore, I had suggested the use of a collapsible, where we could throw the "Whys?" into, while keeping the Cheat sheet digestible enough.

I'd love to work on it, but won't be able to do so in the next months due to deadlines. If anyone wants to give it a go, feel free to make a PR @goulashsoup @bencdelaney @teohhanhui

[jmanico]

FYI: I started this project with a goal - over a decade ago - to keep each cheat sheet not as a comprehensive guide, but a brief as possible “cheat” to the problem that focuses on the solution for developers. I do not mean this arrogantly and I am always willing to have folks want to move in a different direction, but that was my goal when the project started. As soon as we get into collapsables and similar, that’s a clear sign to me that the cheat sheet is getting too big - and we need to split up or reduce the size of the cheat sheet.

[kwwall]

@jmanico wrote:

Can I please get @kwwall to look at this? He's solid of crypto and would like his eye on this.

I will take a look, but I am currently recovering from a root canal and still having pain from it, so it may be a few days. It's hard to focus on anything deep while in pain.

As soon as we get into collapsables and similar, that’s a clear sign to me that the cheat sheet is getting too big - and we need to split up or reduce the size of the cheat sheet.

I think it's more complicated than that because of how the CS series is used as references. For example, many of us use them as a reference to in vulnerability templates as references for providing details solutions to a given vulnerability.

But if we were to break the current Cross-Site Request Forgery Prevention Cheat Sheet into multiple separate cheat sheets, those of us who use these as references to solution references in vulnerability templates (and that's done by a lot of SAST and DAST tool providers) will either have to decide which one to pick or include them all. Either way, I think that's bad for developers as many of them are already overwhelmed. So I think we need to achieve a balance here.

Just my $.02.

[advename]

@jmanico wrote:

As soon as we get into collapsables and similar, that’s a clear sign to me that the cheat sheet is getting too big - and we need to split up or reduce the size of the cheat sheet.

I partially agree.

The cheat sheet has been around for years and has evolved from a simple "cheat sheet" into a comprehensive knowledge base of valuable information. In fact, it has likely outgrown the traditional definition of a cheat sheet quite some time ago.

By incorporating collapsible sections, we have an opportunity to strike a balance—bringing it closer to the original cheat sheet format while still preserving the power of the OWASP cheat sheet: reliable knowledge. This approach ensures that users can quickly access crucial information without being overwhelmed, and it remains a reliable resource—something often difficult to find when searching through platforms like Stack Overflow, Reddit, or various blog posts.

[jmanico]

I am happy to move in that direction. Works for me. Go for it. :)