fix potential timing attack issue

fabpot · Nov 23, 2015 · f1fd768 · f1fd768
1 parent 3dc2244
commit f1fd768
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 27 deletions.
diff --git a/src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php b/src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php
@@ -21,6 +21,7 @@
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Util\SecureRandomInterface;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Security\Core\Util\StringUtils;
 
 /**
  * Concrete implementation of the RememberMeServicesInterface which needs
@@ -90,7 +91,7 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
         list($series, $tokenValue) = $cookieParts;
         $persistentToken = $this->tokenProvider->loadTokenBySeries($series);
 
-        if ($persistentToken->getTokenValue() !== $tokenValue) {
+        if (!StringUtils::equals($persistentToken->getTokenValue(), $tokenValue)) {
             throw new CookieTheftException('This token was already used. The account is possibly compromised.');
         }
 

diff --git a/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php b/src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php
@@ -17,6 +17,7 @@
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\User\UserInterface;
+use Symfony\Component\Security\Core\Util\StringUtils;
 
 /**
  * Concrete implementation of the RememberMeServicesInterface providing
@@ -53,7 +54,7 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
             throw new \RuntimeException(sprintf('The UserProviderInterface implementation must return an instance of UserInterface, but returned "%s".', get_class($user)));
         }
 
-        if (true !== $this->compareHashes($hash, $this->generateCookieHash($class, $username, $expires, $user->getPassword()))) {
+        if (!StringUtils::equals($this->generateCookieHash($class, $username, $expires, $user->getPassword()), $hash)) {
             throw new AuthenticationException('The cookie\'s hash is invalid.');
         }
 
@@ -64,31 +65,6 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
         return $user;
     }
 
-    /**
-     * Compares two hashes using a constant-time algorithm to avoid (remote)
-     * timing attacks.
-     *
-     * This is the same implementation as used in the BasePasswordEncoder.
-     *
-     * @param string $hash1 The first hash
-     * @param string $hash2 The second hash
-     *
-     * @return bool true if the two hashes are the same, false otherwise
-     */
-    private function compareHashes($hash1, $hash2)
-    {
-        if (strlen($hash1) !== $c = strlen($hash2)) {
-            return false;
-        }
-
-        $result = 0;
-        for ($i = 0; $i < $c; ++$i) {
-            $result |= ord($hash1[$i]) ^ ord($hash2[$i]);
-        }
-
-        return 0 === $result;
-    }
-
     /**
      * {@inheritdoc}
      */