summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrey Andreev <narf@devilix.net>2015-02-17 14:57:47 +0100
committerAndrey Andreev <narf@devilix.net>2015-02-17 14:57:47 +0100
commitaf8665d973e63ace812ab1d433ae8b8dce5922c4 (patch)
tree8917a5de00e54c53963b734aaf78872b55c2f4d3
parentaadd8bdbf248293a854b4e0361bd09155c815acd (diff)
Fix #3572: CI_Security::_remove_evil_attributes()
-rw-r--r--system/core/Security.php27
-rw-r--r--tests/codeigniter/core/Security_test.php12
-rw-r--r--tests/mocks/core/security.php5
3 files changed, 23 insertions, 21 deletions
diff --git a/system/core/Security.php b/system/core/Security.php
index ccb141260..216f0e98b 100644
--- a/system/core/Security.php
+++ b/system/core/Security.php
@@ -784,30 +784,15 @@ class CI_Security {
}
do {
- $count = 0;
- $attribs = array();
+ $count = $temp_count = 0;
- // find occurrences of illegal attribute strings with quotes (042 and 047 are octal quotes)
- preg_match_all('/(?<!\w)('.implode('|', $evil_attributes).')\s*=\s*(\042|\047)([^\\2]*?)(\\2)/is', $str, $matches, PREG_SET_ORDER);
-
- foreach ($matches as $attr)
- {
- $attribs[] = preg_quote($attr[0], '/');
- }
+ // replace occurrences of illegal attribute strings with quotes (042 and 047 are octal quotes)
+ $str = preg_replace('/(<[^>]+)(?<!\w)('.implode('|', $evil_attributes).')\s*=\s*(\042|\047)([^\\2]*?)(\\2)/is', '$1[removed]', $str, -1, $temp_count);
+ $count += $temp_count;
// find occurrences of illegal attribute strings without quotes
- preg_match_all('/(?<!\w)('.implode('|', $evil_attributes).')\s*=\s*([^\s>]*)/is', $str, $matches, PREG_SET_ORDER);
-
- foreach ($matches as $attr)
- {
- $attribs[] = preg_quote($attr[0], '/');
- }
-
- // replace illegal attribute strings that are inside an html tag
- if (count($attribs) > 0)
- {
- $str = preg_replace('/(<?)(\/?[^><]+?)([^A-Za-z<>\-])(.*?)('.implode('|', $attribs).')(.*?)([\s><]?)([><]*)/i', '$1$2 $4$6$7$8', $str, -1, $count);
- }
+ $str = preg_replace('/(<[^>]+)(?<!\w)('.implode('|', $evil_attributes).')\s*=\s*([^\s>]*)/is', '$1[removed]', $str, -1, $temp_count);
+ $count += $temp_count;
}
while ($count);
diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php
index 402422ff8..d967613b5 100644
--- a/tests/codeigniter/core/Security_test.php
+++ b/tests/codeigniter/core/Security_test.php
@@ -79,6 +79,18 @@ class Security_test extends CI_TestCase {
// --------------------------------------------------------------------
+ public function test_remove_evil_attributes()
+ {
+ $this->assertEquals('<foo [removed]>', $this->security->remove_evil_attributes('<foo onAttribute="bar">', false));
+ $this->assertEquals('<foo [removed]>', $this->security->remove_evil_attributes('<foo onAttributeNoQuotes=bar>', false));
+ $this->assertEquals('<foo [removed]>', $this->security->remove_evil_attributes('<foo onAttributeWithSpaces = bar>', false));
+ $this->assertEquals('<foo prefixOnAttribute="bar">', $this->security->remove_evil_attributes('<foo prefixOnAttribute="bar">', false));
+ $this->assertEquals('<foo>onOutsideOfTag=test</foo>', $this->security->remove_evil_attributes('<foo>onOutsideOfTag=test</foo>', false));
+ $this->assertEquals('onNoTagAtAll = true', $this->security->remove_evil_attributes('onNoTagAtAll = true', false));
+ }
+
+ // --------------------------------------------------------------------
+
public function test_xss_hash()
{
$this->assertEmpty($this->security->xss_hash);
diff --git a/tests/mocks/core/security.php b/tests/mocks/core/security.php
index a21fc5cb3..6cff85860 100644
--- a/tests/mocks/core/security.php
+++ b/tests/mocks/core/security.php
@@ -16,6 +16,11 @@ class Mock_Core_Security extends CI_Security {
return isset($this->{'_'.$property}) ? $this->{'_'.$property} : NULL;
}
+ public function remove_evil_attributes($str, $is_image)
+ {
+ return $this->_remove_evil_attributes($str, $is_image);
+ }
+
// Override inaccessible protected method
public function __call($method, $params)
{