Commit 484b071d authored by catch's avatar catch

Issue #2507831 by dawehner, tim.plunkett, effulgentsia: Harden redirect...

Issue #2507831 by dawehner, tim.plunkett, effulgentsia: Harden redirect responses to make external URIs opt in (was SA-CORE-2015-002 foward-port)
parent bc62529e
<?php
/**
* @file
* Contains \Drupal\Component\HttpFoundation\SecuredRedirectResponse.
*/
namespace Drupal\Component\HttpFoundation;
use \Symfony\Component\HttpFoundation\RedirectResponse;
/**
* Provides a common base class for safe redirects.
*
* In case you want to redirect to external URLs use
* TrustedRedirectResponse.
*
* For local URLs we use LocalRedirectResponse which opts
* out of external redirects.
*/
abstract class SecuredRedirectResponse extends RedirectResponse {
/**
* Copies an existing redirect response into a safe one.
*
* The safe one cannot accidentally redirect to an external URL, unless
* actively wanted (see TrustedRedirectResponse).
*
* @param \Symfony\Component\HttpFoundation\RedirectResponse $response
* The original redirect.
*
* @return static
*/
public static function createFromRedirectResponse(RedirectResponse $response) {
$safe_response = new static($response->getTargetUrl(), $response->getStatusCode(), $response->headers->allPreserveCase());
$safe_response->setProtocolVersion($response->getProtocolVersion());
$safe_response->setCharset($response->getCharset());
return $safe_response;
}
/**
* {@inheritdoc}
*/
public function setTargetUrl($url) {
if (!$this->isSafe($url)) {
throw new \InvalidArgumentException(sprintf('It is not safe to redirect to %s', $url));
}
return parent::setTargetUrl($url);
}
/**
* Returns whether the URL is considered as safe to redirect to.
*
* @param string $url
* The URL checked for safety.
*
* @return bool
*/
abstract protected function isSafe($url);
}
......@@ -7,13 +7,15 @@
namespace Drupal\Core\EventSubscriber;
use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
use Drupal\Component\Utility\UrlHelper;
use Drupal\Core\Routing\LocalRedirectResponse;
use Drupal\Core\Routing\RequestContext;
use Drupal\Core\Routing\UrlGeneratorInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
......@@ -51,46 +53,86 @@ public function __construct(UrlGeneratorInterface $url_generator, RequestContext
public function checkRedirectUrl(FilterResponseEvent $event) {
$response = $event->getResponse();
if ($response instanceOf RedirectResponse) {
$options = array();
$request = $event->getRequest();
// Let the 'destination' query parameter override the redirect target.
// If $response is already a SecuredRedirectResponse, it might reject the
// new target as invalid, in which case proceed with the old target.
$destination = $request->query->get('destination');
// A destination from \Drupal::request()->query always overrides the
// current RedirectResponse. We do not allow absolute URLs to be passed
// via \Drupal::request()->query, as this can be an attack vector, with
// the following exception:
// - Absolute URLs that point to this site (i.e. same base URL and
// base path) are allowed.
if ($destination) {
if (!UrlHelper::isExternal($destination)) {
// The destination query parameter can be a relative URL in the sense
// of not including the scheme and host, but its path is expected to
// be absolute (start with a '/'). For such a case, prepend the
// scheme and host, because the 'Location' header must be absolute.
if (strpos($destination, '/') === 0) {
$destination = $request->getSchemeAndHttpHost() . $destination;
}
else {
// Legacy destination query parameters can be relative paths that
// have not yet been converted to URLs (outbound path processors
// and other URL handling still needs to be performed).
// @todo As generateFromPath() is deprecated, remove this in
// https://www.drupal.org/node/2418219.
$destination = UrlHelper::parse($destination);
$path = $destination['path'];
$options['query'] = $destination['query'];
$options['fragment'] = $destination['fragment'];
// The 'Location' HTTP header must always be absolute.
$options['absolute'] = TRUE;
$destination = $this->urlGenerator->generateFromPath($path, $options);
}
// The 'Location' HTTP header must always be absolute.
$destination = $this->getDestinationAsAbsoluteUrl($destination, $request->getSchemeAndHttpHost());
try {
$response->setTargetUrl($destination);
}
elseif (UrlHelper::externalIsLocal($destination, $this->requestContext->getCompleteBaseUrl())) {
$response->setTargetUrl($destination);
catch (\InvalidArgumentException $e) {
}
}
// Regardless of whether the target is the original one or the overridden
// destination, ensure that all redirects are safe.
if (!($response instanceOf SecuredRedirectResponse)) {
try {
// SecuredRedirectResponse is an abstract class that requires a
// concrete implementation. Default to LocalRedirectResponse, which
// considers only redirects to within the same site as safe.
$safe_response = LocalRedirectResponse::createFromRedirectResponse($response);
$safe_response->setRequestContext($this->requestContext);
}
catch (\InvalidArgumentException $e) {
// If the above failed, it's because the redirect target wasn't
// local. Do not follow that redirect. Display an error message
// instead. We're already catching one exception, so trigger_error()
// rather than throw another one.
// We don't throw an exception, because this is a client error rather than a
// server error.
$message = 'Redirects to external URLs are not allowed by default, use \Drupal\Core\Routing\TrustedRedirectResponse for it.';
trigger_error($message, E_USER_ERROR);
$safe_response = new Response($message, 400);
}
$event->setResponse($safe_response);
}
}
}
/**
* Converts the passed in destination into an absolute URL.
*
* @param string $destination
* The path for the destination. In case it starts with a slash it should
* have the base path included already.
* @param string $scheme_and_host
* The scheme and host string of the current request.
*
* @return string
* The destination as absolute URL.
*/
protected function getDestinationAsAbsoluteUrl($destination, $scheme_and_host) {
if (!UrlHelper::isExternal($destination)) {
// The destination query parameter can be a relative URL in the sense of
// not including the scheme and host, but its path is expected to be
// absolute (start with a '/'). For such a case, prepend the scheme and
// host, because the 'Location' header must be absolute.
if (strpos($destination, '/') === 0) {
$destination = $scheme_and_host . $destination;
}
else {
// Legacy destination query parameters can be relative paths that have
// not yet been converted to URLs (outbound path processors and other
// URL handling still needs to be performed).
// @todo As generateFromPath() is deprecated, remove this in
// https://www.drupal.org/node/2418219.
$destination = UrlHelper::parse($destination);
$path = $destination['path'];
$options = [
'query' => $destination['query'],
'fragment' => $destination['fragment'],
'absolute' => TRUE,
];
$destination = $this->urlGenerator->generateFromPath($path, $options);
}
}
return $destination;
}
/**
......
<?php
/**
* @file
* Contains \Drupal\Core\Routing\LocalAwareRedirectResponseTrait.
*/
namespace Drupal\Core\Routing;
use Drupal\Component\Utility\UrlHelper;
/**
* Provides a trait which ensures that a URL is safe to redirect to.
*/
trait LocalAwareRedirectResponseTrait {
/**
* The request context.
*
* @var \Drupal\Core\Routing\RequestContext
*/
protected $requestContext;
/**
* {@inheritdoc}
*/
protected function isLocal($url) {
return !UrlHelper::isExternal($url) || UrlHelper::externalIsLocal($url, $this->getRequestContext()->getCompleteBaseUrl());
}
/**
* Returns the request context.
*
* @return \Drupal\Core\Routing\RequestContext
*/
protected function getRequestContext() {
if (!isset($this->requestContext)) {
$this->requestContext = \Drupal::service('router.request_context');
}
return $this->requestContext;
}
/**
* Sets the request context.
*
* @param \Drupal\Core\Routing\RequestContext $request_context
* The request context.
*
* @return $this
*/
public function setRequestContext(RequestContext $request_context) {
$this->requestContext = $request_context;
return $this;
}
}
<?php
/**
* @file
* Contains \Drupal\Core\Routing\LocalRedirectResponse.
*/
namespace Drupal\Core\Routing;
use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
/**
* Provides a redirect response which cannot redirect to an external URL.
*/
class LocalRedirectResponse extends SecuredRedirectResponse {
use LocalAwareRedirectResponseTrait {
LocalAwareRedirectResponseTrait::isLocal as isSafe;
}
}
<?php
/**
* @file
* Contains \Drupal\Core\Routing\TrustedRedirectResponse.
*/
namespace Drupal\Core\Routing;
use Drupal\Component\HttpFoundation\SecuredRedirectResponse;
/**
* Provides a redirect response which contains trusted URLs.
*
* Use this class in case you know that you want to redirect to an external URL.
*/
class TrustedRedirectResponse extends SecuredRedirectResponse {
use LocalAwareRedirectResponseTrait;
/**
* A list of trusted URLs, which are safe to redirect to.
*
* @var string[]
*/
protected $trustedUrls = array();
/**
* {@inheritdoc}
*/
public function __construct($url, $status = 302, $headers = array()) {
$this->trustedUrls[$url] = TRUE;
parent::__construct($url, $status, $headers);
}
/**
* Sets the target URL to a trusted URL.
*
* @param string $url
* A trusted URL.
*
* @return $this
*/
public function setTrustedTargetUrl($url) {
$this->trustedUrls[$url] = TRUE;
return $this->setTargetUrl($url);
}
/**
* {@inheritdoc}
*/
protected function isSafe($url) {
return !empty($this->trustedUrls[$url]) || $this->isLocal($url);
}
}
......@@ -616,6 +616,20 @@ function testDuplicateFieldName() {
$this->assertUrl($url, array(), 'Stayed on the same page.');
}
/**
* Tests that external URLs in the 'destinations' query parameter are blocked.
*/
public function testExternalDestinations() {
$options = [
'query' => ['destinations' => ['http://example.com']],
];
$this->drupalPostForm('admin/structure/types/manage/article/fields/node.article.body/storage', [], 'Save field settings', $options);
// The external redirect should not fire.
$this->assertUrl('admin/structure/types/manage/article/fields/node.article.body/storage', $options);
$this->assertResponse(200);
$this->assertRaw('Attempt to update field <em class="placeholder">Body</em> failed: <em class="placeholder">The internal path component "http://example.com" is external. You are not allowed to specify an external URL together with internal:/.</em>.');
}
/**
* Tests that deletion removes field storages and fields as expected for a term.
*/
......
......@@ -9,7 +9,9 @@
use Drupal\Core\EventSubscriber\RedirectResponseSubscriber;
use Drupal\Core\Routing\RequestContext;
use Drupal\Core\Routing\TrustedRedirectResponse;
use Drupal\Tests\UnitTestCase;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
......@@ -24,6 +26,31 @@
*/
class RedirectResponseSubscriberTest extends UnitTestCase {
/**
* The mocked request context.
*
* @var \Drupal\Core\Routing\RequestContext|\PHPUnit_Framework_MockObject_MockObject
*/
protected $requestContext;
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->requestContext = $this->getMockBuilder('Drupal\Core\Routing\RequestContext')
->disableOriginalConstructor()
->getMock();
$this->requestContext->expects($this->any())
->method('getCompleteBaseUrl')
->willReturn('http://example.com/drupal');
$container = new Container();
$container->set('router.request_context', $this->requestContext);
\Drupal::setContainer($container);
}
/**
* Test destination detection and redirection.
*
......@@ -57,15 +84,9 @@ public function testDestinationRedirect(Request $request, $expected) {
]);
}
$request_context = $this->getMockBuilder('Drupal\Core\Routing\RequestContext')
->disableOriginalConstructor()
->getMock();
$request_context->expects($this->any())
->method('getCompleteBaseUrl')
->willReturn('http://example.com/drupal');
$request->headers->set('HOST', 'example.com');
$listener = new RedirectResponseSubscriber($url_generator, $request_context);
$listener = new RedirectResponseSubscriber($url_generator, $this->requestContext);
$dispatcher->addListener(KernelEvents::RESPONSE, array($listener, 'checkRedirectUrl'));
$event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST, $response);
$dispatcher->dispatch(KernelEvents::RESPONSE, $event);
......@@ -87,12 +108,9 @@ public function testDestinationRedirect(Request $request, $expected) {
public static function providerTestDestinationRedirect() {
return array(
array(new Request(), FALSE),
array(new Request(array('destination' => 'http://example.com')), FALSE),
array(new Request(array('destination' => 'http://example.com/foobar')), FALSE),
array(new Request(array('destination' => 'http://example.ca/drupal')), FALSE),
array(new Request(array('destination' => 'test')), 'http://example.com/drupal/test'),
array(new Request(array('destination' => '/test')), 'http://example.com/test'),
array(new Request(array('destination' => '/example.com')), 'http://example.com/example.com'),
array(new Request(array('destination' => '/drupal/test')), 'http://example.com/drupal/test'),
array(new Request(array('destination' => 'example.com')), 'http://example.com/drupal/example.com'),
array(new Request(array('destination' => 'example:com')), 'http://example.com/drupal/example:com'),
array(new Request(array('destination' => 'javascript:alert(0)')), 'http://example.com/drupal/javascript:alert(0)'),
array(new Request(array('destination' => 'http://example.com/drupal/')), 'http://example.com/drupal/'),
......@@ -101,7 +119,94 @@ public static function providerTestDestinationRedirect() {
}
/**
* @expectedException \InvalidArgumentException
* @dataProvider providerTestDestinationRedirectToExternalUrl
*
* @expectedException \PHPUnit_Framework_Error
*/
public function testDestinationRedirectToExternalUrl($request, $expected) {
$dispatcher = new EventDispatcher();
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
$response = new RedirectResponse('http://other-example.com');
$url_generator = $this->getMockBuilder('Drupal\Core\Routing\UrlGenerator')
->disableOriginalConstructor()
->setMethods(array('generateFromPath'))
->getMock();
$request_context = $this->getMockBuilder('Drupal\Core\Routing\RequestContext')
->disableOriginalConstructor()
->getMock();
$request_context->expects($this->any())
->method('getCompleteBaseUrl')
->willReturn('http://example.com/drupal');
if ($expected) {
$url_generator
->expects($this->any())
->method('generateFromPath')
->willReturnMap([
['test', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/drupal/test'],
['example.com', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/drupal/example.com'],
['example:com', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/drupal/example:com'],
['javascript:alert(0)', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/drupal/javascript:alert(0)'],
['/test', ['query' => [], 'fragment' => '', 'absolute' => TRUE], FALSE, 'http://example.com/test'],
]);
}
$listener = new RedirectResponseSubscriber($url_generator, $request_context);
$dispatcher->addListener(KernelEvents::RESPONSE, array($listener, 'checkRedirectUrl'));
$event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST, $response);
$dispatcher->dispatch(KernelEvents::RESPONSE, $event);
$this->assertEquals(400, $event->getResponse()->getStatusCode());
}
/**
* @covers ::checkRedirectUrl
*/
public function testRedirectWithOptInExternalUrl() {
$dispatcher = new EventDispatcher();
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
$response = new TrustedRedirectResponse('http://external-url.com');
$url_generator = $this->getMockBuilder('Drupal\Core\Routing\UrlGenerator')
->disableOriginalConstructor()
->setMethods(array('generateFromPath'))
->getMock();
$request_context = $this->getMockBuilder('Drupal\Core\Routing\RequestContext')
->disableOriginalConstructor()
->getMock();
$request_context->expects($this->any())
->method('getCompleteBaseUrl')
->willReturn('http://example.com/drupal');
$request = Request::create('');
$request->headers->set('HOST', 'example.com');
$listener = new RedirectResponseSubscriber($url_generator, $request_context);
$dispatcher->addListener(KernelEvents::RESPONSE, array($listener, 'checkRedirectUrl'));
$event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST, $response);
$dispatcher->dispatch(KernelEvents::RESPONSE, $event);
$target_url = $event->getResponse()->getTargetUrl();
$this->assertEquals('http://external-url.com', $target_url);
}
/**
* Data provider for testDestinationRedirectToExternalUrl().
*/
public function providerTestDestinationRedirectToExternalUrl() {
return [
'absolute external url' => [new Request(['destination' => 'http://example.com']), 'http://example.com'],
'absolute external url with folder' => [new Request(['destination' => 'http://example.com/foobar']), 'http://example.com/foobar'],
'absolute external url with folder2' => [new Request(['destination' => 'http://example.ca/drupal']), 'http://example.ca/drupal'],
'path without drupal basepath' => [new Request(['destination' => '/test']), 'http://example.com/test'],
'path with URL' => [new Request(['destination' => '/example.com']), 'http://example.com/example.com'],
'path with URL and two slashes' => [new Request(['destination' => '//example.com']), 'http://example.com//example.com'],
];
}
/**
* @expectedException \PHPUnit_Framework_Error
*
* @dataProvider providerTestDestinationRedirectWithInvalidUrl
*/
......@@ -119,6 +224,8 @@ public function testDestinationRedirectWithInvalidUrl(Request $request) {
$dispatcher->addListener(KernelEvents::RESPONSE, array($listener, 'checkRedirectUrl'));
$event = new FilterResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST, $response);
$dispatcher->dispatch(KernelEvents::RESPONSE, $event);
$this->assertEquals(400, $event->getResponse()->getStatusCode());
}
/**
......@@ -128,6 +235,11 @@ public function providerTestDestinationRedirectWithInvalidUrl() {
$data = [];
$data[] = [new Request(array('destination' => '//example:com'))];
$data[] = [new Request(array('destination' => '//example:com/test'))];
$data['absolute external url'] = [new Request(['destination' => 'http://example.com'])];
$data['absolute external url with folder'] = [new Request(['destination' => 'http://example.ca/drupal'])];
$data['path without drupal basepath'] = [new Request(['destination' => '/test'])];
$data['path with URL'] = [new Request(['destination' => '/example.com'])];
$data['path with URL and two slashes'] = [new Request(['destination' => '//example.com'])];
return $data;
}
......
<?php
/**
* @file
* Contains \Drupal\Tests\Core\Routing\TrustedRedirectResponseTest.
*/
namespace Drupal\Tests\Core\Routing;
use Drupal\Core\Routing\RequestContext;
use Drupal\Core\Routing\TrustedRedirectResponse;
use Drupal\Tests\UnitTestCase;
use Symfony\Component\DependencyInjection\ContainerBuilder;
/**
* @coversDefaultClass \Drupal\Core\Routing\TrustedRedirectResponse
* @group Routing
*/
class TrustedRedirectResponseTest extends UnitTestCase {
/**
* @covers ::setTargetUrl
*/
public function testSetTargetUrlWithInternalUrl() {
$redirect_response = new TrustedRedirectResponse('/example');
$redirect_response->setTargetUrl('/example2');
$this->assertEquals('/example2', $redirect_response->getTargetUrl());
}
/**
* @covers ::setTargetUrl
* @expectedException \InvalidArgumentException
*/
public function testSetTargetUrlWithUntrustedUrl() {
$request_context = new RequestContext();
$request_context->setCompleteBaseUrl('https://www.drupal.org');
$container = new ContainerBuilder();
$container->set('router.request_context', $request_context);
\Drupal::setContainer($container);
$redirect_response = new TrustedRedirectResponse('/example');
$redirect_response->setTargetUrl('http://evil-url.com/example');
}
/**
* @covers ::setTargetUrl
*/
public function testSetTargetUrlWithTrustedUrl() {
$redirect_response = new TrustedRedirectResponse('/example');
$redirect_response->setTrustedTargetUrl('http://good-external-url.com/example');
$this->assertEquals('http://good-external-url.com/example', $redirect_response->getTargetUrl());
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment