Skip to content

Commit 9d3989f

Browse files
fabpotfabpot
committed
security #cve-2018-11406 clear CSRF tokens when the user is logged out
* cve-2018-11406-3.3: clear CSRF tokens when the user is logged out
2 parents f6f2f97 + 86a039a commit 9d3989f

File tree

14 files changed

+309
-4
lines changed

14 files changed

+309
-4
lines changed

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterCsrfTokenClearingLogoutHandlerPass.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
15+
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\DependencyInjection\Reference;
17+
18+
/**
19+
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
20+
*/
21+
class RegisterCsrfTokenClearingLogoutHandlerPass implements CompilerPassInterface
22+
{
23+
public function process(ContainerBuilder $container)
24+
{
25+
if (!$container->has('security.logout_listener') || !$container->has('security.csrf.token_storage')) {
26+
return;
27+
}
28+
29+
$csrfTokenStorage = $container->findDefinition('security.csrf.token_storage');
30+
$csrfTokenStorageClass = $container->getParameterBag()->resolveValue($csrfTokenStorage->getClass());
31+
32+
if (!is_subclass_of($csrfTokenStorageClass, 'Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface')) {
33+
return;
34+
}
35+
36+
$container->register('security.logout.handler.csrf_token_clearing', 'Symfony\Component\Security\Http\Logout\CsrfTokenClearingLogoutHandler')
37+
->addArgument(new Reference('security.csrf.token_storage'))
38+
->setPublic(false);
39+
40+
$container->findDefinition('security.logout_listener')->addMethodCall('addHandler', array(new Reference('security.logout.handler.csrf_token_clearing')));
41+
}
42+
}

‎src/Symfony/Bundle/SecurityBundle/SecurityBundle.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bundle\SecurityBundle;
1313

1414
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\JsonLoginFactory;
15+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfTokenClearingLogoutHandlerPass;
1516
use Symfony\Component\HttpKernel\Bundle\Bundle;
1617
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
1718
use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -60,5 +61,6 @@ public function build(ContainerBuilder $container)
6061
$extension->addUserProviderFactory(new LdapFactory());
6162
$container->addCompilerPass(new AddSecurityVotersPass());
6263
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING);
64+
$container->addCompilerPass(new RegisterCsrfTokenClearingLogoutHandlerPass());
6365
}
6466
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/bundles.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
use Symfony\Bundle\SecurityBundle\SecurityBundle;
13+
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
14+
15+
return array(
16+
new FrameworkBundle(),
17+
new SecurityBundle(),
18+
);

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/config.yml

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
imports:
2+
- { resource: ./../config/framework.yml }
3+
4+
security:
5+
encoders:
6+
Symfony\Component\Security\Core\User\User: plaintext
7+
8+
providers:
9+
in_memory:
10+
memory:
11+
users:
12+
johannes: { password: test, roles: [ROLE_USER] }
13+
14+
firewalls:
15+
default:
16+
form_login:
17+
check_path: login
18+
remember_me: true
19+
require_previous_session: false
20+
remember_me:
21+
always_remember_me: true
22+
key: key
23+
logout:
24+
invalidate_session: false
25+
anonymous: ~
26+
stateless: true

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LogoutWithoutSessionInvalidation/routing.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
login:
2+
path: /login
3+
4+
logout:
5+
path: /logout

‎src/Symfony/Bundle/SecurityBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"require": {
1919
"php": "^5.5.9|>=7.0.8",
2020
"ext-xml": "*",
21-
"symfony/security": "~3.3.13|~3.4-beta5",
21+
"symfony/security": "~3.3.17|~3.4.11",
2222
"symfony/dependency-injection": "~3.3",
2323
"symfony/http-kernel": "~3.3",
2424
"symfony/polyfill-php70": "~1.0"

‎src/Symfony/Component/Security/Csrf/Tests/TokenStorage/NativeSessionTokenStorageTest.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,32 @@ public function testRemoveExistingToken()
113113
$this->assertSame('TOKEN', $this->storage->removeToken('token_id'));
114114
$this->assertFalse($this->storage->hasToken('token_id'));
115115
}
116+
117+
public function testClearRemovesAllTokensFromTheConfiguredNamespace()
118+
{
119+
$this->storage->setToken('foo', 'bar');
120+
$this->storage->clear();
121+
122+
$this->assertFalse($this->storage->hasToken('foo'));
123+
$this->assertArrayNotHasKey(self::SESSION_NAMESPACE, $_SESSION);
124+
}
125+
126+
public function testClearDoesNotRemoveSessionValuesFromOtherNamespaces()
127+
{
128+
$_SESSION['foo']['bar'] = 'baz';
129+
$this->storage->clear();
130+
131+
$this->assertArrayHasKey('foo', $_SESSION);
132+
$this->assertArrayHasKey('bar', $_SESSION['foo']);
133+
$this->assertSame('baz', $_SESSION['foo']['bar']);
134+
}
135+
136+
public function testClearDoesNotRemoveNonNamespacedSessionValues()
137+
{
138+
$_SESSION['foo'] = 'baz';
139+
$this->storage->clear();
140+
141+
$this->assertArrayHasKey('foo', $_SESSION);
142+
$this->assertSame('baz', $_SESSION['foo']);
143+
}
116144
}

‎src/Symfony/Component/Security/Csrf/Tests/TokenStorage/SessionTokenStorageTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,4 +256,31 @@ public function testRemoveExistingTokenFromActiveSession()
256256

257257
$this->assertSame('TOKEN', $this->storage->removeToken('token_id'));
258258
}
259+
260+
public function testClearRemovesAllTokensFromTheConfiguredNamespace()
261+
{
262+
$this->storage->setToken('foo', 'bar');
263+
$this->storage->clear();
264+
265+
$this->assertFalse($this->storage->hasToken('foo'));
266+
$this->assertFalse($this->session->has(self::SESSION_NAMESPACE.'/foo'));
267+
}
268+
269+
public function testClearDoesNotRemoveSessionValuesFromOtherNamespaces()
270+
{
271+
$this->session->set('foo/bar', 'baz');
272+
$this->storage->clear();
273+
274+
$this->assertTrue($this->session->has('foo/bar'));
275+
$this->assertSame('baz', $this->session->get('foo/bar'));
276+
}
277+
278+
public function testClearDoesNotRemoveNonNamespacedSessionValues()
279+
{
280+
$this->session->set('foo', 'baz');
281+
$this->storage->clear();
282+
283+
$this->assertTrue($this->session->has('foo'));
284+
$this->assertSame('baz', $this->session->get('foo'));
285+
}
259286
}

‎src/Symfony/Component/Security/Csrf/TokenStorage/ClearableTokenStorageInterface.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Csrf\TokenStorage;
13+
14+
/**
15+
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
16+
*/
17+
interface ClearableTokenStorageInterface extends TokenStorageInterface
18+
{
19+
/**
20+
* Removes all CSRF tokens.
21+
*/
22+
public function clear();
23+
}

‎src/Symfony/Component/Security/Csrf/TokenStorage/NativeSessionTokenStorage.php

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*
1919
* @author Bernhard Schussek <bschussek@gmail.com>
2020
*/
21-
class NativeSessionTokenStorage implements TokenStorageInterface
21+
class NativeSessionTokenStorage implements ClearableTokenStorageInterface
2222
{
2323
/**
2424
* The namespace used to store values in the session.
@@ -96,6 +96,14 @@ public function removeToken($tokenId)
9696
return $token;
9797
}
9898

99+
/**
100+
* {@inheritdoc}
101+
*/
102+
public function clear()
103+
{
104+
unset($_SESSION[$this->namespace]);
105+
}
106+
99107
private function startSession()
100108
{
101109
if (PHP_SESSION_NONE === session_status()) {

0 commit comments

Comments
 (0)