Skip to content

Commit c519772

Browse files
committed
security symfony#16630 n/a (xabbuh)
This PR was merged into the 2.3 branch. Discussion ---------- n/a n/a Commits ------- 819aa54 prevent timing attacks in digest auth listener 557ea17 mitigate CSRF timing attack vulnerability f1fd768 fix potential timing attack issue
2 parents 3dc2244 + 819aa54 commit c519772

File tree

4 files changed

+19
-29
lines changed

4 files changed

+19
-29
lines changed

src/Symfony/Component/Form/Extension/Csrf/CsrfProvider/DefaultCsrfProvider.php

+13-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Form\Extension\Csrf\CsrfProvider;
1313

14+
use Symfony\Component\Security\Core\Util\StringUtils;
15+
1416
/**
1517
* Default implementation of CsrfProviderInterface.
1618
*
@@ -54,7 +56,17 @@ public function generateCsrfToken($intention)
5456
*/
5557
public function isCsrfTokenValid($intention, $token)
5658
{
57-
return $token === $this->generateCsrfToken($intention);
59+
$expectedToken = $this->generateCsrfToken($intention);
60+
61+
if (function_exists('hash_equals')) {
62+
return hash_equals($expectedToken, $token);
63+
}
64+
65+
if (class_exists('Symfony\Component\Security\Core\Util\StringUtils')) {
66+
return StringUtils::equals($expectedToken, $token);
67+
}
68+
69+
return $token === $expectedToken;
5870
}
5971

6072
/**

src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Component\Security\Core\SecurityContextInterface;
1515
use Symfony\Component\Security\Core\User\UserProviderInterface;
16+
use Symfony\Component\Security\Core\Util\StringUtils;
1617
use Symfony\Component\Security\Http\EntryPoint\DigestAuthenticationEntryPoint;
1718
use Psr\Log\LoggerInterface;
1819
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
@@ -99,7 +100,7 @@ public function handle(GetResponseEvent $event)
99100
return;
100101
}
101102

102-
if ($serverDigestMd5 !== $digestAuth->getResponse()) {
103+
if (!StringUtils::equals($serverDigestMd5, $digestAuth->getResponse())) {
103104
if (null !== $this->logger) {
104105
$this->logger->debug(sprintf('Expected response: "%s" but received: "%s"; is AuthenticationDao returning clear text passwords?', $serverDigestMd5, $digestAuth->getResponse()));
105106
}

src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2222
use Symfony\Component\Security\Core\Util\SecureRandomInterface;
2323
use Psr\Log\LoggerInterface;
24+
use Symfony\Component\Security\Core\Util\StringUtils;
2425

2526
/**
2627
* Concrete implementation of the RememberMeServicesInterface which needs
@@ -90,7 +91,7 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
9091
list($series, $tokenValue) = $cookieParts;
9192
$persistentToken = $this->tokenProvider->loadTokenBySeries($series);
9293

93-
if ($persistentToken->getTokenValue() !== $tokenValue) {
94+
if (!StringUtils::equals($persistentToken->getTokenValue(), $tokenValue)) {
9495
throw new CookieTheftException('This token was already used. The account is possibly compromised.');
9596
}
9697

src/Symfony/Component/Security/Http/RememberMe/TokenBasedRememberMeServices.php

+2-26
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1818
use Symfony\Component\Security\Core\Exception\AuthenticationException;
1919
use Symfony\Component\Security\Core\User\UserInterface;
20+
use Symfony\Component\Security\Core\Util\StringUtils;
2021

2122
/**
2223
* Concrete implementation of the RememberMeServicesInterface providing
@@ -53,7 +54,7 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
5354
throw new \RuntimeException(sprintf('The UserProviderInterface implementation must return an instance of UserInterface, but returned "%s".', get_class($user)));
5455
}
5556

56-
if (true !== $this->compareHashes($hash, $this->generateCookieHash($class, $username, $expires, $user->getPassword()))) {
57+
if (!StringUtils::equals($this->generateCookieHash($class, $username, $expires, $user->getPassword()), $hash)) {
5758
throw new AuthenticationException('The cookie\'s hash is invalid.');
5859
}
5960

@@ -64,31 +65,6 @@ protected function processAutoLoginCookie(array $cookieParts, Request $request)
6465
return $user;
6566
}
6667

67-
/**
68-
* Compares two hashes using a constant-time algorithm to avoid (remote)
69-
* timing attacks.
70-
*
71-
* This is the same implementation as used in the BasePasswordEncoder.
72-
*
73-
* @param string $hash1 The first hash
74-
* @param string $hash2 The second hash
75-
*
76-
* @return bool true if the two hashes are the same, false otherwise
77-
*/
78-
private function compareHashes($hash1, $hash2)
79-
{
80-
if (strlen($hash1) !== $c = strlen($hash2)) {
81-
return false;
82-
}
83-
84-
$result = 0;
85-
for ($i = 0; $i < $c; ++$i) {
86-
$result |= ord($hash1[$i]) ^ ord($hash2[$i]);
87-
}
88-
89-
return 0 === $result;
90-
}
91-
9268
/**
9369
* {@inheritdoc}
9470
*/

0 commit comments

Comments
 (0)