From e3332b0ab5dfcc42994fe4c2c1827f4e41f35c7b Mon Sep 17 00:00:00 2001
From: Derek Jones <derek.jones@ellislab.com>
Date: Tue, 13 May 2008 14:44:32 +0000
Subject: increased security and performance of xss_clean(), added
 _sanitize_naughty_html() callback and removed "never allowed" items to a
 class property

---
 system/libraries/Input.php | 80 ++++++++++++++++++++++++++++++++--------------
 user_guide/changelog.html  |  3 +-
 2 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/system/libraries/Input.php b/system/libraries/Input.php
index 978d1ff34..c86a3cec0 100644
--- a/system/libraries/Input.php
+++ b/system/libraries/Input.php
@@ -32,7 +32,26 @@ class CI_Input {
 	var $ip_address			= FALSE;
 	var $user_agent			= FALSE;
 	var $allow_get_array	= FALSE;
-
+	
+	/* never allowed, string replacement */
+	var $never_allowed_str = array(
+									'document.cookie'	=> '[removed]',
+									'document.write'	=> '[removed]',
+									'.parentNode'		=> '[removed]',
+									'.innerHTML'		=> '[removed]',
+									'window.location'	=> '[removed]',
+									'-moz-binding'		=> '[removed]',
+									'<!--'				=> '&lt;!--',
+									'-->'				=> '--&gt;',
+									'<![CDATA['			=> '&lt;![CDATA['
+									);
+	/* never allowed, regex replacement */
+	var $never_allowed_regex = array(
+										"javascript\s*:"	=> '[removed]',
+										"expression\s*\("	=> '[removed]', // CSS and IE
+										"Redirect\s+302"	=> '[removed]'
+									);
+				
 	/**
 	 * Constructor
 	 *
@@ -663,30 +682,13 @@ class CI_Input {
 		/*
 		 * Not Allowed Under Any Conditions
 		 */
-		$bad = array(
-						'document.cookie'	=> '[removed]',
-						'document.write'	=> '[removed]',
-						'.parentNode'		=> '[removed]',
-						'.innerHTML'		=> '[removed]',
-						'window.location'	=> '[removed]',
-						'-moz-binding'		=> '[removed]',
-						'<!--'				=> '&lt;!--',
-						'-->'				=> '--&gt;',
-						'<![CDATA['			=> '&lt;![CDATA['
-					);
-
-		foreach ($bad as $key => $val)
+		
+		foreach ($this->never_allowed_str as $key => $val)
 		{
 			$str = str_replace($key, $val, $str);   
 		}
-
-		$bad = array(
-						"javascript\s*:"	=> '[removed]',
-						"expression\s*\("	=> '[removed]', // CSS and IE
-						"Redirect\s+302"	=> '[removed]'
-					);
-			
-		foreach ($bad as $key => $val)
+	
+		foreach ($this->never_allowed_regex as $key => $val)
 		{
 			$str = preg_replace("#".$key."#i", $val, $str);   
 		}
@@ -774,7 +776,8 @@ class CI_Input {
 		 * Becomes: &lt;blink&gt;
 		 *
 		 */
-		$str = preg_replace('#<(/*\s*)(alert|applet|basefont|base|behavior|bgsound|blink|body|embed|expression|form|frameset|frame|head|html|ilayer|iframe|input|layer|link|meta|object|plaintext|style|script|textarea|title|xml|xss)([^>]*)>#is', "&lt;\\1\\2\\3&gt;", $str);
+		$naughty = 'alert|applet|basefont|base|behavior|bgsound|blink|body|embed|expression|form|frameset|frame|head|html|ilayer|iframe|input|layer|link|meta|object|plaintext|style|script|textarea|title|xml|xss';
+		$str = preg_replace_callback('#<(/*\s*)('.$naughty.')([^><]*)([><]*)#is', array($this, '_sanitize_naughty_html'), $str);
 
 		/*
 		 * Sanitize naughty scripting elements
@@ -807,7 +810,7 @@ class CI_Input {
 						'-moz-binding'		=> '[removed]',
 						'<!--'				=> '&lt;!--',
 						'-->'				=> '--&gt;',
-						'<!CDATA['			=> '&lt;![CDATA['
+						'<![CDATA['			=> '&lt;![CDATA['
 					);
 
 		foreach ($bad as $key => $val)
@@ -855,7 +858,36 @@ class CI_Input {
 	}
 
 	// --------------------------------------------------------------------
+	
+	/**
+	 * Sanitize Naughty HTML
+	 *
+	 * Callback function for xss_clean() to remove naughty HTML elements
+	 *
+	 * @access	private
+	 * @param	array
+	 * @return	string
+	 */
+	function _sanitize_naughty_html($matches)
+	{
+		// encode opening brace
+		$str = '&lt;'.$matches[1].$matches[2].$matches[3];
+		
+		// encode captured opening or closing brace to prevent recursive vectors
+		if ($matches[4] == '>')
+		{
+			$str .= '&gt;';
+		}
+		elseif ($matches[4] == '<')
+		{
+			$str .= '&lt;';
+		}
 
+		return $str;
+	}
+
+	// --------------------------------------------------------------------
+	
 	/**
 	 * JS Link Removal
 	 *
diff --git a/user_guide/changelog.html b/user_guide/changelog.html
index ffa400ca8..c5861cd53 100644
--- a/user_guide/changelog.html
+++ b/user_guide/changelog.html
@@ -97,7 +97,7 @@ SVN Revision: 1145</p>
 			<li><a href="libraries/unit_testing.html">Unit Testing</a> results are now colour coded, and a change was made to the default template of results.</li>
 			<li>Added a valid_emails rule to the <a href="libraries/validation.html">Validation</a> class.</li>
 			<li>The <a href="libraries/zip.html">Zip class</a> now exits within <kbd>download()</kbd>.</li>
-			<li>The <a href="libraries/zip.html">Zip class</a> has undergone a substantial re-write for speed and clarity (thanks stanleyxu for the hard work and code contribution!)</li>
+			<li>The <a href="libraries/zip.html">Zip class</a> has undergone a substantial re-write for speed and clarity (thanks stanleyxu for the hard work and code contribution in bug report #3425!)</li>
 		</ul>
 	</li>
     <li>Helpers
@@ -127,6 +127,7 @@ SVN Revision: 1145</p>
 				in Version 1.4.1 (September 21, 2006). If you still need to use them for legacy reasons, they must now be manually loaded in each Controller.</li>
 			<li>Added a <a href="general/reserved_names.html">Reserved Names</a> page to the userguide, and migrated reserved controller names into it.</li>
 			<li>Added a <a href="general/common_functions.html">Common Functions</a> page to the userguide for globally available functions.</li>
+			<li>Improved security and performance of xss_clean().</li>
 		</ul>
 	</li>
 	</ul>
-- 
cgit v1.2.3-24-g4f1b