zendframework / zend-validator

Validator component from Zend Framework
BSD 3-Clause "New" or "Revised" License
181 stars 136 forks source link

Default timeout on CSRF validator #191

Closed roelvanduijnhoven closed 7 years ago

roelvanduijnhoven commented 7 years ago

Some users of our service reported errors with the sign up flow. Turns out the default behaviour of the CSRF validator is configured such that it fails the validation after only 5 minutes. That is a very short time-frame for a complex form; or if somebody needs to look up some additional information on another website.

However the true question is: why is a timeout necessary for this component? In my opinion it suffices to drop it; and rely solely on the hash generation.

I'd thus like to propose changing the default value of $timeout to null.

weierophinney commented 7 years ago

Timeouts for CSRF tokens are considered a best practice; see this answer on StackExchange and this AppSec paragraph on CSRF encrypted tokens (which indicates the timestamp should be part of the token).

Leaving out a timeout allows for session fixation attacks that then circumvent the CSRF mitigation.

roelvanduijnhoven commented 7 years ago

Sadly I cannot find anything on a recommended timeout in the given resources. I looked at how Rails configured the timeout. But found that they store the key in the session storage; which by default is based on cookies that do not expire.

Still I find 5 minutes to be very short. So at the very least I would recommend we bump it to a larger value. Say 15 minutes. But I know this is kind of arbitrary :).

alextartan commented 7 years ago

Why do that when you can configure the timeout?

new Csrf(['timeout'=>1000]);

roelvanduijnhoven commented 7 years ago

I think many users of zend-form will not know that their forms will invalidate themselves (let alone after 5 minutes). When a visitor of your site encounters this, it can be quite frustrating. Especially if the form has some file upload fields; as they are lost.

So I would argue that a higher timeout would be beneficial. As the timeout of 5 minutes now is quite arbitrary; I don't see issues raising it somewhat.

But don't want to further pollute the issue tracker. I'll close it for now.