librelist archives

« back to archive

Code review requested for Flask-SeaSurf

Code review requested for Flask-SeaSurf

From:
Max Countryman
Date:
2011-12-28 @ 20:12
Hi all,

A couple of weeks ago I announced a new Flask CSRF protection extension 
that sought to update the existing implementation to be more secure and 
modern. It's based on the Django middleware.

I would greatly appreciate any feedback the list might have. In addition 
I'd like to request it be added to the extensions page as I believe it 
should meet all the basic requirements for listing. Furthermore, it should
in fact meet requirements for "approved" status.

The extension can be found here: https://github.com/maxcountryman/flask-seasurf

Thanks in advance,


Max

Re: [flask] Code review requested for Flask-SeaSurf

From:
Ron DuPlain
Date:
2011-12-30 @ 03:53
On Wed, Dec 28, 2011 at 3:12 PM, Max Countryman <maxc@me.com> wrote:
> I'd like to request it be added to the extensions page

Listed.
http://flask.pocoo.org/extensions/#Flask-SeaSurf

We could use more extension reviewers.  Once you have a good
understanding of Flask extensions, it's a matter of code review and
checking the guidelines here:
http://flask.pocoo.org/docs/extensiondev/#approved-extensions

If you'd like to review, simply add your results and reactions to the
issue tracker:
https://github.com/mitsuhiko/flask/issues?labels=extension+review

Thanks,

Ron

Re: [flask] Code review requested for Flask-SeaSurf

From:
Max Countryman
Date:
2011-12-30 @ 13:18
Ron,

I've added two requests for approval on the issue tracker but they haven't
been properly tagged yet:

https://github.com/mitsuhiko/flask/issues/360

https://github.com/mitsuhiko/flask/issues/363

Thanks,


Max

Re: [flask] Code review requested for Flask-SeaSurf

From:
Jesse Dubay
Date:
2011-12-28 @ 22:55
Hey Max,

In my own apps, I use the CSRF validation built into Flask-WTF and
have found it lacking because it replaces the token for every form
rendered. If a user browses my site in multiple tabs, a new token
being generated for a form in tab A will overwrite the token for a
form in tab B, resulting in a really unfriendly token validation error
when he or she tries to submit the tab B form. This also occurs if the
user sees a form, browses elsewhere, then hits the back button and
tries to fill out that form.

From the code it looks like Flask-SeaSurf has the same issue: requests
write to the same value in the session cookie and interfere with each
other. Any thoughts on that? Would it be reasonable to instead (re)use
a single token for the lifetime of the user's session?

I asked about this sort of problem in #pocoo a while back (2011-09-11
01:05:53) but didn't really get a good response.

Jesse

On Wed, Dec 28, 2011 at 12:12 PM, Max Countryman <maxc@me.com> wrote:
> Hi all,
>
> A couple of weeks ago I announced a new Flask CSRF protection extension 
that sought to update the existing implementation to be more secure and 
modern. It's based on the Django middleware.
>
> I would greatly appreciate any feedback the list might have. In addition
I'd like to request it be added to the extensions page as I believe it 
should meet all the basic requirements for listing. Furthermore, it should
in fact meet requirements for "approved" status.
>
> The extension can be found here: https://github.com/maxcountryman/flask-seasurf
>
> Thanks in advance,
>
>
> Max

Re: [flask] Code review requested for Flask-SeaSurf

From:
Max Countryman
Date:
2011-12-29 @ 02:41
On Dec 28, 2011, at 5:55 PM, Jesse Dubay wrote:

> In my own apps, I use the CSRF validation built into Flask-WTF and
> have found it lacking because it replaces the token for every form
> rendered. If a user browses my site in multiple tabs, a new token
> being generated for a form in tab A will overwrite the token for a
> form in tab B, resulting in a really unfriendly token validation error
> when he or she tries to submit the tab B form.

Flask-SeaSurf doesn't suffer from this issue as a cookie is used to store 
the token until it's consumed, e.g. by POSTing in the context of a form. 
Alternatively the cookie is set to expire after a set amount of time at 
which point the token is no longer valid, a la the Django middleware.

So if I have two tabs open, I navigate in tab A to a a CSRF-protected view
/login and then open tab B and navigate again to /login and then switch 
back to A the CSRF token is still valid and will submit for either page.

However tab B won't be able to POST as the token will be consumed once tab
A is submitted. This is however desired behavior as I understand it and 
and the same way the Django middleware works; i.e. a token should be 
consumed once and only once otherwise it's possible to expose the 
application to potential attack.

Hope this helps,


Max

Re: [flask] Code review requested for Flask-SeaSurf

From:
Cheng-Han Lee
Date:
2011-12-29 @ 00:41
Hi Jesse,

I looked into this issue as well. From what I've read, single, session
wide CSRF tokens does provide a certain level of security. But it is
not as secure as generating a unique, CSRF token for every page or
every post request.

From OWASP:
 "Use of the unique token per page property is currently experimental
but provides a significant amount of improved security. Consider the
exposure of a CSRF token using the legacy unique per-session model.
Exposure of this token facilitates the attacker's ability to carry out
a CSRF attack against the victim's active session for any resource
exposed by the web application. Now consider the exposure of a CSRF
token using the experimental unique token per-page model. Exposure of
this token would only allow the attacker to carry out a CSRF attack
against the victim's active session for a small subset of resources
exposed by the web application. Use of the unique token per-page
property is a strong defense in depth strategy significantly reducing
the impact of exposed CSRF prevention tokens. "
Here are some sources that I've gleamed over:

http://stackoverflow.com/questions/3664044/anti-csrf-token-and-javascript
https://www.owasp.org/index.php/CSRFGuard_3_Configuration#Unique_Token_Per_Page

https://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet

On Wed, Dec 28, 2011 at 2:55 PM, Jesse Dubay <jesse@thefortytwo.net> wrote:
> Hey Max,
>
> In my own apps, I use the CSRF validation built into Flask-WTF and
> have found it lacking because it replaces the token for every form
> rendered. If a user browses my site in multiple tabs, a new token
> being generated for a form in tab A will overwrite the token for a
> form in tab B, resulting in a really unfriendly token validation error
> when he or she tries to submit the tab B form. This also occurs if the
> user sees a form, browses elsewhere, then hits the back button and
> tries to fill out that form.
>
> >From the code it looks like Flask-SeaSurf has the same issue: requests
> write to the same value in the session cookie and interfere with each
> other. Any thoughts on that? Would it be reasonable to instead (re)use
> a single token for the lifetime of the user's session?
>
> I asked about this sort of problem in #pocoo a while back (2011-09-11
> 01:05:53) but didn't really get a good response.
>
> Jesse
>
> On Wed, Dec 28, 2011 at 12:12 PM, Max Countryman <maxc@me.com> wrote:
>> Hi all,
>>
>> A couple of weeks ago I announced a new Flask CSRF protection extension
that sought to update the existing implementation to be more secure and 
modern. It's based on the Django middleware.
>>
>> I would greatly appreciate any feedback the list might have. In 
addition I'd like to request it be added to the extensions page as I 
believe it should meet all the basic requirements for listing. 
Furthermore, it should in fact meet requirements for "approved" status.
>>
>> The extension can be found here: https://github.com/maxcountryman/flask-seasurf
>>
>> Thanks in advance,
>>
>>
>> Max