diff --git a/src/Symfony/Component/Debug/Exception/FlattenException.php b/src/Symfony/Component/Debug/Exception/FlattenException.php index 51baeb9be8b75..ae103126947da 100644 --- a/src/Symfony/Component/Debug/Exception/FlattenException.php +++ b/src/Symfony/Component/Debug/Exception/FlattenException.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Debug\Exception; +use Symfony\Component\HttpFoundation\Exception\RequestExceptionInterface; use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface; /** @@ -41,6 +42,8 @@ public static function create(\Exception $exception, $statusCode = null, array $ if ($exception instanceof HttpExceptionInterface) { $statusCode = $exception->getStatusCode(); $headers = array_merge($headers, $exception->getHeaders()); + } elseif ($exception instanceof RequestExceptionInterface) { + $statusCode = 400; } if (null === $statusCode) { diff --git a/src/Symfony/Component/Debug/Tests/Exception/FlattenExceptionTest.php b/src/Symfony/Component/Debug/Tests/Exception/FlattenExceptionTest.php index aa4c2d0d15d72..b5438f384342b 100644 --- a/src/Symfony/Component/Debug/Tests/Exception/FlattenExceptionTest.php +++ b/src/Symfony/Component/Debug/Tests/Exception/FlattenExceptionTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Debug\Tests\Exception; use Symfony\Component\Debug\Exception\FlattenException; +use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException; @@ -78,6 +79,11 @@ public function testStatusCode() $flattened = FlattenException::create(new UnsupportedMediaTypeHttpException()); $this->assertEquals('415', $flattened->getStatusCode()); + + if (class_exists(SuspiciousOperationException::class)) { + $flattened = FlattenException::create(new SuspiciousOperationException()); + $this->assertEquals('400', $flattened->getStatusCode()); + } } public function testHeadersForHttpException() diff --git a/src/Symfony/Component/HttpFoundation/Exception/ConflictingHeadersException.php b/src/Symfony/Component/HttpFoundation/Exception/ConflictingHeadersException.php index fa5f1c7873270..5fcf5b42695e7 100644 --- a/src/Symfony/Component/HttpFoundation/Exception/ConflictingHeadersException.php +++ b/src/Symfony/Component/HttpFoundation/Exception/ConflictingHeadersException.php @@ -14,10 +14,8 @@ /** * The HTTP request contains headers with conflicting information. * - * This exception should trigger an HTTP 400 response in your application code. - * * @author Magnus Nordlander */ -class ConflictingHeadersException extends \RuntimeException +class ConflictingHeadersException extends \UnexpectedValueException implements RequestExceptionInterface { } diff --git a/src/Symfony/Component/HttpFoundation/Exception/RequestExceptionInterface.php b/src/Symfony/Component/HttpFoundation/Exception/RequestExceptionInterface.php new file mode 100644 index 0000000000000..478d0dc7e4e6b --- /dev/null +++ b/src/Symfony/Component/HttpFoundation/Exception/RequestExceptionInterface.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpFoundation\Exception; + +/** + * Interface for Request exceptions. + * + * Exceptions implementing this interface should trigger an HTTP 400 response in the application code. + */ +interface RequestExceptionInterface +{ +} diff --git a/src/Symfony/Component/HttpFoundation/Exception/SuspiciousOperationException.php b/src/Symfony/Component/HttpFoundation/Exception/SuspiciousOperationException.php new file mode 100644 index 0000000000000..ae7a5f1332b1f --- /dev/null +++ b/src/Symfony/Component/HttpFoundation/Exception/SuspiciousOperationException.php @@ -0,0 +1,20 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpFoundation\Exception; + +/** + * Raised when a user has performed an operation that should be considered + * suspicious from a security perspective. + */ +class SuspiciousOperationException extends \UnexpectedValueException implements RequestExceptionInterface +{ +} diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index 4c383ec69da20..9b01180ac83d9 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpFoundation; use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException; +use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException; use Symfony\Component\HttpFoundation\Session\SessionInterface; /** @@ -206,6 +207,9 @@ class Request protected static $requestFactory; + private $isHostValid = true; + private $isClientIpsValid = true; + /** * Constructor. * @@ -819,7 +823,12 @@ public function getClientIps() } if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) { - throw new ConflictingHeadersException('The request has both a trusted Forwarded header and a trusted Client IP header, conflicting with each other with regards to the originating IP addresses of the request. This is the result of a misconfiguration. You should either configure your proxy only to send one of these headers, or configure Symfony to distrust one of them.'); + if (!$this->isClientIpsValid) { + return array('0.0.0.0'); + } + $this->isClientIpsValid = false; + + throw new ConflictingHeadersException('The request has both a trusted Forwarded header and a trusted Client IP header, conflicting with each other with regards to the originating IP addresses of the request. This is the result of a misconfiguration. You should either configure your proxy only to send one of these headers, or configure your project to distrust one of them.'); } if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) { @@ -1198,7 +1207,7 @@ public function isSecure() * * @return string * - * @throws \UnexpectedValueException when the host name is invalid + * @throws SuspiciousOperationException when the host name is invalid or not trusted */ public function getHost() { @@ -1220,7 +1229,12 @@ public function getHost() // check that it does not contain forbidden characters (see RFC 952 and RFC 2181) // use preg_replace() instead of preg_match() to prevent DoS attacks with long host names if ($host && '' !== preg_replace('/(?:^\[)?[a-zA-Z0-9-:\]_]+\.?/', '', $host)) { - throw new \UnexpectedValueException(sprintf('Invalid Host "%s"', $host)); + if (!$this->isHostValid) { + return ''; + } + $this->isHostValid = false; + + throw new SuspiciousOperationException(sprintf('Invalid Host "%s".', $host)); } if (count(self::$trustedHostPatterns) > 0) { @@ -1238,7 +1252,12 @@ public function getHost() } } - throw new \UnexpectedValueException(sprintf('Untrusted Host "%s"', $host)); + if (!$this->isHostValid) { + return ''; + } + $this->isHostValid = false; + + throw new SuspiciousOperationException(sprintf('Untrusted Host "%s".', $host)); } return $host; diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php index 9e8e5fe71c750..c2653b9b831fd 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\HttpFoundation\Tests; +use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException; use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Request; @@ -1871,8 +1872,8 @@ public function testTrustedHosts() try { $request->getHost(); $this->fail('Request::getHost() should throw an exception when host is not trusted.'); - } catch (\UnexpectedValueException $e) { - $this->assertEquals('Untrusted Host "evil.com"', $e->getMessage()); + } catch (SuspiciousOperationException $e) { + $this->assertEquals('Untrusted Host "evil.com".', $e->getMessage()); } // trusted hosts @@ -1935,7 +1936,7 @@ public function testHostValidity($host, $isValid, $expectedHost = null, $expecte $this->assertSame($expectedPort, $request->getPort()); } } else { - $this->setExpectedException('UnexpectedValueException', 'Invalid Host'); + $this->setExpectedException(SuspiciousOperationException::class, 'Invalid Host'); $request->getHost(); } } diff --git a/src/Symfony/Component/HttpKernel/EventListener/ValidateRequestListener.php b/src/Symfony/Component/HttpKernel/EventListener/ValidateRequestListener.php index 00096ccf9e4f2..b96c7814ee433 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/ValidateRequestListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/ValidateRequestListener.php @@ -16,8 +16,7 @@ use Symfony\Component\HttpKernel\KernelEvents; /** - * Validates that the headers and other information indicating the - * client IP address of a request are consistent. + * Validates Requests. * * @author Magnus Nordlander */ @@ -36,9 +35,10 @@ public function onKernelRequest(GetResponseEvent $event) $request = $event->getRequest(); if ($request::getTrustedProxies()) { - // This will throw an exception if the headers are inconsistent. $request->getClientIps(); } + + $request->getHost(); } /** diff --git a/src/Symfony/Component/HttpKernel/HttpKernel.php b/src/Symfony/Component/HttpKernel/HttpKernel.php index c63a6af832021..cad23df99ad39 100644 --- a/src/Symfony/Component/HttpKernel/HttpKernel.php +++ b/src/Symfony/Component/HttpKernel/HttpKernel.php @@ -25,7 +25,7 @@ use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent; use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; use Symfony\Component\HttpKernel\Event\PostResponseEvent; -use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException; +use Symfony\Component\HttpFoundation\Exception\RequestExceptionInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Response; @@ -67,8 +67,8 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ try { return $this->handleRaw($request, $type); } catch (\Exception $e) { - if ($e instanceof ConflictingHeadersException) { - $e = new BadRequestHttpException('The request headers contain conflicting information regarding the origin of this request.', $e); + if ($e instanceof RequestExceptionInterface) { + $e = new BadRequestHttpException($e->getMessage(), $e); } if (false === $catch) { $this->finishRequest($request, $type); diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/RouterListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/RouterListenerTest.php index 0b772e5d11154..a2c61736f51b0 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/RouterListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/RouterListenerTest.php @@ -9,11 +9,17 @@ * file that was distributed with this source code. */ -namespace Symfony\Component\HttpKernel\Tests\EventListener; - +use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Controller\ArgumentResolver; +use Symfony\Component\HttpKernel\Controller\ControllerResolver; +use Symfony\Component\HttpKernel\EventListener\ExceptionListener; use Symfony\Component\HttpKernel\EventListener\RouterListener; +use Symfony\Component\HttpKernel\EventListener\ValidateRequestListener; use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\HttpKernel\HttpKernel; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\Routing\RequestContext; @@ -154,4 +160,26 @@ public function getLoggingParameterData() array(array(), 'Matched route "{route}".', array('route' => 'n/a', 'route_parameters' => array(), 'request_uri' => 'http://localhost/', 'method' => 'GET')), ); } + + public function testWithBadRequest() + { + $requestStack = new RequestStack(); + + $requestMatcher = $this->getMock('Symfony\Component\Routing\Matcher\RequestMatcherInterface'); + $requestMatcher->expects($this->never())->method('matchRequest'); + + $dispatcher = new EventDispatcher(); + $dispatcher->addSubscriber(new ValidateRequestListener()); + $dispatcher->addSubscriber(new RouterListener($requestMatcher, $requestStack, new RequestContext())); + $dispatcher->addSubscriber(new ExceptionListener(function () { + return new Response('Exception handled', 400); + })); + + $kernel = new HttpKernel($dispatcher, new ControllerResolver(), $requestStack, new ArgumentResolver()); + + $request = Request::create('http://localhost/'); + $request->headers->set('host', '###'); + $response = $kernel->handle($request); + $this->assertSame(400, $response->getStatusCode()); + } } diff --git a/src/Symfony/Component/HttpKernel/composer.json b/src/Symfony/Component/HttpKernel/composer.json index 66e50ef80c642..eb72ca763287d 100644 --- a/src/Symfony/Component/HttpKernel/composer.json +++ b/src/Symfony/Component/HttpKernel/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=5.5.9", "symfony/event-dispatcher": "~2.8|~3.0", - "symfony/http-foundation": "~2.8.13|~3.1.6|~3.2", + "symfony/http-foundation": "~3.3", "symfony/debug": "~2.8|~3.0", "psr/log": "~1.0" },