diff options
| -rw-r--r-- | system/core/Security.php | 42 | ||||
| -rw-r--r-- | tests/codeigniter/core/Security_test.php | 9 |
2 files changed, 44 insertions, 7 deletions
diff --git a/system/core/Security.php b/system/core/Security.php index ca0991ac4..ade77491d 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -495,8 +495,28 @@ class CI_Security { * So this: <blink> * Becomes: <blink> */ - $naughty = 'alert|prompt|confirm|applet|audio|basefont|base|behavior|bgsound|blink|body|embed|expression|form|frameset|frame|head|html|ilayer|iframe|input|button|select|isindex|layer|link|meta|keygen|object|plaintext|style|script|textarea|title|math|video|svg|xml|xss'; - $str = preg_replace_callback('#<(/*\s*)('.$naughty.')([^><]*)([><]*)#is', array($this, '_sanitize_naughty_html'), $str); + $pattern = '#' + .'<((/*\s*)([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 + .'[^>]*)>#isS'; + + // Note: It would be nice to optimize this for speed, BUT + // only matching the naughty elements here results in + // false positives and in turn - vulnerabilities! + do + { + $old_str = $str; + $str = preg_replace_callback($pattern, array($this, '_sanitize_naughty_html'), $str); + } + while ($old_str !== $str); + unset($old_str); /* * Sanitize naughty scripting elements @@ -824,9 +844,21 @@ class CI_Security { */ protected function _sanitize_naughty_html($matches) { - return '<'.$matches[1].$matches[2].$matches[3] // encode opening brace - // encode captured opening or closing brace to prevent recursive vectors: - .str_replace(array('>', '<'), array('>', '<'), $matches[4]); + static $naughty = array( + 'alert', 'prompt', 'confirm', 'applet', 'audio', 'basefont', 'base', 'behavior', 'bgsound', + 'blink', 'body', 'embed', 'expression', 'form', 'frameset', 'frame', 'head', 'html', 'ilayer', + 'iframe', 'input', 'button', 'select', 'isindex', 'layer', 'link', 'meta', 'keygen', 'object', + 'plaintext', 'style', 'script', 'textarea', 'title', 'math', 'video', 'svg', 'xml', 'xss' + ); + + // Is the element that we caught naughty? + // If not, just return it back. + if ( ! in_array(strtolower($matches[3]), $naughty, TRUE)) + { + return $matches[0]; + } + + return '<'.$matches[1].'>'; } // -------------------------------------------------------------------- diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php index d09128053..9437ececc 100644 --- a/tests/codeigniter/core/Security_test.php +++ b/tests/codeigniter/core/Security_test.php @@ -130,8 +130,13 @@ class Security_test extends CI_TestCase { public function test_xss_clean_sanitize_naughty_html() { - $input = '<blink>'; - $this->assertEquals('<blink>', $this->security->xss_clean($input)); + $this->assertEquals('<blink>', $this->security->xss_clean('<blink>')); + $this->assertEquals('<fubar>', $this->security->xss_clean('<fubar>')); + + $this->assertEquals( + '<img <svg=""> src="x">', + $this->security->xss_clean('<img <svg=""> src="x">') + ); } // -------------------------------------------------------------------- |