Last modified: 2014-03-03 19:20:40 UTC
Aaron pointed out that when checking the Token cookie, we use a string comparison, which could allow someone to brute-force the correct token faster than brute-forcing the entire key space. $passwordCorrect = ( strlen( $token ) && $token === $request->getCookie( 'Token' ) ); This should use a constant time comparison (xor and check if the result > 1)
Created attachment 14585 [details] Check xor of tokens
Looks fine, but that comparison should go in it's own protected method.
Created attachment 14631 [details] Check xor of tokens
Looks mergable
Deployed today, and we'll add it to the next release in a couple of weeks.
For reference, see I2a9e89120f7092015495e638c6fa9f67adc9b84f (Looks like gerrit bot can't edit security bugs)
I guess there are more routines and many more _Extensions_ which should start using the constant-time compareSecrets function.
Chris, next time pls. inform me directly.
Not sure, if the following lines in your patch are correct as they make the function return quickly if the lenghts are unequal -> timing attack made easy if ( strlen( $answer ) !== strlen( $test ) ) { + $passwordCorrect = false; + } else {
(In reply to T. Gries from comment #9) > Not sure, if the following lines in your patch are correct as they make the > function return quickly if the lenghts are unequal -> timing attack made easy > > if ( strlen( $answer ) !== strlen( $test ) ) { > + $passwordCorrect = false; > + } else { We could move this check out of the function, so the function is always constant time. But in this context, MediaWiki user tokens have been 32 characters since 2004, so knowing that the token is 32 characters doesn't give an attacker any extra information.
This was assigned CVE-2014-2243. http://www.openwall.com/lists/oss-security/2014/03/01/2 I'm closing this bug since the token comparison in MediaWiki core is fixed. For this vulnerability in other extensions, let's open a separate bug for each, so we can track those separately.
> let's open a separate bug for each, so we can track those separately. Yes! Thx. Don't forget to mail me directly in case you detect further issues.
