[SECURITY] t3lib_div::quoteJSvalue allows XSS
authorHelmut Hummel <helmut.hummel@typo3.org>
Wed, 15 Aug 2012 10:18:14 +0000 (12:18 +0200)
committerOliver Hader <oliver.hader@typo3.org>
Wed, 15 Aug 2012 10:18:17 +0000 (12:18 +0200)
When t3lib_div::quoteJSvalue() was used with second
parameter set to TRUE closing HTML script tags were
not escaped correctly.

Now every character except harmless ones is encoded
to a hex representation.

Change-Id: I98d752ca13abb8655eb1fc06c003d9228c61b952
Releases: 6.0, 4.7, 4.6, 4.5
Fixes: #23226
Security-Commit: 5df5647a9ed543de5451f4ab4baa6767218d89db
Security-Bulletin: TYPO3-CORE-SA-2012-004
Reviewed-on: http://review.typo3.org/13745
Reviewed-by: Oliver Hader
Tested-by: Oliver Hader
t3lib/class.t3lib_div.php
t3lib/codec/class.t3lib_codec_javascriptencoder.php [new file with mode: 0644]
t3lib/core_autoload.php
tests/t3lib/class.t3lib_divTest.php
tests/t3lib/codec/t3lib_codec_javascriptencoderTest.php [new file with mode: 0644]

index ba8ae2c..2b90b62 100644 (file)
@@ -5777,25 +5777,15 @@ final class t3lib_div {
 
 
        /**
-        * Quotes a string for usage as JS parameter. Depends whether the value is
-        * used in script tags (it doesn't need/must not get htmlspecialchar'ed in
-        * this case).
+        * Quotes a string for usage as JS parameter.
         *
         * @param string $value the string to encode, may be empty
-        * @param boolean $withinCData
-        *              whether the escaped data is expected to be used as CDATA and thus
-        *              does not need to be htmlspecialchared
         *
         * @return string the encoded value already quoted (with single quotes),
         *                              will not be empty
         */
-       static public function quoteJSvalue($value, $withinCData = FALSE) {
-               $escapedValue = addcslashes(
-                       $value, '\'' . '"' . '\\' . TAB . LF . CR
-               );
-               if (!$withinCData) {
-                       $escapedValue = htmlspecialchars($escapedValue);
-               }
+       public static function quoteJSvalue($value) {
+               $escapedValue = t3lib_div::makeInstance('t3lib_codec_JavaScriptEncoder')->encode($value);
                return '\'' . $escapedValue . '\'';
        }
 
diff --git a/t3lib/codec/class.t3lib_codec_javascriptencoder.php b/t3lib/codec/class.t3lib_codec_javascriptencoder.php
new file mode 100644 (file)
index 0000000..41a39a8
--- /dev/null
@@ -0,0 +1,189 @@
+<?php
+/***************************************************************
+ * Copyright notice
+ *
+ * (c) 2012 Franz G. Jahn <franzjahn@cron-it.de>
+ * (c) 2012 Helmut Hummel <helmut.hummel@typo3.org>
+ * 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!
+ ***************************************************************/
+
+/**
+ * Adopted from OWASP Enterprise Security API (ESAPI) reference implementation for the JavaScript Codec.
+ * Original Author: Mike Boberski
+ *
+ * This class provides encoding for user input that is intended to be used in a JavaScript context.
+ * It encodes all characters except alphanumericals and the immune characters to a hex representation.
+ *
+ * @package TYPO3
+ * @subpackage t3lib
+ *
+ * @author Mike Boberski <boberski_michael@bah.com>
+ * @copyright 2009-2010 The OWASP Foundation
+ * @link http://www.owasp.org/index.php/ESAPI
+ *
+ * @author Franz G. Jahn <franzjahn@cron-it.de>
+ * @author Helmut Hummel <helmut.hummel@typo3.org>
+ */
+class t3lib_codec_JavaScriptEncoder implements t3lib_Singleton {
+       /**
+        * A map where the keys are ordinal values of non-alphanumeric single-byte
+        * characters and the values are hexadecimal equivalents as strings.
+        *
+        * @var array
+        */
+       protected $hexMatrix = array();
+
+       /**
+        * Characters that are immune (not dangerous) in the JavaScript context
+        *
+        * @var array
+        */
+       protected $immuneCharacters = array(',', '.', '_' );
+
+       /**
+        * Encoding that is used in the current context
+        *
+        * @var string
+        */
+       protected $encoding;
+
+       /**
+        * TYPO3 charset encoding object
+        *
+        * @var t3lib_cs
+        */
+       protected $charsetConversion = NULL;
+
+       /**
+        * Populates the $hex map of non-alphanumeric single-byte characters.
+        *
+        * Alphanumerical character are set to NULL in the matrix.
+        */
+       public function __construct() {
+               $this->charsetConversion = t3lib_div::makeInstance('t3lib_cs');
+               $this->encoding = $this->getEncoding();
+
+               for ($i = 0; $i < 256; $i++) {
+                       if (($i >= ord('0') && $i <= ord('9')) || ($i >= ord('A') && $i <= ord('Z')) || ($i >= ord('a') && $i <= ord('z'))) {
+                               $this->hexMatrix[$i] = NULL;
+                       } else {
+                               $this->hexMatrix[$i] = dechex($i);
+                       }
+               }
+       }
+
+       /**
+        * Encodes a string for JavaScript.
+        *
+        * @param string $input The string to encode, may be empty.
+        * @return string The encoded string.
+        */
+       public function encode($input) {
+               $normalizedInput = $this->charsetConversion->conv($input, $this->encoding, 'utf-8');
+               $stringLength = $this->charsetConversion->strlen('utf-8', $normalizedInput);
+               $encodedString = '';
+               for ($i = 0; $i < $stringLength; $i++) {
+                       $c = $this->charsetConversion->substr('utf-8', $normalizedInput, $i, 1);
+                       $encodedString .= $this->encodeCharacter($c);
+               }
+
+               return $encodedString;
+       }
+
+       /**
+        * Returns backslash encoded numeric format. Does not use backslash
+        * character escapes such as, \" or \' as these may cause parsing problems.
+        * For example, if a javascript attribute, such as onmouseover, contains
+        * a \" that will close the entire attribute and allow an attacker to inject
+        * another script attribute.
+        *
+        * @param string $character utf-8 character that needs to be encoded
+        * @return string encoded character
+        */
+       protected function encodeCharacter($character) {
+               if ($this->isImmuneCharacter($character)) {
+                       return $character;
+               }
+
+               $ordinalValue = $this->charsetConversion->utf8CharToUnumber($character);
+
+                       // Check for alphanumeric characters
+               $hex = $this->getHexForNonAlphanumeric($ordinalValue);
+               if ($hex === NULL) {
+                       return $character;
+               }
+
+                       // Encode up to 256 with \\xHH
+               if ($ordinalValue < 256) {
+                       $pad = substr('00', strlen($hex));
+                       return '\\x' . $pad . strtoupper($hex);
+               }
+
+                       // Otherwise encode with \\uHHHH
+               $pad = substr('0000', strlen($hex));
+               return '\\u' . $pad . strtoupper($hex);
+       }
+
+       /**
+        * Checks if the given character is one of the immune characters
+        *
+        * @param string $character utf-8 character to search for, must not be empty
+        * @return boolean TRUE if character is immune, FALSE otherwise
+        */
+       protected function isImmuneCharacter($character) {
+               return in_array($character, $this->immuneCharacters, TRUE);
+       }
+
+       /**
+        * Returns the ordinal value as a hex string of any character that is not a
+        * single-byte alphanumeric. The character should be supplied as a string in
+        * the utf-8 character encoding.
+        * If the character is an alphanumeric character with ordinal value below 255,
+        * then this method will return NULL.
+        *
+        * @param integer $ordinalValue Ordinal value of the character
+        * @return string hexadecimal ordinal value of non-alphanumeric characters or NULL otherwise.
+        */
+       protected function getHexForNonAlphanumeric($ordinalValue) {
+               if ($ordinalValue <= 255) {
+                       return $this->hexMatrix[$ordinalValue];
+               }
+               return dechex($ordinalValue);
+       }
+
+    /**
+        * Gets the encoding depending on the current context (TYPO3_MODE)
+        *
+        * @return string
+        */
+       protected function getEncoding() {
+               if (TYPO3_MODE == 'FE') {
+                       $charset = $GLOBALS['TSFE']->renderCharset;
+               } elseif (is_object($GLOBALS['LANG'])) {
+                       $charset = $GLOBALS['LANG']->charSet;
+               } else if (!empty($GLOBALS['TYPO3_CONF_VARS']['BE']['forceCharset'])) {
+                       $charset = $GLOBALS['TYPO3_CONF_VARS']['BE']['forceCharset'];
+               } else {
+                       $charset = 'utf-8';
+               }
+
+               return $charset;
+       }
+}
+?>
\ No newline at end of file
index f08ef33..73416fe 100644 (file)
@@ -17,6 +17,7 @@ $t3libClasses = array(
        't3lib_cli' => PATH_t3lib . 'class.t3lib_cli.php',
        't3lib_clipboard' => PATH_t3lib . 'class.t3lib_clipboard.php',
        't3lib_compressor' => PATH_t3lib . 'class.t3lib_compressor.php',
+       't3lib_codec_javascriptencoder' => PATH_t3lib . 'codec/class.t3lib_codec_javascriptencoder.php',
        't3lib_cs' => PATH_t3lib . 'class.t3lib_cs.php',
        't3lib_db' => PATH_t3lib . 'class.t3lib_db.php',
        't3lib_db_preparedstatement' => PATH_t3lib . 'db/class.t3lib_db_preparedstatement.php',
index 490e87b..c4a741c 100644 (file)
@@ -1879,124 +1879,66 @@ class t3lib_divTest extends tx_phpunit_testcase {
        //////////////////////////////////
 
        /**
-        * @test
-        */
-       public function quoteJSvalueHtmlspecialcharsDataByDefault() {
-               $this->assertContains(
-                       '&gt;',
-                       t3lib_div::quoteJSvalue('>')
-               );
-       }
-
-       /**
-        * @test
-        */
-       public function quoteJSvaluetHtmlspecialcharsDataWithinCDataSetToFalse() {
-               $this->assertContains(
-                       '&gt;',
-                       t3lib_div::quoteJSvalue('>', FALSE)
-               );
-       }
-
-       /**
-        * @test
-        */
-       public function quoteJSvaluetNotHtmlspecialcharsDataWithinCDataSetToTrue() {
-               $this->assertContains(
-                       '>',
-                       t3lib_div::quoteJSvalue('>', TRUE)
-               );
-       }
-
-       /**
-        * @test
-        */
-       public function quoteJSvalueReturnsEmptyStringQuotedInSingleQuotes() {
-               $this->assertEquals(
-                       "''",
-                       t3lib_div::quoteJSvalue("", TRUE)
-               );
-       }
-
-       /**
-        * @test
-        */
-       public function quoteJSvalueNotModifiesStringWithoutSpecialCharacters() {
-               $this->assertEquals(
-                       "'Hello world!'",
-                       t3lib_div::quoteJSvalue("Hello world!", TRUE)
-               );
-       }
-
-       /**
-        * @test
-        */
-       public function quoteJSvalueEscapesSingleQuote() {
-               $this->assertEquals(
-                       "'\\''",
-                       t3lib_div::quoteJSvalue("'", TRUE)
-               );
-       }
-
-       /**
-        * @test
-        */
-       public function quoteJSvalueEscapesDoubleQuoteWithinCDataSetToTrue() {
-               $this->assertEquals(
-                       "'\\\"'",
-                       t3lib_div::quoteJSvalue('"', TRUE)
-               );
-       }
-
-       /**
-        * @test
-        */
-       public function quoteJSvalueEscapesAndHtmlspecialcharsDoubleQuoteWithinCDataSetToFalse() {
-               $this->assertEquals(
-                       "'\\&quot;'",
-                       t3lib_div::quoteJSvalue('"', FALSE)
-               );
-       }
-
-       /**
-        * @test
-        */
-       public function quoteJSvalueEscapesTab() {
-               $this->assertEquals(
-                       "'" . '\t' . "'",
-                       t3lib_div::quoteJSvalue(TAB)
-               );
-       }
-
-       /**
-        * @test
+        * Data provider for quoteJSvalueTest.
+        *
+        * @return array
         */
-       public function quoteJSvalueEscapesLinefeed() {
-               $this->assertEquals(
-                       "'" . '\n' . "'",
-                       t3lib_div::quoteJSvalue(LF)
+       public function quoteJsValueDataProvider() {
+               return array(
+                       'Immune characters are returned as is' => array(
+                               '._,',
+                               '._,',
+                       ),
+                       'Alphanumerical characters are returned as is' => array(
+                               'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789',
+                               'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789',
+                       ),
+                       'Angel brackets and ampersand are encoded' => array(
+                               '<>&',
+                               '\x3C\x3E\x26',
+                       ),
+                       'Quotes and slashes are encoded' => array(
+                               '"\'\\/',
+                               '\x22\x27\x5C\x2F',
+                       ),
+                       'Empty string stays empty' => array(
+                               '',
+                               '',
+                       ),
+                       'Exclamation mark and space are properly encoded' => array(
+                               'Hello World!',
+                               'Hello\x20World\x21',
+                       ),
+                       'Whitespaces are properly encoded' => array(
+                               TAB . LF . CR . ' ',
+                               '\x09\x0A\x0D\x20',
+                       ),
+                       'Null byte is properly encoded' => array(
+                               chr(0),
+                               '\x00',
+                       ),
+                       'Umlauts are properly encoded' => array(
+                               'ÜüÖöÄä',
+                               '\xDC\xFC\xD6\xF6\xC4\xE4',
+                       ),
                );
        }
 
        /**
         * @test
+        *
+        * @param string $input
+        * @param string $expected
+        *
+        * @dataProvider quoteJsValueDataProvider
         */
-       public function quoteJSvalueEscapesCarriageReturn() {
-               $this->assertEquals(
-                       "'" . '\r' . "'",
-                       t3lib_div::quoteJSvalue(CR)
+       public function quoteJsValueTest($input, $expected) {
+               $this->assertSame(
+                       '\'' . $expected . '\'',
+                       t3lib_div::quoteJSvalue($input)
                );
        }
 
-       /**
-        * @test
-        */
-       public function quoteJSvalueEscapesBackslah() {
-               $this->assertEquals(
-                       "'\\\\'",
-                       t3lib_div::quoteJSvalue('\\')
-               );
-       }
 
        //////////////////////////////////
        // Tests concerning readLLfile
diff --git a/tests/t3lib/codec/t3lib_codec_javascriptencoderTest.php b/tests/t3lib/codec/t3lib_codec_javascriptencoderTest.php
new file mode 100644 (file)
index 0000000..cd01a4a
--- /dev/null
@@ -0,0 +1,108 @@
+<?php
+/***************************************************************
+ * Copyright notice
+ *
+ * (c) 2012 Helmut Hummel <helmut.hummel@typo3.org>
+ * 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!
+ ***************************************************************/
+
+/**
+ * Test cases for t3lib_codec_JavaScriptEncoder.
+ *
+ * @package TYPO3
+ * @subpackage t3lib
+ *
+ * @author Helmut Hummel <helmut.hummel@typo3.org>
+ */
+class t3lib_codec_JavaScriptEncoderTest extends Tx_Phpunit_TestCase {
+       /**
+        * @var t3lib_codec_JavaScriptEncoder
+        */
+       protected $fixture = NULL;
+
+       public function setUp() {
+               $this->fixture = new t3lib_codec_JavaScriptEncoder();
+       }
+
+       public function tearDown() {
+               unset($this->fixture);
+       }
+
+       /**
+        * Data provider for encodeEncodesCorrectly.
+        *
+        * @return array
+        */
+       public function encodeEncodesCorrectlyDataProvider() {
+               return array(
+                       'Immune characters are returned as is' => array(
+                               '._,',
+                               '._,'
+                       ),
+                       'Alphanumerical characters are returned as is' => array(
+                               'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789',
+                               'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789',
+                       ),
+                       'Angel brackets and ampersand are encoded' => array(
+                               '<>&',
+                               '\x3C\x3E\x26',
+                       ),
+                       'Quotes and slashes are encoded' => array(
+                               '"\'\\/',
+                               '\x22\x27\x5C\x2F',
+                       ),
+                       'Empty string stays empty' => array(
+                               '',
+                               '',
+                       ),
+                       'Exclamation mark and space are properly encoded' => array(
+                               'Hello World!',
+                               'Hello\x20World\x21',
+                       ),
+                       'Whitespaces are properly encoded' => array(
+                               TAB . LF . CR . ' ',
+                               '\x09\x0A\x0D\x20',
+                       ),
+                       'Null byte is properly encoded' => array(
+                               chr(0),
+                               '\x00',
+                       ),
+                       'Umlauts are properly encoded' => array(
+                               'ÜüÖöÄä',
+                               '\xDC\xFC\xD6\xF6\xC4\xE4',
+                       ),
+               );
+       }
+
+       /**
+        * @test
+        *
+        * @param string $input
+        * @param string  $expected
+        *
+        * @dataProvider encodeEncodesCorrectlyDataProvider
+        */
+       public function encodeEncodesCorrectly($input, $expected) {
+               $this->assertSame(
+                       $expected,
+                       $this->fixture->encode($input)
+               );
+       }
+}
+?>
\ No newline at end of file