summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Core/Util/StringUtils.php10
-rw-r--r--Tests/Core/Util/StringUtilsTest.php44
2 files changed, 50 insertions, 4 deletions
diff --git a/Core/Util/StringUtils.php b/Core/Util/StringUtils.php
index 5e13037..acf8e9e 100644
--- a/Core/Util/StringUtils.php
+++ b/Core/Util/StringUtils.php
@@ -27,6 +27,7 @@ class StringUtils
* 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 @@ class StringUtils
*/
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 @@ class StringUtils
$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/Tests/Core/Util/StringUtilsTest.php b/Tests/Core/Util/StringUtilsTest.php
index aac4139..b16cbac 100644
--- a/Tests/Core/Util/StringUtilsTest.php
+++ b/Tests/Core/Util/StringUtilsTest.php
@@ -13,11 +13,49 @@ namespace Symfony\Component\Security\Tests\Core\Util;
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));
}
}