[SECURITY] XSS possibility in RemoveXSS
authorAndreas Wolf <andreas.wolf@ikt-werk.de>
Wed, 28 Mar 2012 11:54:19 +0000 (13:54 +0200)
committerOliver Hader <oliver@typo3.org>
Wed, 28 Mar 2012 11:54:21 +0000 (13:54 +0200)
RemoveXSS fails to properly remove non printable characters, especially
zero-byte (\x00) chars.

Change-Id: If1caf9bda7338bd47203b55e27c5a99bbdfed3b0
Fixes: #30188
Security-Commit: 1ff7a55aefd3d4c1690e3f35760ea5ef30dab9b2
Security-Bulletin: TYPO3-CORE-SA-2012-001
Reviewed-on: http://review.typo3.org/10006
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
tests/contrib/removexssTest.php
typo3/contrib/RemoveXSS/RemoveXSS.php

index 7932607..083dce2 100644 (file)
@@ -389,6 +389,14 @@ class RemoveXSSTest extends tx_phpunit_testcase {
                                '<a href="j&#6&#53;;vascript:alert(123);">click</a>',
                                '<a href="ja<x>vascript:alert(123);">click</a>',
                        ),
+                       'attack with null character' => array(
+                               '<scr' . chr(0) . 'ipt></script>',
+                               '<sc<x>ript></script>'
+                       ),
+                       'attack with null character in attribute' => array(
+                               '<a href="j' . chr(0) . 'avascript:alert(123);"></a>',
+                               '<a href="ja<x>vascript:alert(123);"></a>'
+                       ),
                );
        }
 
@@ -406,5 +414,37 @@ class RemoveXSSTest extends tx_phpunit_testcase {
                        RemoveXSS::process($input)
                );
        }
+
+       /**
+        * Allowed combinations
+        */
+       public function processValidDataProvider() {
+               return array(
+                       'multibyte characters' => array(
+                               '<img®€ÜüÖöÄä></img>',
+                       ),
+                       'tab' => array(
+                               '<im' . chr(9) . 'g></img>',
+                       ),
+                       'line feed' => array(
+                               '<im' . chr(10) . 'g></img>',
+                       ),
+                       'carriage return' => array(
+                               '<im' . chr(13) . 'g></img>',
+                       ),
+               );
+       }
+
+       /**
+        * @test
+        * @param string $input Value to test
+        * @dataProvider processValidDataProvider
+        */
+       public function proccessValidStrings($input) {
+               $this->assertEquals(
+                       $input,
+                       RemoveXSS::process($input)
+               );
+       }
 }
 ?>
\ No newline at end of file
index fb3677c..ce12053 100644 (file)
@@ -57,7 +57,7 @@ final class RemoveXSS {
                // remove all non-printable characters. CR(0a) and LF(0b) and TAB(9) are allowed
                // this prevents some character re-spacing such as <java\0script>
                // note that you have to handle splits with \n, \r, and \t later since they *are* allowed in some inputs
-               $val = preg_replace('/([\x00-\x08][\x0b-\x0c][\x0e-\x19])/', '', $val);
+               $val = preg_replace('/([\x00-\x08]|[\x0b-\x0c]|[\x0e-\x19])/', '', $val);
 
                // straight replacements, the user should never need these since they're normal characters
                // this prevents like <IMG SRC=&#X40&#X61&#X76&#X61&#X73&#X63&#X72&#X69&#X70&#X74&#X3A&#X61&#X6C&#X65&#X72&#X74&#X28&#X27&#X58&#X53&#X53&#X27&#X29>