8000 security #cve-2018-11385 Adding session strategy to ALL listeners to … · symfony/symfony@fa5bf4b · GitHub
[go: up one dir, main page]
Skip to content

Commit fa5bf4b

Browse files
fabpotfabpot
committed
security #cve-2018-11385 Adding session strategy to ALL listeners to avoid *any* possible fixation
* cve-2018-11385-2.7: Adding session strategy to ALL listeners to avoid *any* possible fixation
2 parents 47e7268 + a5855e8 commit fa5bf4b

File tree

6 files changed

+67
-3
lines changed

6 files changed

+67
-3
lines changed

‎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
@@ -118,6 +118,8 @@ public function handle(GetResponseEvent $event)
118118
$this->logger->info('Digest authentication successful.', array('username' => $digestAuth->getUsername(), 'received' => $digestAuth->getResponse()));
119119
}
120120

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

@@ -134,6 +136,18 @@ private function fail(GetResponseEvent $event, Request $request, AuthenticationE
134136

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

139153
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/Session/SessionAuthenticationStrategy.php

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

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

‎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)
0