CVE-2009-3633

@@ -1,6 +1,7 @@
 2009-10-22  Ernesto Baschny <ernst@cron-it.de>
 
 	* Fixed bug #11586: Potential SQL injection in frontend editing (thanks to Oliver Klee)
+	* Fixed bug #12303: XSS vulnerability due to not proper sanitizing in function t3lib_div::quoteJSvalue (thanks to Oliver Klee)
 
 2009-10-21  Rupert Germann  <rupi@gmx.li>
 

@@ -4693,23 +4693,29 @@ function unQuoteFilenames($parameters,$unQuote=FALSE)	{
 		return $paramsArr;
 	}
 
-
 	/**
-	 * Quotes a string for usage as JS parameter. Depends wheter 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. Depends whether the value is
+	 * used in script tags (it doesn't need/must not get htmlspecialchar'ed in
+	 * this case).
+	 *
+	 * @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
 	 *
-	 * @param	string		The string to encode.
-	 * @param	boolean		If the values get's used in <script> tags.
-	 * @return	string		The encoded value already quoted
+	 * @access public
 	 */
-	function quoteJSvalue($value, $inScriptTags = false)	{
-		$value = addcslashes($value, '\''.'"'.chr(10).chr(13));
-		if (!$inScriptTags)	{
-			$value = htmlspecialchars($value);
+	function quoteJSvalue($value, $withinCData = false)	{
+		$escapedValue = addcslashes(
+			$value, '\'' . '"' . '\\' . chr(9) . chr(10) . chr(13)
+		);
+		if (!$withinCData) {
+			$escapedValue = htmlspecialchars($escapedValue);
 		}
-		return '\''.$value.'\'';
+		return '\'' . $escapedValue . '\'';
 	}
-
-
 }
-
-?>
+?>
\ No newline at end of file

@@ -2,6 +2,7 @@
 
 	* Security Issue #11664: Updated RemoveXSS code to the latest knowledge in this area (thanks to Jigal van Hemert)
 	* Fixed bug #11586: Potential SQL injection in frontend editing (thanks to Oliver Klee)
+	* Fixed bug #12303: XSS vulnerability due to not proper sanitizing in function t3lib_div::quoteJSvalue (thanks to Oliver Klee)
 
 2009-10-21  Rupert Germann  <rupi@gmx.li>
 

@@ -1534,8 +1534,8 @@ public static function generateRandomBytes($count) {
 		return $output;
 	}
 
- 
-	
+
+
 
 
 
@@ -5207,21 +5207,27 @@ public static function unQuoteFilenames($parameters,$unQuote=FALSE)	{
 		return $paramsArr;
 	}
 
-
 	/**
-	 * Quotes a string for usage as JS parameter. Depends wheter 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. Depends whether the value is
+	 * used in script tags (it doesn't need/must not get htmlspecialchar'ed in
+	 * this case).
 	 *
-	 * @param	string		The string to encode.
-	 * @param	boolean		If the values get's used in <script> tags.
-	 * @return	string		The encoded value already quoted
+	 * @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
 	 */
-	public static function quoteJSvalue($value, $inScriptTags = false)	{
-		$value = addcslashes($value, '\''.'"'.chr(10).chr(13));
-		if (!$inScriptTags)	{
-			$value = htmlspecialchars($value);
+	public static function quoteJSvalue($value, $withinCData = false)	{
+		$escapedValue = addcslashes(
+			$value, '\'' . '"' . '\\' . chr(9) . chr(10) . chr(13)
+		);
+		if (!$withinCData) {
+			$escapedValue = htmlspecialchars($escapedValue);
 		}
-		return '\''.$value.'\'';
+		return '\'' . $escapedValue . '\'';
 	}
 }
-
-?>
+?>
\ No newline at end of file

@@ -1,6 +1,7 @@
 2009-10-22  Ernesto Baschny <ernst@cron-it.de>
 
 	* Fixed bug #11586: Potential SQL injection in frontend editing (thanks to Oliver Klee)
+	* Fixed bug #12303: XSS vulnerability due to not proper sanitizing in function t3lib_div::quoteJSvalue (thanks to Oliver Klee)
 
 2009-10-21  Sebastian Kurfuerst  <sebastian@typo3.org>
 

@@ -5679,18 +5679,26 @@ public static function unQuoteFilenames($parameters,$unQuote=FALSE)	{
 
 
 	/**
-	 * Quotes a string for usage as JS parameter. Depends wheter the value is used in script tags (it doesn't need/must not get htmlspecialchar'ed in this case)
-	 *
-	 * @param	string		The string to encode.
-	 * @param	boolean		If the values get's used in <script> tags.
-	 * @return	string		The encoded value already quoted
-	 */
-	public static function quoteJSvalue($value, $inScriptTags = false)	{
-		$value = addcslashes($value, '\''.'"'.chr(10).chr(13));
-		if (!$inScriptTags) {
-			$value = htmlspecialchars($value);
-		}
-		return '\''.$value.'\'';
+	 * 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).
+	 *
+	 * @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, '\'' . '"' . '\\' . chr(9) . chr(10) . chr(13)
+		);
+		if (!$withinCData) {
+			$escapedValue = htmlspecialchars($escapedValue);
+		}
+		return '\'' . $escapedValue . '\'';
 	}
 
 

@@ -26,12 +26,13 @@
 /**
  * Testcase for class t3lib_div
  *
- * @author	Ingo Renner <ingo@typo3.org>
+ * @author Ingo Renner <ingo@typo3.org>
+ * @author Oliver Klee <typo3-coding@oliverklee.de>
+ *
  * @package TYPO3
  * @subpackage t3lib
  */
 class t3lib_div_testcase extends tx_phpunit_testcase {
-
 	/**
 	 * @test
 	 */
@@ -454,6 +455,131 @@ public function checkGetDirsReturnsStringErrorOnPathFailure() {
 
 		$this->assertEquals($expectedResult, $result);
 	}
+
+
+	//////////////////////////////////
+	// Tests concerning quoteJSvalue
+	//////////////////////////////////
+
+	/**
+	 * @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(chr(9))
+		);
+	}
+
+	/**
+	 * @test
+	 */
+	public function quoteJSvalueEscapesLinefeed() {
+		$this->assertEquals(
+			"'" . '\n' . "'",
+			t3lib_div::quoteJSvalue(chr(10))
+		);
+	}
+
+	/**
+	 * @test
+	 */
+	public function quoteJSvalueEscapesCarriageReturn() {
+		$this->assertEquals(
+			"'" . '\r' . "'",
+			t3lib_div::quoteJSvalue(chr(13))
+		);
+	}
+
+	/**
+	 * @test
+	 */
+	public function quoteJSvalueEscapesBackslah() {
+		$this->assertEquals(
+			"'\\\\'",
+			t3lib_div::quoteJSvalue('\\')
+		);
+	}
 }
 
 ?>
\ No newline at end of file