[Security\Http] detect bad redirect targets using backslashes

xabbuh · nicolas-grekas · commit 99a0cec0a6be · 2018-11-29T16:25:45.000+01:00
diff --git a/src/Symfony/Component/Security/Http/HttpUtils.php b/src/Symfony/Component/Security/Http/HttpUtils.php
@@ -59,7 +59,7 @@ public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatc
      */
     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])) {
+        if (null !== $this->domainRegexp && preg_match('#^https?:[/\\\\]{2,}+[^/]++#i', $path, $host) && !preg_match(sprintf($this->domainRegexp, preg_quote($request->getHttpHost())), $host[0])) {
             $path = '/';
         }
 
diff --git a/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php b/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php
@@ -54,14 +54,28 @@ public function testCreateRedirectResponseWithRequestsDomain()
         $this->assertTrue($response->isRedirect('http://localhost/blog'));
     }
 
-    public function testCreateRedirectResponseWithBadRequestsDomain()
+    /**
+     * @dataProvider badRequestDomainUrls
+     */
+    public function testCreateRedirectResponseWithBadRequestsDomain($url)
     {
         $utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i');
-        $response = $utils->createRedirectResponse($this->getRequest(), 'http://pirate.net/foo');
+        $response = $utils->createRedirectResponse($this->getRequest(), $url);
 
         $this->assertTrue($response->isRedirect('http://localhost/'));
     }
 
+    public function badRequestDomainUrls()
+    {
+        return array(
+            array('http://pirate.net/foo'),
+            array('http:\\\\pirate.net/foo'),
+            array('http:/\\pirate.net/foo'),
+            array('http:\\/pirate.net/foo'),
+            array('http://////pirate.net/foo'),
+        );
+    }
+
     public function testCreateRedirectResponseWithProtocolRelativeTarget()
     {
         $utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i');

Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,7 @@ public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatc`
`59`	`59`	`*/`
`60`	`60`	`public function createRedirectResponse(Request $request, $path, $status = 302)`
`61`	`61`	`{`
`62`		`- if (null !== $this->domainRegexp && preg_match('#^https?://[^/]++#i', $path, $host) && !preg_match(sprintf($this->domainRegexp, preg_quote($request->getHttpHost())), $host[0])) {`
	`62`	`+ if (null !== $this->domainRegexp && preg_match('#^https?:[/\\\\]{2,}+[^/]++#i', $path, $host) && !preg_match(sprintf($this->domainRegexp, preg_quote($request->getHttpHost())), $host[0])) {`
`63`	`63`	`$path = '/';`
`64`	`64`	`}`
`65`	`65`