CSRF token is generated using math/random not crypto/random

[robvdl]

Issue Description

Can someone please explain why labstack/gommon/random is using math/random over crypto/random. This is what is used to generate the CSRF tokens.

All it does is reset the random seed in the New function, but I'm a bit concerned it's not using crypto/random.

That means we can get predictable CSRF tokens, not good.

Checklist

• Dependencies installed
• No typos
• Searched existing issues and docs

Expected behaviour

Actual behaviour

Steps to reproduce

Working code to debug

package main

func main() {
}

Version/commit

[aldas]

I really can not say why as that part is very old. I'll submit PR to change it (it is already done in v5 branch)

They way things work, I would not be that worried - for this to work attacker would need to guess target CRSF (before that the seed) and provide LINK with that token to some page that does harmful action on GET. Note here - links are opened as GET request. Actions are POST/PUT/DELETE etc. Now if attacker manages to find out the seed, the target token generated from that seed needs to be still figured out and to do that - attacker needs to provide target N amount of Links to try. If that do not raise suspicions from the target or is not detected - I guess - the game is already lost without that attack vector succeeding.

[robvdl]

I think the current v4 middleware code is also not tying the csrftoken to the session properly and in the middleware it quite happily just reads the previous token from a cookie the way I am reading the csrf middleware code. The cookie is also not encrypted with HMAC.

So it really depends on how well a site is being protected by other means like HTTPS and proper XSS protection, otherwise there could be a bunch of problems. Because any forged cookies and the backend happily accepts this as a new csrftoken, that doesn't seem right. You're only really going to be protected by having SameSite cookies, XSS protection, and making the cookies HttpOnly.

Reading OWASP on CSRF, more work needs to be done to tie the csrftoken to a session https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html so it can't be forged anymore.

Off course that requires session support first, for which you can use Gorilla/sessions or scs. I'm using scs with my own encryption added on top because I wasn't happy that scs wasn't encrypting session data.

For now, at least for my project, I could just take a copy of the csrf middleware and do something myself that ties it to the session properly.

[robvdl]

The original issue is fixed, which is why I'm closing this.

The fact that the csrftoken middleware isn't really upto scratch and blindly accepts the cookie from the client as truth, is not related, and I have been able to resolve it by copying the csrftoken middleware into my own codebase and changing it to use a session instead.