Problem/Motivation
PHP has the hash_equals() function which should be used when comparing strings in security validation contexts. Currently CsrfTokenGenerator uses ===, which is bad.
The only problem is that hash_equals() does not exist in PHP 5.5, but we can use Crypt::hashEquals().
Proposed resolution
Use Crypt::hashEquals().
Comment | File | Size | Author |
---|---|---|---|
#5 | csrf-timing-2822499-5.patch | 1.11 KB | klausi |
Comments
Comment #2
klausiAh, we have Crypt::hashEquals().
Thinking more about this: CSRF attacks are performed blind, so an attacker can never look at the response and do timing analysis. I still think it is a good idea to do this because our csrf_token service might be used in other ways we don't know where an attacker has access to the response.
Comment #3
klausiPatch.
Comment #5
klausiUgh, we should never pass NULL to a string validate function.
Comment #6
gregglesMakes sense to me. Thanks, Klausi!
Comment #8
catchCommitted/pushed to 8.3.x, thanks!
This looks fine to me for 8.2.x as well - any reason not to cherry-pick?
Comment #9
gregglesIMO it seems good to backport/cherry pick, yes.
Comment #10
klausiRTBC+1, the queued test run for 8.2.x cam back green.
Comment #11
alexpottI've cherry-picked to 8.2.x as a bug and a security improvement with no BC implications I think this is fine.