From: Nicolas Grekas <nicolas.grekas@gmail.com>
Date: Wed, 19 Jul 2017 11:06:40 +0200
Subject: [Security] Validate redirect targets using the session cookie domain

[CVE-2017-16652] https://symfony.com/blog/cve-2017-16652-open-redirect-vulnerability-on-security-handlers

Origin: upstream, https://github.com/symfony/symfony/commit/52b06f1c21c69a4557ab3716afe56dea4815dfdf
---
 .../Compiler/AddSessionDomainConstraintPass.php    |  40 +++++++
 .../Bundle/SecurityBundle/SecurityBundle.php       |   3 +
 .../AddSessionDomainConstraintPassTest.php         | 131 +++++++++++++++++++++
 src/Symfony/Component/Security/Http/HttpUtils.php  |   9 +-
 .../Security/Http/Tests/HttpUtilsTest.php          |  32 +++++
 5 files changed, 214 insertions(+), 1 deletion(-)
 create mode 100644 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php
 create mode 100644 src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php
new file mode 100644
index 0000000..0adc143
--- /dev/null
+++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php
@@ -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));
+    }
+}
diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php
index f2dfc99..61649f3 100644
--- a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php
+++ b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php
@@ -12,8 +12,10 @@
 namespace Symfony\Bundle\SecurityBundle;
 
 use Symfony\Component\HttpKernel\Bundle\Bundle;
+use Symfony\Component\DependencyInjection\Compiler\PassConfig;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 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\FormLoginLdapFactory;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory;
@@ -55,5 +57,6 @@ class SecurityBundle extends Bundle
         $extension->addUserProviderFactory(new InMemoryFactory());
         $extension->addUserProviderFactory(new LdapFactory());
         $container->addCompilerPass(new AddSecurityVotersPass());
+        $container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING);
     }
 }
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php
new file mode 100644
index 0000000..f476b5e
--- /dev/null
+++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php
@@ -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;
+    }
+}
diff --git a/src/Symfony/Component/Security/Http/HttpUtils.php b/src/Symfony/Component/Security/Http/HttpUtils.php
index ed737a2..adfac12 100644
--- a/src/Symfony/Component/Security/Http/HttpUtils.php
+++ b/src/Symfony/Component/Security/Http/HttpUtils.php
@@ -29,22 +29,25 @@ class HttpUtils
 {
     private $urlGenerator;
     private $urlMatcher;
+    private $domainRegexp;
 
     /**
      * Constructor.
      *
      * @param UrlGeneratorInterface                       $urlGenerator A UrlGeneratorInterface instance
      * @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
      */
-    public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatcher = null)
+    public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatcher = null, $domainRegexp = null)
     {
         $this->urlGenerator = $urlGenerator;
         if ($urlMatcher !== null && !$urlMatcher instanceof UrlMatcherInterface && !$urlMatcher instanceof RequestMatcherInterface) {
             throw new \InvalidArgumentException('Matcher must either implement UrlMatcherInterface or RequestMatcherInterface.');
         }
         $this->urlMatcher = $urlMatcher;
+        $this->domainRegexp = $domainRegexp;
     }
 
     /**
@@ -58,6 +61,10 @@ class HttpUtils
      */
     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);
     }
 
diff --git a/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php b/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php
index 45a0281..93be3cf 100644
--- a/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php
+++ b/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php
@@ -37,6 +37,38 @@ class HttpUtilsTest extends \PHPUnit_Framework_TestCase
         $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()
     {
         $utils = new HttpUtils($urlGenerator = $this->getMock('Symfony\Component\Routing\Generator\UrlGeneratorInterface'));