Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fabpot committed May 23, 2018
2 parents 47e7268 + a5855e8 commit fa5bf4b
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 3 deletions.
Expand Up @@ -82,6 +82,9 @@ final public function handle(GetResponseEvent $event)
if (null !== $this->logger) {
$this->logger->info('Pre-authentication successful.', array('token' => (string) $token));
}

$this->migrateSession($request);

$this->tokenStorage->setToken($token);

if (null !== $this->dispatcher) {
Expand Down Expand Up @@ -114,4 +117,16 @@ private function clearToken(AuthenticationException $exception)
* @return array An array composed of the user and the credentials
*/
abstract protected function getPreAuthenticatedData(Request $request);

private function migrateSession(Request $request)
{
if (!$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);
}
}
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Security\Http\Firewall;

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

try {
$token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey));

$this->migrateSession($request);

$this->tokenStorage->setToken($token);
} catch (AuthenticationException $e) {
$token = $this->tokenStorage->getToken();
Expand All @@ -88,4 +92,16 @@ public function handle(GetResponseEvent $event)
$event->setResponse($this->authenticationEntryPoint->start($request, $e));
}
}

private function migrateSession(Request $request)
{
if (!$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);
}
}
Expand Up @@ -118,6 +118,8 @@ public function handle(GetResponseEvent $event)
$this->logger->info('Digest authentication successful.', array('username' => $digestAuth->getUsername(), 'received' => $digestAuth->getResponse()));
}

$this->migrateSession($request);

$this->tokenStorage->setToken(new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey));
}

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

$event->setResponse($this->authenticationEntryPoint->start($request, $authException));
}

private function migrateSession(Request $request)
{
if (!$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);
}
}

class DigestData
Expand Down
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Security\Http\Firewall;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
Expand Down Expand Up @@ -85,6 +86,9 @@ public function handle(GetResponseEvent $event)
}

$token = $this->authenticationManager->authenticate($token);

$this->migrateSession($request);

$this->tokenStorage->setToken($token);

if (null !== $this->dispatcher) {
Expand Down Expand Up @@ -119,4 +123,16 @@ public function handle(GetResponseEvent $event)
}
}
}

private function migrateSession(Request $request)
{
if (!$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);
}
}
Expand Up @@ -47,8 +47,11 @@ public function onAuthentication(Request $request, TokenInterface $token)
return;

case self::MIGRATE:
// Note: this logic is duplicated in several authentication listeners
// until Symfony 5.0 due to a security fix with BC compat

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See php bug #63379
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);

Expand Down
Expand Up @@ -27,8 +27,8 @@ interface SessionAuthenticationStrategyInterface
/**
* This performs any necessary changes to the session.
*
* This method is called before the TokenStorage is populated with a
* Token, and only by classes inheriting from AbstractAuthenticationListener.
* This method should be called before the TokenStorage is populated with a
* Token. It should be used by authentication listeners when a session is used.
*/
public function onAuthentication(Request $request, TokenInterface $token);
}

0 comments on commit fa5bf4b

Please sign in to comment.