Skip to content

Commit f6f2f97

Browse files
fabpotfabpot
committed
security #cve-2018-11385 migrating session for UsernamePasswordJsonAuthenticationListener
* cve-2018-11385-3.3: migrating session for UsernamePasswordJsonAuthenticationListener Adding session authentication strategy to Guard to avoid session fixation Adding session strategy to ALL listeners to avoid *any* possible fixation
2 parents 1e1cf5c + 51fa731 commit f6f2f97

8 files changed

+96
-3
lines changed

‎src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public function __construct(TokenStorageInterface $tokenStorage, EventDispatcher
4747
*/
4848
public function authenticateWithToken(TokenInterface $token, Request $request)
4949
{
50+
$this->migrateSession($request);
5051
$this->tokenStorage->setToken($token);
5152

5253
if (null !== $this->dispatcher) {
@@ -133,4 +134,16 @@ public function handleAuthenticationFailure(AuthenticationException $authenticat
133134
is_object($response) ? get_class($response) : gettype($response)
134135
));
135136
}
137+
138+
private function migrateSession(Request $request)
139+
{
140+
if (!$request->hasSession() || !$request->hasPreviousSession()) {
141+
return;
142+
}
143+
144+
// Destroying the old session is broken in php 5.4.0 - 5.4.10
145+
// See https://bugs.php.net/63379
146+
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
147+
$request->getSession()->migrate($destroy);
148+
}
136149
}

‎src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ final public function handle(GetResponseEvent $event)
8282
if (null !== $this->logger) {
8383
$this->logger->info('Pre-authentication successful.', array('token' => (string) $token));
8484
}
85+
86+
$this->migrateSession($request);
87+
8588
$this->tokenStorage->setToken($token);
8689

8790
if (null !== $this->dispatcher) {
@@ -114,4 +117,16 @@ private function clearToken(AuthenticationException $exception)
114117
* @return array An array composed of the user and the credentials
115118
*/
116119
abstract protected function getPreAuthenticatedData(Request $request);
120+
121+
private function migrateSession(Request $request)
122+
{
123+
if (!$request->hasSession() || !$request->hasPreviousSession()) {
124+
return;
125+
}
126+
127+
// Destroying the old session is broken in php 5.4.0 - 5.4.10
128+
// See https://bugs.php.net/63379
129+
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
130+
$request->getSession()->migrate($destroy);
131+
}
117132
}

‎src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Security\Http\Firewall;
1313

14+
use Symfony\Component\HttpFoundation\Request;
1415
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1516
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
1617
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
@@ -70,6 +71,9 @@ public function handle(GetResponseEvent $event)
7071

7172
try {
7273
$token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey));
74+
75+
$this->migrateSession($request);
76+
7377
$this->tokenStorage->setToken($token);
7478
} catch (AuthenticationException $e) {
7579
$token = $this->tokenStorage->getToken();
@@ -88,4 +92,16 @@ public function handle(GetResponseEvent $event)
8892
$event->setResponse($this->authenticationEntryPoint->start($request, $e));
8993
}
9094
}
95+
96+
private function migrateSession(Request $request)
97+
{
98+
if (!$request->hasSession() || !$request->hasPreviousSession()) {
99+
return;
100+
}
101+
102+
// Destroying the old session is broken in php 5.4.0 - 5.4.10
103+
// See https://bugs.php.net/63379
104+
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
105+
$request->getSession()->migrate($destroy);
106+
}
91107
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ public function handle(GetResponseEvent $event)
117117
$this->logger->info('Digest authentication successful.', array('username' => $digestAuth->getUsername(), 'received' => $digestAuth->getResponse()));
118118
}
119119

120+
$this->migrateSession($request);
121+
120122
$this->tokenStorage->setToken(new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey));
121123
}
122124

@@ -133,6 +135,18 @@ private function fail(GetResponseEvent $event, Request $request, AuthenticationE
133135

134136
$event->setResponse($this->authenticationEntryPoint->start($request, $authException));
135137
}
138+
139+
private function migrateSession(Request $request)
140+
{
141+
if (!$request->hasSession() || !$request->hasPreviousSession()) {
142+
return;
143+
}
144+
145+
// Destroying the old session is broken in php 5.4.0 - 5.4.10
146+
// See https://bugs.php.net/63379
147+
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
148+
$request->getSession()->migrate($destroy);
149+
}
136150
}
137151

138152
class DigestData

‎src/Symfony/Component/Security/Http/Firewall/SimplePreAuthenticationListener.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Security\Http\Firewall;
1313

14+
use Symfony\Component\HttpFoundation\Request;
1415
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1516
use Psr\Log\LoggerInterface;
1617
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
@@ -85,6 +86,9 @@ public function handle(GetResponseEvent $event)
8586
}
8687

8788
$token = $this->authenticationManager->authenticate($token);
89+
90+
$this->migrateSession($request);
91+
8892
$this->tokenStorage->setToken($token);
8993

9094
if (null !== $this->dispatcher) {
@@ -119,4 +123,16 @@ public function handle(GetResponseEvent $event)
119123
}
120124
}
121125
}
126+
127+
private function migrateSession(Request $request)
128+
{
129+
if (!$request->hasSession() || !$request->hasPreviousSession()) {
130+
return;
131+
}
132+
133+
// Destroying the old session is broken in php 5.4.0 - 5.4.10
134+
// See https://bugs.php.net/63379
135+
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
136+
$request->getSession()->migrate($destroy);
137+
}
122138
}

‎src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ private function onSuccess(Request $request, TokenInterface $token)
141141
$this->logger->info('User has been authenticated successfully.', array('username' => $token->getUsername()));
142142
}
143143

144+
$this->migrateSession($request);
145+
144146
$this->tokenStorage->setToken($token);
145147

146148
if (null !== $this->eventDispatcher) {
@@ -184,4 +186,15 @@ private function onFailure(Request $request, AuthenticationException $failed)
184186

185187
return $response;
186188
}
189+
190+
private function migrateSession(Request $request)
191+
{
192+
if (!$request->hasSession() || !$request->hasPreviousSession()) {
193+
return;
194+
}
195+
// Destroying the old session is broken in php 5.4.0 - 5.4.10
196+
// See https://bugs.php.net/63379
197+
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
198+
$request->getSession()->migrate($destroy);
199+
}
187200
}

‎src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategy.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,13 @@ public function onAuthentication(Request $request, TokenInterface $token)
4747
return;
4848

4949
case self::MIGRATE:
50-
$request->getSession()->migrate(true);
50+
// Note: this logic is duplicated in several authentication listeners
51+
// until Symfony 5.0 due to a security fix with BC compat
52+
53+
// Destroying the old session is broken in php 5.4.0 - 5.4.10
54+
// See https://bugs.php.net/63379
55+
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
56+
$request->getSession()->migrate($destroy);
5157

5258
return;
5359

‎src/Symfony/Component/Security/Http/Session/SessionAuthenticationStrategyInterface.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ interface SessionAuthenticationStrategyInterface
2727
/**
2828
* This performs any necessary changes to the session.
2929
*
30-
* This method is called before the TokenStorage is populated with a
31-
* Token, and only by classes inheriting from AbstractAuthenticationListener.
30+
* This method should be called before the TokenStorage is populated with a
31+
* Token. It should be used by authentication listeners when a session is used.
3232
*/
3333
public function onAuthentication(Request $request, TokenInterface $token);
3434
}

0 commit comments

Comments
 (0)