Commit 55b85580 authored by catch's avatar catch

Issue #2371141 by pfrenssen: Fixed XSS vulnerability when displaying exception backtrace.

parent 7473eaf4
......@@ -139,8 +139,9 @@ protected function onHtml(GetResponseForExceptionEvent $event) {
// once more in the backtrace.
array_shift($backtrace);
// Generate a backtrace containing only scalar argument values.
$message .= '<pre class="backtrace">' . Error::formatFlattenedBacktrace($backtrace) . '</pre>';
// Generate a backtrace containing only scalar argument values. Make
// sure the backtrace is escaped as it can contain user submitted data.
$message .= '<pre class="backtrace">' . SafeMarkup::escape(Error::formatFlattenedBacktrace($backtrace)) . '</pre>';
}
drupal_set_message(SafeMarkup::set($message), $class, TRUE);
}
......
......@@ -196,7 +196,8 @@ public static function formatBacktrace(array $backtrace) {
* The backtrace of a Symfony\Component\Debug\Exception\FlattenException.
*
* @return string
* A plain-text line-wrapped string ready to be put inside <pre>.
* A plain-text line-wrapped string. The string needs to be run through
* SafeMarkup::escape when rendering it as HTML.
*/
public static function formatFlattenedBacktrace(array $backtrace) {
$return = '';
......
......@@ -7,6 +7,7 @@
namespace Drupal\system\Tests\Routing;
use Drupal\Component\Utility\String;
use Drupal\simpletest\KernelTestBase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
......@@ -99,5 +100,27 @@ public function testHtml404() {
$this->assertEqual($response->headers->get('Content-type'), 'text/html; charset=UTF-8');
}
}
/**
* Tests if exception backtraces are properly escaped when output to HTML.
*/
public function testBacktraceEscaping() {
// Enable verbose error logging.
\Drupal::config('system.logging')->set('error_level', ERROR_REPORTING_DISPLAY_VERBOSE)->save();
$request = Request::create('/router_test/test17');
$request->headers->set('Accept', 'text/html');
$request->setFormat('html', ['text/html']);
/** @var \Symfony\Component\HttpKernel\HttpKernelInterface $kernel */
$kernel = \Drupal::getContainer()->get('http_kernel');
$response = $kernel->handle($request)->prepare($request);
$this->assertEqual($response->getStatusCode(), Response::HTTP_INTERNAL_SERVER_ERROR);
$this->assertEqual($response->headers->get('Content-type'), 'text/html; charset=UTF-8');
// Test both that the backtrace is properly escaped, and that the unescaped
// string is not output at all.
$this->assertTrue(strpos($response->getContent(), String::checkPlain('<script>alert(\'xss\')</script>')) !== FALSE);
$this->assertTrue(strpos($response->getContent(), '<script>alert(\'xss\')</script>') === FALSE);
}
}
......@@ -107,6 +107,13 @@ router_test.16:
requirements:
_permission: 'administer users'
router_test.17:
path: '/router_test/test17'
defaults:
_controller: '\Drupal\router_test\TestControllers::test10'
requirements:
_access: 'TRUE'
router_test.hierarchy_parent:
path: '/menu-test/parent'
defaults:
......
......@@ -66,4 +66,35 @@ public function test9($uid) {
return new Response($text);
}
/**
* Test controller for ExceptionHandlingTest::testBacktraceEscaping().
*
* Passes unsafe HTML as an argument to a method which throws an exception.
* This can be used to test if the generated backtrace is properly escaped.
*/
public function test10() {
// Remove the exception logger from the event dispatcher. We are going to
// throw an exception to check if it is properly escaped when rendered as a
// backtrace. The exception logger does a call to error_log() which is not
// handled by the Simpletest error handler and would cause a test failure.
$event_dispatcher = \Drupal::service('event_dispatcher');
$exception_logger = \Drupal::service('exception.logger');
$event_dispatcher->removeSubscriber($exception_logger);
$this->throwException('<script>alert(\'xss\')</script>');
}
/**
* Throws an exception.
*
* @param string $message
* The message to use in the exception.
*
* @throws \Exception
* Always thrown.
*/
protected function throwException($message) {
throw new \Exception($message);
}
}
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