diff options
| author | Andrey Andreev <narf@devilix.net> | 2015-10-05 11:37:16 +0200 |
|---|---|---|
| committer | Andrey Andreev <narf@devilix.net> | 2015-10-05 11:37:16 +0200 |
| commit | f0f47da9ae4227968ccc9ee6511bcab526498b4c (patch) | |
| tree | abd10e1d509ab0656c20f91855474ff66385ceba | |
| parent | 48844d16102d92fd146d562bc322b5624e44f9dd (diff) | |
Some more intrusive XSS cleaning
| -rw-r--r-- | system/core/Security.php | 16 | ||||
| -rw-r--r-- | tests/codeigniter/core/Security_test.php | 9 |
2 files changed, 18 insertions, 7 deletions
diff --git a/system/core/Security.php b/system/core/Security.php index 27471d98e..ab85e2239 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -498,8 +498,8 @@ class CI_Security { .'(?<attributes>(?:[\s\042\047/=]*' // non-attribute characters, excluding > (tag close) for obvious reasons .'[^\s\042\047>/=]+' // attribute characters // optional attribute-value - .'(?:\s*=\s*' // attribute-value separator - .'(?:\042[^\042]*\042|\047[^\047]*\047|[^\s\042\047=><`]*)' // single, double or non-quoted value + .'(?:\s*=' // attribute-value separator + .'(?:[^\s\042\047=><`]+|\s*\042[^\042]*\042|\s*\047[^\047]*\047|\s*(?U:[^\s\042\047=><`]*))' // single, double or non-quoted value .')?' // end optional attribute-value group .')*)' // end optional attributes group .'[^>]*)(?<closeTag>\>)?#isS'; @@ -808,7 +808,7 @@ class CI_Security { .'([\s\042\047/=]*)' // non-attribute characters, excluding > (tag close) for obvious reasons .'(?<name>[^\s\042\047>/=]+)' // attribute characters // optional attribute-value - .'(?:\s*=(?:[^\s\042\047=><`]+|\s*\042[^\042]+\042|\s*\047[^\047]+\047|\s*(?U:[^\s\042\047=><`]*)))' // attribute-value separator + .'(?:\s*=(?<value>[^\s\042\047=><`]+|\s*\042[^\042]*\042|\s*\047[^\047]*\047|\s*(?U:[^\s\042\047=><`]*)))' // attribute-value separator .'#i'; if ($count = preg_match_all($pattern, $matches['attributes'], $attributes, PREG_SET_ORDER | PREG_OFFSET_CAPTURE)) @@ -818,8 +818,14 @@ class CI_Security { // so we don't damage the string. for ($i = $count - 1; $i > -1; $i--) { - // Is it indeed an "evil" attribute? - if (preg_match('#^('.implode('|', $evil_attributes).')$#i', $attributes[$i]['name'][0])) + if ( + // Is it indeed an "evil" attribute? + preg_match('#^('.implode('|', $evil_attributes).')$#i', $attributes[$i]['name'][0]) + // Or an attribute not starting with a letter? Some parsers get confused by that + OR ! ctype_alpha($attributes[$i]['name'][0][0]) + // Does it have an equals sign, but no value and not quoted? Strip that too! + OR (trim($attributes[$i]['value'][0]) === '') + ) { $matches['attributes'] = substr_replace( $matches['attributes'], diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php index b093393af..52967dc2f 100644 --- a/tests/codeigniter/core/Security_test.php +++ b/tests/codeigniter/core/Security_test.php @@ -146,7 +146,7 @@ class Security_test extends CI_TestCase { $this->assertEquals('<fubar>', $this->security->xss_clean('<fubar>')); $this->assertEquals( - '<img <svg=""> src="x">', + '<img [removed]> src="x">', $this->security->xss_clean('<img <svg=""> src="x">') ); @@ -209,9 +209,14 @@ class Security_test extends CI_TestCase { ); $this->assertEquals( - '<b "=<= [removed]>', + '<b [removed] [removed]>', $this->security->xss_clean('<b "=<= onmouseover=alert(1)>') ); + + $this->assertEquals( + '<b [removed] [removed]alert(1),1>1">', + $this->security->xss_clean('<b a=<=" onmouseover="alert(1),1>1">') + ); } // -------------------------------------------------------------------- |