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

CSRF vulnerability, again #55

Closed
berdario opened this issue Aug 3, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@berdario
Copy link

commented Aug 3, 2016

I'm afraid that the issue hasn't been fixed properly. I realized it today after deploying your new release.

You can try: login (or simply visit in a barebone new grails application) the main app website, but DON'T visit the console. Trigger the poc I sent you the last time, and you'll see that it'll still work. After accessing the console the protection will be effective and the poc will stop working.

I suspect that the reason might be the request.getHeader('X-CSRFToken') != session['CONSOLE_CSRF_TOKEN']) check.

If the console hasn't been visited yet, there will be no CONSOLE_CSRF_TOKEN in the session, and thus both side of the equations will be null, letting the check pass.

@berdario

This comment has been minimized.

Copy link
Author

commented Aug 3, 2016

Is there a reason why you haven't used the builtin protection offered by Grails, btw? That's not affected by this bug

@berdario berdario referenced this issue Aug 3, 2016

Closed

CSRF prevention #54

@sheehan

This comment has been minimized.

Copy link
Owner

commented Aug 4, 2016

My bad. Will be fixed in the next version. withForm is for a single form submission from a gsp, the console plugin is doing many async calls from JavaScript.

@berdario

This comment has been minimized.

Copy link
Author

commented Aug 4, 2016

Ok, thank you

Indeed, withForm is not a great name, but it only means that it expects the csrf tokens to be provided in the POST body (instead of via an header), as such you can use it just as well for xhr from Javascript

@berdario

This comment has been minimized.

Copy link
Author

commented Aug 4, 2016

I added a small comment
1edba78#commitcomment-18519584

I see that you added a fix, but you haven't closed yet this issue... I gather that there's more that you want to fix?

@sheehan sheehan closed this in 1edba78 Aug 6, 2016

@sheehan

This comment has been minimized.

Copy link
Owner

commented Aug 6, 2016

I was just waiting until i had time to release. Thanks for reporting the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.