diff --git a/src/Symfony/Component/Security/Core/Util/StringUtils.php b/src/Symfony/Component/Security/Core/Util/StringUtils.php index 5e130375b725..acf8e9eed8af 100644 --- a/src/Symfony/Component/Security/Core/Util/StringUtils.php +++ b/src/Symfony/Component/Security/Core/Util/StringUtils.php @@ -27,6 +27,7 @@ private function __construct() {} * Compares two strings. * * This method implements a constant-time algorithm to compare strings. + * Regardless of the used implementation, it will leak length information. * * @param string $knownString The string of known length to compare against * @param string $userInput The string that the user can control @@ -35,6 +36,13 @@ private function __construct() {} */ public static function equals($knownString, $userInput) { + $knownString = (string) $knownString; + $userInput = (string) $userInput; + + if (function_exists('hash_equals')) { + return hash_equals($knownString, $userInput); + } + $knownLen = strlen($knownString); $userLen = strlen($userInput); @@ -45,7 +53,7 @@ public static function equals($knownString, $userInput) $result = $knownLen - $userLen; // Note that we ALWAYS iterate over the user-supplied length - // This is to prevent leaking length information + // This is to mitigate leaking length information for ($i = 0; $i < $userLen; $i++) { $result |= (ord($knownString[$i]) ^ ord($userInput[$i])); } diff --git a/src/Symfony/Component/Security/Tests/Core/Util/StringUtilsTest.php b/src/Symfony/Component/Security/Tests/Core/Util/StringUtilsTest.php index aac4139254a4..b16cbacc030f 100644 --- a/src/Symfony/Component/Security/Tests/Core/Util/StringUtilsTest.php +++ b/src/Symfony/Component/Security/Tests/Core/Util/StringUtilsTest.php @@ -13,11 +13,49 @@ use Symfony\Component\Security\Core\Util\StringUtils; +/** + * Data from PHP.net's hash_equals tests + */ class StringUtilsTest extends \PHPUnit_Framework_TestCase { - public function testEquals() + public function dataProviderTrue() + { + return array( + array('same', 'same'), + array('', ''), + array(123, 123), + array(null, ''), + array(null, null), + ); + } + + public function dataProviderFalse() + { + return array( + array('not1same', 'not2same'), + array('short', 'longer'), + array('longer', 'short'), + array('', 'notempty'), + array('notempty', ''), + array(123, 'NaN'), + array('NaN', 123), + array(null, 123), + ); + } + + /** + * @dataProvider dataProviderTrue + */ + public function testEqualsTrue($known, $user) + { + $this->assertTrue(StringUtils::equals($known, $user)); + } + + /** + * @dataProvider dataProviderFalse + */ + public function testEqualsFalse($known, $user) { - $this->assertTrue(StringUtils::equals('password', 'password')); - $this->assertFalse(StringUtils::equals('password', 'foo')); + $this->assertFalse(StringUtils::equals($known, $user)); } }