csrf logic is flawed

[mmerickel]

Apologies for the structure of this issue. I am not a go developer, nor a user of echo and thus do not have any working code examples.

I was linked to the CSRF implementation in echo by someone with some questions about how it works. As I've looked through its logic, it appears to be fundamentally broken and failing to serve its purpose of protecting against malicious third-party websites. It appears as if any token signed using the global secret is considered a valid CSRF token. This means that an attacker only needs to acquire a single token from the server, and then they can forge a malicious form which can be used against visitors.

Attack:

• Attacker requests any page on the site.
• Attacker grabs the token from the cookie.
• Attacker builds new website with a form that posts back to your website with a copy of your delete account form using the hijacked token.
• Visitor clicks the button on the attackers website, triggering a post with a valid csrf token and their auth credentials.
• Visitor's account is now deleted.

This type of automated attack should be impossible if CSRF were implemented properly. The token must be tied to the user or the user's logged in session in some way to make it unguessable. Currently the token is only tied to a global secret, making any token valid for any user and only protecting against the most naive attack in which an attacker creates a form with no csrf token at all. The tokens also never expire which is poor design as it's standard practice to change security tokens when a user crosses a privilege boundary (such as logging in or out).

[jayd3e]

After further research on this topic, I've found that the Rails and Django versions of CSRF protection implement a whitelist at the session level(something that Echo is not doing), so they actually verify that the token that is presented in the request is a token that was generated specifically for the user.

[vishr]

https://github.com/pillarjs/understanding-csrf

[mmerickel]

Thanks for the link but it basically agrees with my original post.

The token just needs to be "unguessable", making it difficult for an attacker to successfully guess within a couple of tries. It does not have to be cryptographically secure. An attack is one or two clicks by an unbeknownst user, not a brute force attack by a server.

As I said above, the token is guessable because it is valid for any user as long as the global secret does not change. This means the attacker does not need to do a GET request per-user, which is a stated goal of the token.

[mmerickel]

FWIW a common trick is to simply store the token in a cookie. Then you don't need to track any concept of a user in the middleware. This gets you pretty close to a solid solution as you cannot re-use a token from one session against someone else's session because they won't have the same cookie (trusted token). Ideally there would also be an api where people could invalidate the tokens (change them) at privilege boundaries to avoid any hijacking / vulnerabilities present in HTTPP-only pages, but that's certainly less of a concern than making the tokens unique per-session.

[vishr]

Agreed. Need to move secret to user level using httponly cookie. I will investigate on invalidating tokens.

[mmerickel]

I think you're even storing the token in a cookie already. The main issue appears to be that the value of the cookie isn't actually being used to validate the header/form. I think you should modify the validation routines to ensure the value is signed by the secret (this is already being done), and then compare it to the value in the header/form.

[vishr]

Yea I realized that . Thanks for your input.

[vp894138]

@vishr In the latest codebase the HMAC based signing of the CSRF token is now removed.
This effectively means that this bug has been reopened

We now generate a CSRF token using random function.