8000 [Security] Validate redirect targets using the session cookie domain · symfony/symfony@435b217 · GitHub
[go: up one dir, main page]
Skip to content

Commit 435b217

Browse files
nicolas-grekasnicolas-grekas
committed
[Security] Validate redirect targets using the session cookie domain
1 parent 3d45e10 commit 435b217

File tree

5 files changed

+214
-1
lines changed

5 files changed

+214
-1
lines changed

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
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\Reference;
15+
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
17+
18+
/**
19+
* Uses the session domain to restrict allowed redirection targets.
20+
*
21+
* @author Nicolas Grekas <p@tchwork.com>
22+
*/
23+
class AddSessionDomainConstraintPass implements CompilerPassInterface
24+
{
25+
/**
26+
* {@inheritdoc}
27+
*/
28+
public function process(ContainerBuilder $container)
29+
{
30+
if (!$container->hasParameter('session.storage.options') || !$container->has('security.http_utils')) {
31+
return;
32+
}
33+
34+
$sessionOptions = $container->getParameter('session.storage.options');
35+
$domainRegexp = empty($sessionOptions['cookie_domain']) ? '%s' : sprintf('(?:%%s|(?:.+\.)?%s) 57A7 ', preg_quote(trim($sessionOptions['cookie_domain'], '.')));
36+
$domainRegexp = (empty($sessionOptions['cookie_secure']) ? 'https?://' : 'https://').$domainRegexp;
37+
38+
$container->findDefinition('security.http_utils')->addArgument(sprintf('{^%s$}i', $domainRegexp));
39+
}
40+
}

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

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

1414
use Symfony\Component\HttpKernel\Bundle\Bundle;
15+
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
1516use Symfony\Component\DependencyInjection\ContainerBuilder;
1617
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
18+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
1719
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
1820
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginLdapFactory;
1921
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory;
@@ -55,5 +57,6 @@ public function build(ContainerBuilder $container)
5557
$extension->addUserProviderFactory(new InMemoryFactory());
5658
$extension->addUserProviderFactory(new LdapFactory());
5759
$container->addCompilerPass(new AddSecurityVotersPass());
60+
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING);
5861
}
5962
}

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
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\Tests\DependencyInjection\Compiler;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension;
16+
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
17+
use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
18+
use Symfony\Component\DependencyInjection\ContainerBuilder;
19+
use Symfony\Component\HttpFoundation\Request;
20+
21+
class AddSessionDomainConstraintPassTest extends TestCase
22+
{
23+
public function testSessionCookie()
24+
{
25+
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.', 'cookie_secure' => true));
26+
27+
$utils = $container->get('security.http_utils');
28+
$request = Request::create('/', 'get');
29+
30+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
31+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
32+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
33+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
34+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
35+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
36+
}
37+
38+
public function testSessionNoDomain()
39+
{
40+
$container = $this->createContainer(array('cookie_secure' => true));
41+
42+
$utils = $container->get('security.http_utils');
43+
$request = Request::create('/', 'get');
44+
45+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/'));
46+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/'));
47+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
48+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
49+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
50+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
51+
}
52+
53+
public function testSessionNoSecure()
54+
{
55+
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.'));
56+
57+
$utils = $container->get('security.http_utils');
58+
$request = Request::create('/', 'get');
59+
60+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
61+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
62+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
63+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
64+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
65+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
66+
}
67+
68+
public function testSessionNoSecureAndNoDomain()
69+
{
70+
$container = $this->createContainer(array());
71+
72+
$utils = $container->get('security.http_utils');
73+
$request = Request::create('/', 'get');
74+
75+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/'));
76+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/'));
77+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
78+
$this->assertTrue($utils->createRedirectResponse($request, 'http://localhost/foo')->isRedirect('http://localhost/foo'));
79+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
80+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
81+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
82+
}
83+
84+
public function testNoSession()
85+
{
86+
$container = $this->createContainer(null);
87+
88+
$utils = $container->get('security.http_utils');
89+
$request = Request::create('/', 'get');
90+
91+
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
92+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
93+
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
94+
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('https://www.localhost/foo'));
95+
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
96+
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://pirate.com/foo'));
97+
}
98+
99+
private function createContainer($sessionStorageOptions)
100+
{
101+
$container = new ContainerBuilder();
102+
$container->setParameter('kernel.cache_dir', __DIR__);
103+
$container->setParameter('kernel.charset', 'UTF-8');
104+
$container->setParameter('kernel.container_class', 'cc');
105+
$container->setParameter('kernel.debug', true);
106+
$container->setParameter('kernel.root_dir', __DIR__);
107+
$container->setParameter('kernel.secret', __DIR__);
108+
if (null !== $sessionStorageOptions) {
109+
$container->setParameter('session.storage.options', $sessionStorageOptions);
110+
}
111+
$container->setParameter('request_listener.http_port', 80);
112+
$container->setParameter('request_listener.https_port', 443);
113+
114+
$config = array(
115+
'security' => array(
116+
'providers' => array('some_provider' => array('id' => 'foo')),
117+
'firewalls' => array('some_firewall' => array('security' => false)),
118+
),
119+
);
120+
121+
$ext = new FrameworkExtension();
122+
$ext->load(array(), $container);
123+
124+
$ext = new SecurityExtension();
125+
$ext->load($config, $container);
126+
127+
(new AddSessionDomainConstraintPass())->process($container);
128+
129+
return $container;
130+
}
131+
}

‎src/Symfony/Component/Security/Http/HttpUtils.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,22 +29,25 @@ class HttpUtils
2929
{
3030
private $urlGenerator;
3131
private $urlMatcher;
32+
private $domainRegexp;
3233

3334
/**
3435
* Constructor.
3536
*
3637
* @param UrlGeneratorInterface $urlGenerator A UrlGeneratorInterface instance
3738
* @param UrlMatcherInterface|RequestMatcherInterface $urlMatcher The URL or Request matcher
39+
* @param string|null $domainRegexp A regexp that the target of HTTP redirections must match, scheme included
3840
*
3941
* @throws \InvalidArgumentException
4042
*/
41-
public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatcher = null)
43+
public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatcher = null, $domainRegexp = null)
4244
{
4345
$this->urlGenerator = $urlGenerator;
4446
if ($urlMatcher !== null && !$urlMatcher instanceof UrlMatcherInterface && !$urlMatcher instanceof RequestMatcherInterface) {
4547
throw new \InvalidArgumentException('Matcher must either implement UrlMatcherInterface or RequestMatcherInterface.');
4648
}
4749
$this->urlMatcher = $urlMatcher;
50+
$this->domainRegexp = $domainRegexp;
4851
}
4952

5053
/**
@@ -58,6 +61,10 @@ public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatc
5861
*/
5962
public function createRedirectResponse(Request $request, $path, $status = 302)
6063
{
64+
if (null !== $this->domainRegexp && preg_match('#^https?://[^/]++#i', $path, $host) && !preg_match(sprintf($this->domainRegexp, preg_quote($request->getHttpHost())), $host[0])) {
65+
$path = '/';
66+
}
67+
6168
return new RedirectResponse($this->generateUri($request, $path), $status);
6269
}
6370

‎src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,38 @@ public function testCreateRedirectResponseWithAbsoluteUrl()
3838
$this->assertTrue($response->isRedirect('http://symfony.com/'));
3939
}
4040

41+
public function testCreateRedirectResponseWithDomainRegexp()
42+
{
43+
$utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://symfony\.com$#i');
44+
$response = $utils->createRedirectResponse($this->getRequest(), 'http://symfony.com/blog');
45+
46+
$this->assertTrue($response->isRedirect('http://symfony.com/blog'));
47+
}
48+
49+
public function testCreateRedirectResponseWithRequestsDomain()
50+
{
51+
$utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i');
52+
$response = $utils->createRedirectResponse($this->getRequest(), 'http://localhost/blog');
53+
54+
$this->assertTrue($response->isRedirect('http://localhost/blog'));
55+
}
56+
57+
public function testCreateRedirectResponseWithBadRequestsDomain()
58+
{
59+
$utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i');
60+
$response = $utils->createRedirectResponse($this->getRequest(), 'http://pirate.net/foo');
61+
62+
$this->assertTrue($response->isRedirect('http://localhost/'));
63+
}
64+
65+
public function testCreateRedirectResponseWithProtocolRelativeTarget()
66+
{
67+
$utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i');
68+
$response = $utils->createRedirectResponse($this->getRequest(), '//evil.com/do-bad-things');
69+
70+
$this->assertTrue($response->isRedirect('http://localhost//evil.com/do-bad-things'), 'Protocol-relative redirection should not be supported for security reasons');
71+
}
72+
4173
public function testCreateRedirectResponseWithRouteName()
4274
{
4375
$utils = new HttpUtils($urlGenerator = $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock());

0 commit comments

Comments
 (0)
0