| • csteipp | |
| Feb 13 2014, 11:01 PM |
| F12865: bug61346b.patch | |
| Nov 22 2014, 2:52 AM |
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)
Version: unspecified
Severity: normal
Created attachment 14585
Check xor of tokens
attachment bug61346.patch ignored as obsolete
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.
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 easyif ( 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.