[SECURITY] XSS in exception handler
authorOliver Klee <typo3-coding@oliverklee.de>
Tue, 17 Apr 2012 09:10:46 +0000 (11:10 +0200)
committerOliver Hader <oliver.hader@typo3.org>
Tue, 17 Apr 2012 09:10:53 +0000 (11:10 +0200)
Change-Id: Id4c3a2cfeb95f4dbf20a2e50b052b2ab75032211
Releases: 6.0, 4.7, 4.6, 4.5, 4.4
Fixes: #34348
Security-Review: http://review.typo3.org/10312
Security-Commit: 6b2b6590356a4aca7d8cc5dfbdfa3356edee091d
Security-Bulletin: TYPO3-CORE-SA-2012-002
Reviewed-on: http://review.typo3.org/10566
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
t3lib/error/class.t3lib_error_debugexceptionhandler.php
t3lib/error/class.t3lib_error_productionexceptionhandler.php
tests/t3lib/error/class.t3lib_error_debugexceptionhandlerTest.php [new file with mode: 0644]
tests/t3lib/error/class.t3lib_error_productionexceptionhandlerTest.php [new file with mode: 0644]

index 419d244..ba14f45 100644 (file)
@@ -121,11 +121,11 @@ class t3lib_error_DebugExceptionHandler extends t3lib_error_AbstractExceptionHan
                                                ">
                                                <div style="width: 100%; background-color: #515151; color: white; padding: 2px; margin: 0 0 6px 0;">Uncaught TYPO3 Exception</div>
                                                <div style="width: 100%; padding: 2px; margin: 0 0 6px 0;">
-                                                       <strong style="color: #BE0027;">' . $exceptionCodeNumber . $exception->getMessage() . '</strong> ' . /* $moreInformationLink .*/
+                                                       <strong style="color: #BE0027;">' . $exceptionCodeNumber . htmlspecialchars($exception->getMessage()) . '</strong> ' . /* $moreInformationLink .*/
                         '<br />
                                                        <br />
                                                        <span class="ExceptionProperty">' . get_class($exception) . '</span> thrown in file<br />
-                                                       <span class="ExceptionProperty">' . $filePathAndName . '</span> in line
+                                                       <span class="ExceptionProperty">' . htmlspecialchars($filePathAndName) . '</span> in line
                                                        <span class="ExceptionProperty">' . $exception->getLine() . '</span>.<br />
                                                        <br />
                                                        ' . $backtraceCode . '
index 0cd3787..c8d0f40 100644 (file)
@@ -55,7 +55,7 @@ class t3lib_error_ProductionExceptionHandler extends t3lib_error_AbstractExcepti
                }
                $this->writeLogEntries($exception, self::CONTEXT_WEB);
                        // we use a nice-looking title for our visitors instead of the exception's class name
-               $messageObj = t3lib_div::makeInstance('t3lib_message_ErrorPageMessage', $exception->getMessage(), 'Oops, an error occured!');
+               $messageObj = t3lib_div::makeInstance('t3lib_message_ErrorPageMessage', htmlspecialchars($exception->getMessage()), 'Oops, an error occured!');
                $messageObj->output();
        }
 
diff --git a/tests/t3lib/error/class.t3lib_error_debugexceptionhandlerTest.php b/tests/t3lib/error/class.t3lib_error_debugexceptionhandlerTest.php
new file mode 100644 (file)
index 0000000..8a71ed1
--- /dev/null
@@ -0,0 +1,79 @@
+<?php
+/***************************************************************
+ *  Copyright notice
+ *
+ *  (c) 2012 Oliver Klee <typo3-coding@oliverklee.de>
+ *  All rights reserved
+ *
+ *  This script is part of the TYPO3 project. The TYPO3 project is
+ *  free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  The GNU General Public License can be found at
+ *  http://www.gnu.org/copyleft/gpl.html.
+ *
+ *  This script is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  This copyright notice MUST APPEAR in all copies of the script!
+ ***************************************************************/
+
+/**
+ * testcase for the t3lib_error_DebugExceptionHandler class.
+ *
+ * @author Oliver Klee <typo3-coding@oliverklee.de>
+ * @package TYPO3
+ * @subpackage t3lib_error
+ */
+class t3lib_error_DebugExceptionHandlerTest extends Tx_Phpunit_TestCase {
+       /**
+        * @var t3lib_error_DebugExceptionHandler|PHPUnit_Framework_MockObject_MockObject
+        */
+       private $fixture = NULL;
+
+       /**
+        * Sets up this test case.
+        */
+       protected function setUp() {
+               $this->fixture = $this->getMock(
+                       't3lib_error_DebugExceptionHandler',
+                       array('sendStatusHeaders', 'writeLogEntries'),
+                       array(), '', FALSE
+               );
+               $this->fixture->expects($this->any())->method('discloseExceptionInformation')->will($this->returnValue(TRUE));
+       }
+
+       /**
+        * Tears down this test case.
+        */
+       protected function tearDown() {
+               unset($this->fixture);
+       }
+
+       /**
+        * @test
+        */
+       public function echoExceptionWebEscapesExceptionMessage() {
+               $message = '<b>b</b><script>alert(1);</script>';
+               $exception = new Exception($message);
+
+               ob_start();
+               $this->fixture->echoExceptionWeb($exception);
+               $output = ob_get_contents();
+               ob_end_clean();
+
+               $this->assertContains(
+                       htmlspecialchars($message),
+                       $output
+               );
+               $this->assertNotContains(
+                       $message,
+                       $output
+               );
+       }
+}
+?>
\ No newline at end of file
diff --git a/tests/t3lib/error/class.t3lib_error_productionexceptionhandlerTest.php b/tests/t3lib/error/class.t3lib_error_productionexceptionhandlerTest.php
new file mode 100644 (file)
index 0000000..8ead32d
--- /dev/null
@@ -0,0 +1,99 @@
+<?php
+/***************************************************************
+ *  Copyright notice
+ *
+ *  (c) 2012 Oliver Klee <typo3-coding@oliverklee.de>
+ *  All rights reserved
+ *
+ *  This script is part of the TYPO3 project. The TYPO3 project is
+ *  free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  The GNU General Public License can be found at
+ *  http://www.gnu.org/copyleft/gpl.html.
+ *
+ *  This script is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  This copyright notice MUST APPEAR in all copies of the script!
+ ***************************************************************/
+
+/**
+ * testcase for the t3lib_error_ProductionExceptionHandler class.
+ *
+ * @author Oliver Klee <typo3-coding@oliverklee.de>
+ * @package TYPO3
+ * @subpackage t3lib_error
+ */
+class t3lib_error_ProductionExceptionHandlerTest extends Tx_Phpunit_TestCase {
+       /**
+        * @var t3lib_error_ProductionExceptionHandler|PHPUnit_Framework_MockObject_MockObject
+        */
+       private $fixture = NULL;
+
+       /**
+        * Sets up this test case.
+        */
+       protected function setUp() {
+               $this->fixture = $this->getMock(
+                       't3lib_error_ProductionExceptionHandler',
+                       array('discloseExceptionInformation', 'sendStatusHeaders', 'writeLogEntries'),
+                       array(), '', FALSE
+               );
+               $this->fixture->expects($this->any())->method('discloseExceptionInformation')->will($this->returnValue(TRUE));
+       }
+
+       /**
+        * Tears down this test case.
+        */
+       protected function tearDown() {
+               unset($this->fixture);
+       }
+
+       /**
+        * @test
+        */
+       public function echoExceptionWebEscapesExceptionMessage() {
+               $message = '<b>b</b><script>alert(1);</script>';
+               $exception = new Exception($message);
+
+               ob_start();
+               $this->fixture->echoExceptionWeb($exception);
+               $output = ob_get_contents();
+               ob_end_clean();
+
+               $this->assertContains(
+                       htmlspecialchars($message),
+                       $output
+               );
+               $this->assertNotContains(
+                       $message,
+                       $output
+               );
+       }
+
+       /**
+        * @test
+        */
+       public function echoExceptionWebEscapesExceptionTitle() {
+               $title = '<b>b</b><script>alert(1);</script>';
+               /** @var $exception Exception|PHPUnit_Framework_MockObject_MockObject */
+               $exception = $this->getMock('Exception', array('getTitle'), array('some message'));
+               $exception->expects($this->any())->method('getTitle')->will($this->returnValue($title));
+
+               ob_start();
+               $this->fixture->echoExceptionWeb($exception);
+               $output = ob_get_contents();
+               ob_end_clean();
+
+               $this->assertNotContains(
+                       $title,
+                       $output
+               );
+       }
+}
+?>
\ No newline at end of file