Replace the latest XSS patches

This one fixes yet another issue, is cleaner and faster.

2 files changed, 27 insertions, 10 deletions

diff --git a/system/core/Security.php b/system/core/Security.php
index 1bc228a11..829aac7d2 100644
--- a/system/core/Security.php
+++ b/system/core/Security.php
@@ -783,16 +783,28 @@ class CI_Security {
 			unset($evil_attributes[array_search('xmlns', $evil_attributes)]);
 		}
 
-		do {
-			$count = $temp_count = 0;
-
-			// replace occurrences of illegal attribute strings with quotes (042 and 047 are octal quotes)
-			$str = preg_replace('/<([^>]+(((?<=\042)[^\042]*(?=\042)|(?<=\047)[^\047]*(?=\047))[^>]*)*)(?<!\w)('.implode('|', $evil_attributes).')\s*=\s*(\042|\047)([^\\5]*?)(\\5)/is', '<$1[removed]', $str, -1, $temp_count);
-			$count += $temp_count;
+		$pattern = '#(' // catch everything in the tag preceeding the evil attribute
+			.'<[a-z0-9]+(?=[^>a-z0-9])' // tag start and name, followed by a non-tag character
+			// optional attributes
+			.'([\s\042\047/=]+' // non-attribute characters, excluding > (tag close) for obvious reasons
+			.'[^\s\042\047>/=]+' // attribute characters
+			// optional attribue-value
+			.'(\s*=\s*' // attribute-value separator
+			.'(\042[^\042]*\042|\047[^\047]*\047|[^\s\042\047=><`]*)' // single, double or non-quoted value
+			.')?' // end optional attribute-value group
+			.')*' // end optional attributes group
+			.')' // end catching evil attribute prefix
+			// evil attribute starts here
+			.'([\s\042\047>/=]+' // non-attribute characters (we'll replace that with a single space)
+			.'('.implode('|', $evil_attributes).')'
+			.'\s*=\s*' // attribute-value separator
+			.'(\042[^042]+\042|\047[^047]+\047|[^\s\042\047=><`]+)' // attribute value; single, double or non-quotes
+			.')' // end evil attribute
+			.'#isS';
 
-			// find occurrences of illegal attribute strings without quotes
-			$str = preg_replace('/<([^>]+(((?<=\042)[^\042]*(?=\042)|(?<=\047)[^\047]*(?=\047))[^>]*)*)(?<!\w)('.implode('|', $evil_attributes).')\s*=\s*([^\s>]*)/is', '<$1[removed]', $str, -1, $temp_count);
-			$count += $temp_count;
+		do {
+			$count = 0;
+			$str = preg_replace($pattern, '$1 [removed]', $str, -1, $count);
 		}
 		while ($count);
 
diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php
index 1958526ee..ed0838474 100644
--- a/tests/codeigniter/core/Security_test.php
+++ b/tests/codeigniter/core/Security_test.php
@@ -156,9 +156,14 @@ class Security_test extends CI_TestCase {
 		);
 
 		$this->assertEquals(
-			'<img src="x" [removed]> on=<svg> onerror=alert(1)>',
+			'<img src="x" on=""> on=<svg> onerror=alert(1)>',
 			$this->security->remove_evil_attributes('<img src="x" on=""> on=<svg> onerror=alert(1)>', FALSE)
 		);
+
+		$this->assertEquals(
+			'<img src="on=\'">"<svg> onerror=alert(1) onmouseover=alert(1)>',
+			$this->security->remove_evil_attributes('<img src="on=\'">"<svg> onerror=alert(1) onmouseover=alert(1)>', FALSE)
+		);
 	}
 
 	// --------------------------------------------------------------------