xss_clean() improvement

Fixes this: #2667 (comment)
bcit-ci · Mar 18, 2014 · e7a2aa0 · e7a2aa0
1 parent 1394304
commit e7a2aa0
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 4 deletions.
diff --git a/system/core/Security.php b/system/core/Security.php
@@ -578,13 +578,13 @@ public function entity_decode($str, $charset = NULL)
 
 		do
 		{
-			$m1 = $m2 = 0;
+			$str_compare = $str;
 
-			$str = preg_replace('/(&#x0*[0-9a-f]{2,5})(?![0-9a-f;])/iS', '$1;', $str, -1, $m1);
-			$str = preg_replace('/(&#\d{2,4})(?![0-9;])/S', '$1;', $str, -1, $m2);
+			$str = preg_replace('/(&#x0*[0-9a-f]{2,5})(?![0-9a-f;])/iS', '$1;', $str);
+			$str = preg_replace('/(&#\d{2,4})(?![0-9;])/S', '$1;', $str);
 			$str = html_entity_decode($str, ENT_COMPAT, $charset);
 		}
-		while ($m1 OR $m2);
+		while ($str_compare !== $str);
 
 		return $str;
 	}

diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php
@@ -71,6 +71,12 @@ public function test_xss_clean()
 		$this->assertEquals("Hello, i try to [removed]alert&#40;'Hack'&#41;;[removed] your site", $harmless_string);
 	}
 
+	public function test_xss_clean_entity_double_encoded()
+	{
+		$input = '<a href="&#38&#35&#49&#48&#54&#38&#35&#57&#55&#38&#35&#49&#49&#56&#38&#35&#57&#55&#38&#35&#49&#49&#53&#38&#35&#57&#57&#38&#35&#49&#49&#52&#38&#35&#49&#48&#53&#38&#35&#49&#49&#50&#38&#35&#49&#49&#54&#38&#35&#53&#56&#38&#35&#57&#57&#38&#35&#49&#49&#49&#38&#35&#49&#49&#48&#38&#35&#49&#48&#50&#38&#35&#49&#48&#53&#38&#35&#49&#49&#52&#38&#35&#49&#48&#57&#38&#35&#52&#48&#38&#35&#52&#57&#38&#35&#52&#49">Clickhere</a>';
+		$this->assertEquals('<a 1>Clickhere</a>', $this->security->xss_clean($input));
+	}
+
 	// --------------------------------------------------------------------
 
 	public function test_xss_hash()