security #24995 Validate redirect targets using the session cookie domain (nicolas-grekas)
This PR was merged into the 2.7 branch.
Discussion
----------
Validate redirect targets using the session cookie domain
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | n/a
| License | MIT
| Doc PR | n/a
<!--
- Bug fixes must be submitted against the lowest branch where they apply
(lowest branches are regularly merged to upper ones so they get the fixes too).
- Features and deprecations must be submitted against the master branch.
- Please fill in this template according to the PR you're about to submit.
- Replace this comment by a description of what your PR is solving.
-->
Commits
-------
52b06f1c21
[Security] Validate redirect targets using the session cookie domain
This commit is contained in:
commit
4d288439bc
@ -0,0 +1,40 @@
|
|||||||
|
<?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\Reference;
|
||||||
|
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||||
|
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Uses the session domain to restrict allowed redirection targets.
|
||||||
|
*
|
||||||
|
* @author Nicolas Grekas <p@tchwork.com>
|
||||||
|
*/
|
||||||
|
class AddSessionDomainConstraintPass implements CompilerPassInterface
|
||||||
|
{
|
||||||
|
/**
|
||||||
|
* {@inheritdoc}
|
||||||
|
*/
|
||||||
|
public function process(ContainerBuilder $container)
|
||||||
|
{
|
||||||
|
if (!$container->hasParameter('session.storage.options') || !$container->has('security.http_utils')) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
$sessionOptions = $container->getParameter('session.storage.options');
|
||||||
|
$domainRegexp = empty($sessionOptions['cookie_domain']) ? '%s' : sprintf('(?:%%s|(?:.+\.)?%s)', preg_quote(trim($sessionOptions['cookie_domain'], '.')));
|
||||||
|
$domainRegexp = (empty($sessionOptions['cookie_secure']) ? 'https?://' : 'https://').$domainRegexp;
|
||||||
|
|
||||||
|
$container->findDefinition('security.http_utils')->addArgument(sprintf('{^%s$}i', $domainRegexp));
|
||||||
|
}
|
||||||
|
}
|
@ -12,8 +12,10 @@
|
|||||||
namespace Symfony\Bundle\SecurityBundle;
|
namespace Symfony\Bundle\SecurityBundle;
|
||||||
|
|
||||||
use Symfony\Component\HttpKernel\Bundle\Bundle;
|
use Symfony\Component\HttpKernel\Bundle\Bundle;
|
||||||
|
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
|
||||||
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||||
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
|
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
|
||||||
|
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
|
||||||
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
|
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
|
||||||
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory;
|
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory;
|
||||||
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpDigestFactory;
|
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpDigestFactory;
|
||||||
@ -47,5 +49,6 @@ class SecurityBundle extends Bundle
|
|||||||
|
|
||||||
$extension->addUserProviderFactory(new InMemoryFactory());
|
$extension->addUserProviderFactory(new InMemoryFactory());
|
||||||
$container->addCompilerPass(new AddSecurityVotersPass());
|
$container->addCompilerPass(new AddSecurityVotersPass());
|
||||||
|
$container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -0,0 +1,131 @@
|
|||||||
|
<?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\Tests\DependencyInjection\Compiler;
|
||||||
|
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension;
|
||||||
|
use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
|
||||||
|
use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
|
||||||
|
use Symfony\Component\DependencyInjection\ContainerBuilder;
|
||||||
|
use Symfony\Component\HttpFoundation\Request;
|
||||||
|
|
||||||
|
class AddSessionDomainConstraintPassTest extends TestCase
|
||||||
|
{
|
||||||
|
public function testSessionCookie()
|
||||||
|
{
|
||||||
|
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.', 'cookie_secure' => true));
|
||||||
|
|
||||||
|
$utils = $container->get('security.http_utils');
|
||||||
|
$request = Request::create('/', 'get');
|
||||||
|
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSessionNoDomain()
|
||||||
|
{
|
||||||
|
$container = $this->createContainer(array('cookie_secure' => true));
|
||||||
|
|
||||||
|
$utils = $container->get('security.http_utils');
|
||||||
|
$request = Request::create('/', 'get');
|
||||||
|
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSessionNoSecure()
|
||||||
|
{
|
||||||
|
$container = $this->createContainer(array('cookie_domain' => '.symfony.com.'));
|
||||||
|
|
||||||
|
$utils = $container->get('security.http_utils');
|
||||||
|
$request = Request::create('/', 'get');
|
||||||
|
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSessionNoSecureAndNoDomain()
|
||||||
|
{
|
||||||
|
$container = $this->createContainer(array());
|
||||||
|
|
||||||
|
$utils = $container->get('security.http_utils');
|
||||||
|
$request = Request::create('/', 'get');
|
||||||
|
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://localhost/foo')->isRedirect('http://localhost/foo'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/'));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testNoSession()
|
||||||
|
{
|
||||||
|
$container = $this->createContainer(null);
|
||||||
|
|
||||||
|
$utils = $container->get('security.http_utils');
|
||||||
|
$request = Request::create('/', 'get');
|
||||||
|
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('https://www.localhost/foo'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog'));
|
||||||
|
$this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://pirate.com/foo'));
|
||||||
|
}
|
||||||
|
|
||||||
|
private function createContainer($sessionStorageOptions)
|
||||||
|
{
|
||||||
|
$container = new ContainerBuilder();
|
||||||
|
$container->setParameter('kernel.cache_dir', __DIR__);
|
||||||
|
$container->setParameter('kernel.charset', 'UTF-8');
|
||||||
|
$container->setParameter('kernel.container_class', 'cc');
|
||||||
|
$container->setParameter('kernel.debug', true);
|
||||||
|
$container->setParameter('kernel.root_dir', __DIR__);
|
||||||
|
$container->setParameter('kernel.secret', __DIR__);
|
||||||
|
if (null !== $sessionStorageOptions) {
|
||||||
|
$container->setParameter('session.storage.options', $sessionStorageOptions);
|
||||||
|
}
|
||||||
|
$container->setParameter('request_listener.http_port', 80);
|
||||||
|
$container->setParameter('request_listener.https_port', 443);
|
||||||
|
|
||||||
|
$config = array(
|
||||||
|
'security' => array(
|
||||||
|
'providers' => array('some_provider' => array('id' => 'foo')),
|
||||||
|
'firewalls' => array('some_firewall' => array('security' => false)),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
$ext = new FrameworkExtension();
|
||||||
|
$ext->load(array(), $container);
|
||||||
|
|
||||||
|
$ext = new SecurityExtension();
|
||||||
|
$ext->load($config, $container);
|
||||||
|
|
||||||
|
(new AddSessionDomainConstraintPass())->process($container);
|
||||||
|
|
||||||
|
return $container;
|
||||||
|
}
|
||||||
|
}
|
@ -29,20 +29,23 @@ class HttpUtils
|
|||||||
{
|
{
|
||||||
private $urlGenerator;
|
private $urlGenerator;
|
||||||
private $urlMatcher;
|
private $urlMatcher;
|
||||||
|
private $domainRegexp;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param UrlGeneratorInterface $urlGenerator A UrlGeneratorInterface instance
|
* @param UrlGeneratorInterface $urlGenerator A UrlGeneratorInterface instance
|
||||||
* @param UrlMatcherInterface|RequestMatcherInterface $urlMatcher The URL or Request matcher
|
* @param UrlMatcherInterface|RequestMatcherInterface $urlMatcher The URL or Request matcher
|
||||||
|
* @param string|null $domainRegexp A regexp that the target of HTTP redirections must match, scheme included
|
||||||
*
|
*
|
||||||
* @throws \InvalidArgumentException
|
* @throws \InvalidArgumentException
|
||||||
*/
|
*/
|
||||||
public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatcher = null)
|
public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatcher = null, $domainRegexp = null)
|
||||||
{
|
{
|
||||||
$this->urlGenerator = $urlGenerator;
|
$this->urlGenerator = $urlGenerator;
|
||||||
if (null !== $urlMatcher && !$urlMatcher instanceof UrlMatcherInterface && !$urlMatcher instanceof RequestMatcherInterface) {
|
if (null !== $urlMatcher && !$urlMatcher instanceof UrlMatcherInterface && !$urlMatcher instanceof RequestMatcherInterface) {
|
||||||
throw new \InvalidArgumentException('Matcher must either implement UrlMatcherInterface or RequestMatcherInterface.');
|
throw new \InvalidArgumentException('Matcher must either implement UrlMatcherInterface or RequestMatcherInterface.');
|
||||||
}
|
}
|
||||||
$this->urlMatcher = $urlMatcher;
|
$this->urlMatcher = $urlMatcher;
|
||||||
|
$this->domainRegexp = $domainRegexp;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -56,6 +59,10 @@ class HttpUtils
|
|||||||
*/
|
*/
|
||||||
public function createRedirectResponse(Request $request, $path, $status = 302)
|
public function createRedirectResponse(Request $request, $path, $status = 302)
|
||||||
{
|
{
|
||||||
|
if (null !== $this->domainRegexp && preg_match('#^https?://[^/]++#i', $path, $host) && !preg_match(sprintf($this->domainRegexp, preg_quote($request->getHttpHost())), $host[0])) {
|
||||||
|
$path = '/';
|
||||||
|
}
|
||||||
|
|
||||||
return new RedirectResponse($this->generateUri($request, $path), $status);
|
return new RedirectResponse($this->generateUri($request, $path), $status);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -38,6 +38,38 @@ class HttpUtilsTest extends TestCase
|
|||||||
$this->assertTrue($response->isRedirect('http://symfony.com/'));
|
$this->assertTrue($response->isRedirect('http://symfony.com/'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testCreateRedirectResponseWithDomainRegexp()
|
||||||
|
{
|
||||||
|
$utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://symfony\.com$#i');
|
||||||
|
$response = $utils->createRedirectResponse($this->getRequest(), 'http://symfony.com/blog');
|
||||||
|
|
||||||
|
$this->assertTrue($response->isRedirect('http://symfony.com/blog'));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testCreateRedirectResponseWithRequestsDomain()
|
||||||
|
{
|
||||||
|
$utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i');
|
||||||
|
$response = $utils->createRedirectResponse($this->getRequest(), 'http://localhost/blog');
|
||||||
|
|
||||||
|
$this->assertTrue($response->isRedirect('http://localhost/blog'));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testCreateRedirectResponseWithBadRequestsDomain()
|
||||||
|
{
|
||||||
|
$utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i');
|
||||||
|
$response = $utils->createRedirectResponse($this->getRequest(), 'http://pirate.net/foo');
|
||||||
|
|
||||||
|
$this->assertTrue($response->isRedirect('http://localhost/'));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testCreateRedirectResponseWithProtocolRelativeTarget()
|
||||||
|
{
|
||||||
|
$utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i');
|
||||||
|
$response = $utils->createRedirectResponse($this->getRequest(), '//evil.com/do-bad-things');
|
||||||
|
|
||||||
|
$this->assertTrue($response->isRedirect('http://localhost//evil.com/do-bad-things'), 'Protocol-relative redirection should not be supported for security reasons');
|
||||||
|
}
|
||||||
|
|
||||||
public function testCreateRedirectResponseWithRouteName()
|
public function testCreateRedirectResponseWithRouteName()
|
||||||
{
|
{
|
||||||
$utils = new HttpUtils($urlGenerator = $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock());
|
$utils = new HttpUtils($urlGenerator = $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock());
|
||||||
|
Reference in New Issue
Block a user