Skip to content

Commit

Permalink
security #cve-2018-11406 clear CSRF tokens when the user is logged out
Browse files Browse the repository at this point in the history
* cve-2018-11406-2.7:
  clear CSRF tokens when the user is logged out
  • Loading branch information
fabpot committed May 24, 2018
2 parents fa5bf4b + 4b91c17 commit 319e1bd
Show file tree
Hide file tree
Showing 15 changed files with 327 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;

/**
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
*/
class RegisterCsrfTokenClearingLogoutHandlerPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
if (!$container->has('security.logout_listener') || !$container->has('security.csrf.token_storage')) {
return;
}

$csrfTokenStorage = $container->findDefinition('security.csrf.token_storage');
$csrfTokenStorageClass = $container->getParameterBag()->resolveValue($csrfTokenStorage->getClass());

if (!is_subclass_of($csrfTokenStorageClass, 'Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface')) {
return;
}

$container->register('security.logout.handler.csrf_token_clearing', 'Symfony\Component\Security\Http\Logout\CsrfTokenClearingLogoutHandler')
->addArgument(new Reference('security.csrf.token_storage'))
->setPublic(false);

$container->findDefinition('security.logout_listener')->addMethodCall('addHandler', array(new Reference('security.logout.handler.csrf_token_clearing')));
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/SecurityBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Bundle\SecurityBundle;

use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfTokenClearingLogoutHandlerPass;
use Symfony\Component\HttpKernel\Bundle\Bundle;
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
use Symfony\Component\DependencyInjection\ContainerBuilder;
Expand Down Expand Up @@ -50,5 +51,6 @@ public function build(ContainerBuilder $container)
$extension->addUserProviderFactory(new InMemoryFactory());
$container->addCompilerPass(new AddSecurityVotersPass());
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING);
$container->addCompilerPass(new RegisterCsrfTokenClearingLogoutHandlerPass());
}
}
18 changes: 18 additions & 0 deletions src/Symfony/Bundle/SecurityBundle/Tests/Functional/LogoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,22 @@ public function testSessionLessRememberMeLogout()

$this->assertNull($cookieJar->get('REMEMBERME'));
}

public function testCsrfTokensAreClearedOnLogout()
{
$client = $this->createClient(array('test_case' => 'LogoutWithoutSessionInvalidation', 'root_config' => 'config.yml'));
$client->getContainer()->get('security.csrf.token_storage')->setToken('foo', 'bar');

$client->request('POST', '/login', array(
'_username' => 'johannes',
'_password' => 'test',
));

$this->assertTrue($client->getContainer()->get('security.csrf.token_storage')->hasToken('foo'));
$this->assertSame('bar', $client->getContainer()->get('security.csrf.token_storage')->getToken('foo'));

$client->request('GET', '/logout');

$this->assertFalse($client->getContainer()->get('security.csrf.token_storage')->hasToken('foo'));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

use Symfony\Bundle\SecurityBundle\SecurityBundle;
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;

return array(
new FrameworkBundle(),
new SecurityBundle(),
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
imports:
- { resource: ./../config/framework.yml }

security:
encoders:
Symfony\Component\Security\Core\User\User: plaintext

providers:
in_memory:
memory:
users:
johannes: { password: test, roles: [ROLE_USER] }

firewalls:
default:
form_login:
check_path: login
remember_me: true
require_previous_session: false
remember_me:
always_remember_me: true
key: key
logout:
invalidate_session: false
anonymous: ~
stateless: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
login:
path: /login

logout:
path: /logout
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/SecurityBundle/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"require": {
"php": ">=5.3.9",
"ext-xml": "*",
"symfony/security": "~2.7.47|~2.8.40",
"symfony/security": "~2.7.48|~2.8.41",
"symfony/security-acl": "~2.7",
"symfony/http-kernel": "~2.7"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,32 @@ public function testRemoveExistingToken()
$this->assertSame('TOKEN', $this->storage->removeToken('token_id'));
$this->assertFalse($this->storage->hasToken('token_id'));
}

public function testClearRemovesAllTokensFromTheConfiguredNamespace()
{
$this->storage->setToken('foo', 'bar');
$this->storage->clear();

$this->assertFalse($this->storage->hasToken('foo'));
$this->assertArrayNotHasKey(self::SESSION_NAMESPACE, $_SESSION);
}

public function testClearDoesNotRemoveSessionValuesFromOtherNamespaces()
{
$_SESSION['foo']['bar'] = 'baz';
$this->storage->clear();

$this->assertArrayHasKey('foo', $_SESSION);
$this->assertArrayHasKey('bar', $_SESSION['foo']);
$this->assertSame('baz', $_SESSION['foo']['bar']);
}

public function testClearDoesNotRemoveNonNamespacedSessionValues()
{
$_SESSION['foo'] = 'baz';
$this->storage->clear();

$this->assertArrayHasKey('foo', $_SESSION);
$this->assertSame('baz', $_SESSION['foo']);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,31 @@ public function testRemoveExistingTokenFromActiveSession()

$this->assertSame('TOKEN', $this->storage->removeToken('token_id'));
}

public function testClearRemovesAllTokensFromTheConfiguredNamespace()
{
$this->storage->setToken('foo', 'bar');
$this->storage->clear();

$this->assertFalse($this->storage->hasToken('foo'));
$this->assertFalse($this->session->has(self::SESSION_NAMESPACE.'/foo'));
}

public function testClearDoesNotRemoveSessionValuesFromOtherNamespaces()
{
$this->session->set('foo/bar', 'baz');
$this->storage->clear();

$this->assertTrue($this->session->has('foo/bar'));
$this->assertSame('baz', $this->session->get('foo/bar'));
}

public function testClearDoesNotRemoveNonNamespacedSessionValues()
{
$this->session->set('foo', 'baz');
$this->storage->clear();

$this->assertTrue($this->session->has('foo'));
$this->assertSame('baz', $this->session->get('foo'));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Csrf\TokenStorage;

/**
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
*/
interface ClearableTokenStorageInterface extends TokenStorageInterface
{
/**
* Removes all CSRF tokens.
*/
public function clear();
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class NativeSessionTokenStorage implements TokenStorageInterface
class NativeSessionTokenStorage implements ClearableTokenStorageInterface
{
/**
* The namespace used to store values in the session.
Expand Down Expand Up @@ -96,6 +96,14 @@ public function removeToken($tokenId)
return $token;
}

/**
* {@inheritdoc}
*/
public function clear()
{
unset($_SESSION[$this->namespace]);
}

private function startSession()
{
if (\PHP_VERSION_ID >= 50400) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class SessionTokenStorage implements TokenStorageInterface
class SessionTokenStorage implements ClearableTokenStorageInterface
{
/**
* The namespace used to store values in the session.
Expand Down Expand Up @@ -92,4 +92,16 @@ public function removeToken($tokenId)

return $this->session->remove($this->namespace.'/'.$tokenId);
}

/**
* {@inheritdoc}
*/
public function clear()
{
foreach (array_keys($this->session->all()) as $key) {
if (0 === strpos($key, $this->namespace.'/')) {
$this->session->remove($key);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Http\Logout;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface;

/**
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
*/
class CsrfTokenClearingLogoutHandler implements LogoutHandlerInterface
{
private $csrfTokenStorage;

public function __construct(ClearableTokenStorageInterface $csrfTokenStorage)
{
$this->csrfTokenStorage = $csrfTokenStorage;
}

public function logout(Request $request, Response $response, TokenInterface $token)
{
$this->csrfTokenStorage->clear();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Security\Http\Tests\Logout;

use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\Session\Session;
use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
use Symfony\Component\Security\Csrf\TokenStorage\SessionTokenStorage;
use Symfony\Component\Security\Http\Logout\CsrfTokenClearingLogoutHandler;

class CsrfTokenClearingLogoutHandlerTest extends TestCase
{
private $session;
private $csrfTokenStorage;
private $csrfTokenClearingLogoutHandler;

protected function setUp()
{
$this->session = new Session(new MockArraySessionStorage());
$this->csrfTokenStorage = new SessionTokenStorage($this->session, 'foo');
$this->csrfTokenStorage->setToken('foo', 'bar');
$this->csrfTokenStorage->setToken('foobar', 'baz');
$this->csrfTokenClearingLogoutHandler = new CsrfTokenClearingLogoutHandler($this->csrfTokenStorage);
}

public function testCsrfTokenCookieWithSameNamespaceIsRemoved()
{
$this->assertSame('bar', $this->session->get('foo/foo'));
$this->assertSame('baz', $this->session->get('foo/foobar'));

$this->csrfTokenClearingLogoutHandler->logout(new Request(), new Response(), $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock());

$this->assertFalse($this->csrfTokenStorage->hasToken('foo'));
$this->assertFalse($this->csrfTokenStorage->hasToken('foobar'));

$this->assertFalse($this->session->has('foo/foo'));
$this->assertFalse($this->session->has('foo/foobar'));
}

public function testCsrfTokenCookieWithDifferentNamespaceIsNotRemoved()
{
$barNamespaceCsrfSessionStorage = new SessionTokenStorage($this->session, 'bar');
$barNamespaceCsrfSessionStorage->setToken('foo', 'bar');
$barNamespaceCsrfSessionStorage->setToken('foobar', 'baz');

$this->assertSame('bar', $this->session->get('foo/foo'));
$this->assertSame('baz', $this->session->get('foo/foobar'));
$this->assertSame('bar', $this->session->get('bar/foo'));
$this->assertSame('baz', $this->session->get('bar/foobar'));

$this->csrfTokenClearingLogoutHandler->logout(new Request(), new Response(), $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock());

$this->assertTrue($barNamespaceCsrfSessionStorage->hasToken('foo'));
$this->assertTrue($barNamespaceCsrfSessionStorage->hasToken('foobar'));
$this->assertSame('bar', $barNamespaceCsrfSessionStorage->getToken('foo'));
$this->assertSame('baz', $barNamespaceCsrfSessionStorage->getToken('foobar'));
$this->assertFalse($this->csrfTokenStorage->hasToken('foo'));
$this->assertFalse($this->csrfTokenStorage->hasToken('foobar'));

$this->assertFalse($this->session->has('foo/foo'));
$this->assertFalse($this->session->has('foo/foobar'));
$this->assertSame('bar', $this->session->get('bar/foo'));
$this->assertSame('baz', $this->session->get('bar/foobar'));
}
}
Loading

0 comments on commit 319e1bd

Please sign in to comment.