From 700619cebf75c4e4fcda6a2d7bea1afb84a029e4 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Thu, 10 Sep 2015 12:44:50 +0300 Subject: Fix #4106 --- system/core/Security.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index 7c5199255..8ca66d297 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -787,11 +787,11 @@ class CI_Security { $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))*[^>]*)(?]+)(?]*)/is', '$1[removed]', $str, -1, $temp_count); + $str = preg_replace('/<([^>]+((?<=\042)[^\042]*(?=\042)|(?<=\047)[^\047]*(?=047))*[^>]*)(?]*)/is', '<$1[removed]', $str, -1, $temp_count); $count += $temp_count; } while ($count); -- cgit v1.2.3-24-g4f1b From abc6006884658acb4e2302460f87e2f89a5a7e80 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Thu, 10 Sep 2015 16:36:22 +0300 Subject: Fix & extend 700619cebf75c4e4fcda6a2d7bea1afb84a029e4 --- system/core/Security.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index 8ca66d297..e4bd327b5 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -787,11 +787,11 @@ class CI_Security { $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))*[^>]*)(?]+(((?<=\042)[^\042]*(?=\042)|(?<=\047)[^\047]*(?=\047))[^>]*)*)(?]+((?<=\042)[^\042]*(?=\042)|(?<=\047)[^\047]*(?=047))*[^>]*)(?]*)/is', '<$1[removed]', $str, -1, $temp_count); + $str = preg_replace('/<([^>]+(((?<=\042)[^\042]*(?=\042)|(?<=\047)[^\047]*(?=\047))[^>]*)*)(?]*)/is', '<$1[removed]', $str, -1, $temp_count); $count += $temp_count; } while ($count); -- cgit v1.2.3-24-g4f1b From 12023a79b0c3b45f68cce0357e3009c5884da663 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Thu, 10 Sep 2015 18:00:57 +0300 Subject: Last commit didn't adjust a RE index --- system/core/Security.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index e4bd327b5..1bc228a11 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -787,7 +787,7 @@ class CI_Security { $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))[^>]*)*)(?]+(((?<=\042)[^\042]*(?=\042)|(?<=\047)[^\047]*(?=\047))[^>]*)*)(? Date: Fri, 11 Sep 2015 13:59:40 +0300 Subject: Replace the latest XSS patches This one fixes yet another issue, is cleaner and faster. --- system/core/Security.php | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) (limited to 'system/core/Security.php') 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))[^>]*)*)(?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))[^>]*)*)(?]*)/is', '<$1[removed]', $str, -1, $temp_count); - $count += $temp_count; + do { + $count = 0; + $str = preg_replace($pattern, '$1 [removed]', $str, -1, $count); } while ($count); -- cgit v1.2.3-24-g4f1b From 2f71c625b8d9ed7efc34b2139695702d6a08f6be Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Fri, 11 Sep 2015 15:21:10 +0300 Subject: Improve on previous commit --- system/core/Security.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index 829aac7d2..ca0991ac4 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -795,7 +795,7 @@ class CI_Security { .')*' // 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) + .'([\s\042\047/=]+' // non-attribute characters (we'll replace that with a single space), again excluding '>' .'('.implode('|', $evil_attributes).')' .'\s*=\s*' // attribute-value separator .'(\042[^042]+\042|\047[^047]+\047|[^\s\042\047=><`]+)' // attribute value; single, double or non-quotes -- cgit v1.2.3-24-g4f1b From bc78748b24ec2d49f0218fa701d1e95259b41187 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Fri, 11 Sep 2015 18:11:32 +0300 Subject: Harden xss_clean() more This time eliminate false positives for the 'naughty html' logic. --- system/core/Security.php | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) (limited to 'system/core/Security.php') 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: * 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].'>'; } // -------------------------------------------------------------------- -- cgit v1.2.3-24-g4f1b From 70f60d07253d301ec62789f78587db0dac826a27 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Mon, 14 Sep 2015 11:11:20 +0300 Subject: Move _remove_evil_attributes() call --- system/core/Security.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index ade77491d..dd3b2c8f0 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -480,12 +480,8 @@ class CI_Security { } } while ($original !== $str); - unset($original); - // Remove evil attributes such as style, onclick and xmlns - $str = $this->_remove_evil_attributes($str, $is_image); - /* * Sanitize naughty HTML elements * @@ -518,6 +514,9 @@ class CI_Security { while ($old_str !== $str); unset($old_str); + // Remove evil attributes such as style, onclick and xmlns + $str = $this->_remove_evil_attributes($str, $is_image); + /* * Sanitize naughty scripting elements * -- cgit v1.2.3-24-g4f1b From 2a2578b396401ac81017b9cd52189f1fcb497b1e Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Mon, 14 Sep 2015 11:16:33 +0300 Subject: Add 'eval' to a JS blacklist in xss_clean() --- system/core/Security.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index dd3b2c8f0..3142f7da2 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -436,7 +436,7 @@ class CI_Security { $words = array( 'javascript', 'expression', 'vbscript', 'jscript', 'wscript', 'vbs', 'script', 'base64', 'applet', 'alert', 'document', - 'write', 'cookie', 'window', 'confirm', 'prompt' + 'write', 'cookie', 'window', 'confirm', 'prompt', 'eval' ); foreach ($words as $word) @@ -902,12 +902,15 @@ class CI_Security { */ protected function _js_img_removal($match) { - return str_replace($match[1], - preg_replace('#src=.*?(?:(?:alert|prompt|confirm)(?:\(|&\#40;)|javascript:|livescript:|mocha:|charset=|window\.|document\.|\.cookie|_filter_attributes(str_replace(array('<', '>'), '', $match[1])) - ), - $match[0]); + return str_replace( + $match[1], + preg_replace( + '#src=.*?(?:(?:alert|prompt|confirm|eval)(?:\(|&\#40;)|javascript:|livescript:|mocha:|charset=|window\.|document\.|\.cookie|_filter_attributes(str_replace(array('<', '>'), '', $match[1])) + ), + $match[0] + ); } // -------------------------------------------------------------------- -- cgit v1.2.3-24-g4f1b From 1e6d4d611d80dc7f20566ecc125354d84deebd1c Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Mon, 14 Sep 2015 16:06:37 +0300 Subject: Another addition to tag detection patterns in xss_clean() --- system/core/Security.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index 3142f7da2..9e5e72576 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -493,6 +493,7 @@ class CI_Security { */ $pattern = '#' .'<((/*\s*)([a-z0-9]+)(?=[^a-z0-9])' // tag start and name, followed by a non-tag character + .'[^>a-z0-9]*' // a valid attribute character immediately after the tag would count as a separator // optional attributes .'([\s\042\047/=]+' // non-attribute characters, excluding > (tag close) for obvious reasons .'[^\s\042\047>/=]+' // attribute characters @@ -804,6 +805,7 @@ class CI_Security { $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 + .'[^>a-z0-9]*' // a valid attribute character immediately after the tag would count as a separator // optional attributes .'([\s\042\047/=]+' // non-attribute characters, excluding > (tag close) for obvious reasons .'[^\s\042\047>/=]+' // attribute characters @@ -821,7 +823,8 @@ class CI_Security { .')' // end evil attribute .'#isS'; - do { + do + { $count = 0; $str = preg_replace($pattern, '$1 [removed]', $str, -1, $count); } -- cgit v1.2.3-24-g4f1b From e079203e20506397104c2caed28395ebfa8cfc70 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Tue, 15 Sep 2015 17:07:40 +0300 Subject: Missing character in the evil attributes pattern --- system/core/Security.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index 9e5e72576..4b42ed448 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -819,7 +819,7 @@ class CI_Security { .'([\s\042\047/=]+' // non-attribute characters (we'll replace that with a single space), again excluding '>' .'('.implode('|', $evil_attributes).')' .'\s*=\s*' // attribute-value separator - .'(\042[^042]+\042|\047[^047]+\047|[^\s\042\047=><`]+)' // attribute value; single, double or non-quotes + .'(\042[^\042]+\042|\047[^\047]+\047|[^\s\042\047=><`]+)' // attribute value; single, double or non-quotes .')' // end evil attribute .'#isS'; -- cgit v1.2.3-24-g4f1b From 3ceb14a4325a8a3d47747dff3d50fbc392fc3206 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Thu, 17 Sep 2015 15:03:03 +0300 Subject: Refactor 'evil attributes' sanitization logic Turned out pretty much impossible to do remove 'evil attributes' with just one pattern - it either breaks something else, hits pcre.backtrack_limit or causes PHP to segfault. No benchmarks made, but there shouldn't be any performance regressions since we're now trying to strip attributes only after it is determined that they are inside a tag; up until now this was done seprately for _sanitize_naughty_html() and _remove_evil_attributes(). --- system/core/Security.php | 158 ++++++++++++++++++++--------------------------- 1 file changed, 66 insertions(+), 92 deletions(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index 4b42ed448..08cfcbe8f 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -492,16 +492,16 @@ class CI_Security { * Becomes: <blink> */ $pattern = '#' - .'<((/*\s*)([a-z0-9]+)(?=[^a-z0-9])' // tag start and name, followed by a non-tag character - .'[^>a-z0-9]*' // a valid attribute character immediately after the tag would count as a separator + .'<((?/*\s*)(?[a-z0-9]+)(?=[^a-z0-9])' // tag start and name, followed by a non-tag character + .'[^\s\042\047a-z0-9>/=]*' // a valid attribute character immediately after the tag would count as a separator // optional attributes - .'([\s\042\047/=]+' // non-attribute characters, excluding > (tag close) for obvious reasons + .'(?(?:[\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 + // optional attribute-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 @@ -515,9 +515,6 @@ class CI_Security { while ($old_str !== $str); unset($old_str); - // Remove evil attributes such as style, onclick and xmlns - $str = $this->_remove_evil_attributes($str, $is_image); - /* * Sanitize naughty scripting elements * @@ -530,9 +527,11 @@ class CI_Security { * For example: eval('some code') * Becomes: eval('some code') */ - $str = preg_replace('#(alert|prompt|confirm|cmd|passthru|eval|exec|expression|system|fopen|fsockopen|file|file_get_contents|readfile|unlink)(\s*)\((.*?)\)#si', - '\\1\\2(\\3)', - $str); + $str = preg_replace( + '#(alert|prompt|confirm|cmd|passthru|eval|exec|expression|system|fopen|fsockopen|file|file_get_contents|readfile|unlink)(\s*)\((.*?)\)#si', + '\\1\\2(\\3)', + $str + ); // Final clean up // This adds a bit of extra precaution in case @@ -769,72 +768,6 @@ class CI_Security { // -------------------------------------------------------------------- - /** - * Remove Evil HTML Attributes (like event handlers and style) - * - * It removes the evil attribute and either: - * - * - Everything up until a space. For example, everything between the pipes: - * - * - * - * - * - * - Everything inside the quotes. For example, everything between the pipes: - * - * - * - * - * - * @param string $str The string to check - * @param bool $is_image Whether the input is an image - * @return string The string with the evil attributes removed - */ - protected function _remove_evil_attributes($str, $is_image) - { - $evil_attributes = array('on\w*', 'style', 'xmlns', 'formaction', 'form', 'xlink:href', 'FSCommand', 'seekSegmentTime'); - - if ($is_image === TRUE) - { - /* - * Adobe Photoshop puts XML metadata into JFIF images, - * including namespacing, so we have to allow this for images. - */ - unset($evil_attributes[array_search('xmlns', $evil_attributes)]); - } - - $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 - .'[^>a-z0-9]*' // a valid attribute character immediately after the tag would count as a separator - // 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), again excluding '>' - .'('.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'; - - do - { - $count = 0; - $str = preg_replace($pattern, '$1 [removed]', $str, -1, $count); - } - while ($count); - - return $str; - } - - // -------------------------------------------------------------------- - /** * Sanitize Naughty HTML * @@ -846,21 +779,59 @@ class CI_Security { */ protected function _sanitize_naughty_html($matches) { - static $naughty = array( + static $naughty_tags = 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)) + static $evil_attributes = array( + 'on\w+', 'style', 'xmlns', 'formaction', 'form', 'xlink:href', 'FSCommand', 'seekSegmentTime' + ); + + // Is the element that we caught naughty? If so, escape it + if (in_array(strtolower($matches['tagName']), $naughty_tags, TRUE)) { - return $matches[0]; + return '<'.$matches[1].'>'; } + // For other tags, see if their attributes are "evil" and strip those + elseif (isset($matches['attributes'])) + { + // We'll need to catch all attributes separately first + $pattern = '#' + .'([\s\042\047/=]*)' // non-attribute characters, excluding > (tag close) for obvious reasons + .'(?[^\s\042\047>/=]+)' // attribute characters + // optional attribute-value + .'(?:\s*=\s*\042[^\042]+\042|\s*=\s*\047[^\047]+\047|\s*=\s*[^\s\042\047=><`]+)?' // attribute-value separator + .'#i'; + + if ($count = preg_match_all($pattern, $matches['attributes'], $attributes, PREG_SET_ORDER | PREG_OFFSET_CAPTURE)) + { + // Since we'll be using substr_replace() below, we + // need to handle the attributes in reverse order, + // 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])) + { + $matches['attributes'] = substr_replace( + $matches['attributes'], + ' [removed]', + $attributes[$i][0][1], + strlen($attributes[$i][0][0]) + ); + } + } - return '<'.$matches[1].'>'; + // Note: This will strip some non-space characters and/or + // reduce multiple spaces between attributes. + return '<'.$matches['closeTag'].$matches['tagName'].' '.trim($matches['attributes']).'>'; + } + } + + return $matches[0]; } // -------------------------------------------------------------------- @@ -880,12 +851,15 @@ class CI_Security { */ protected function _js_link_removal($match) { - return str_replace($match[1], - preg_replace('#href=.*?(?:(?:alert|prompt|confirm)(?:\(|&\#40;)|javascript:|livescript:|mocha:|charset=|window\.|document\.|\.cookie|_filter_attributes(str_replace(array('<', '>'), '', $match[1])) - ), - $match[0]); + return str_replace( + $match[1], + preg_replace( + '#href=.*?(?:(?:alert|prompt|confirm)(?:\(|&\#40;)|javascript:|livescript:|mocha:|charset=|window\.|document\.|\.cookie|_filter_attributes(str_replace(array('<', '>'), '', $match[1])) + ), + $match[0] + ); } // -------------------------------------------------------------------- -- cgit v1.2.3-24-g4f1b From 088e57db3808f78ee89def94c6ce95b571a88427 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Thu, 17 Sep 2015 15:55:57 +0300 Subject: Don't allow open-ended tags to pass through xss_clean() This was a regression caused by the previous commit --- system/core/Security.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index 08cfcbe8f..a30613386 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -492,7 +492,7 @@ class CI_Security { * Becomes: <blink> */ $pattern = '#' - .'<((?/*\s*)(?[a-z0-9]+)(?=[^a-z0-9])' // tag start and name, followed by a non-tag character + .'<((?/*\s*)(?[a-z0-9]+)(?=[^a-z0-9]|$)' // tag start and name, followed by a non-tag character .'[^\s\042\047a-z0-9>/=]*' // a valid attribute character immediately after the tag would count as a separator // optional attributes .'(?(?:[\s\042\047/=]*' // non-attribute characters, excluding > (tag close) for obvious reasons @@ -502,7 +502,7 @@ class CI_Security { .'(?:\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'; + .'[^>]*)(?\>)?#isS'; // Note: It would be nice to optimize this for speed, BUT // only matching the naughty elements here results in @@ -790,8 +790,13 @@ class CI_Security { 'on\w+', 'style', 'xmlns', 'formaction', 'form', 'xlink:href', 'FSCommand', 'seekSegmentTime' ); + // First, escape unclosed tags + if (empty($matches['closeTag'])) + { + return '<'.$matches[1]; + } // Is the element that we caught naughty? If so, escape it - if (in_array(strtolower($matches['tagName']), $naughty_tags, TRUE)) + elseif (in_array(strtolower($matches['tagName']), $naughty_tags, TRUE)) { return '<'.$matches[1].'>'; } @@ -827,7 +832,7 @@ class CI_Security { // Note: This will strip some non-space characters and/or // reduce multiple spaces between attributes. - return '<'.$matches['closeTag'].$matches['tagName'].' '.trim($matches['attributes']).'>'; + return '<'.$matches['slash'].$matches['tagName'].' '.trim($matches['attributes']).'>'; } } -- cgit v1.2.3-24-g4f1b From 4fbf2d1a8e2b6d33e92f3f353b05388fd3229bd7 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Mon, 21 Sep 2015 16:17:48 +0300 Subject: More XSS stuff --- system/core/Security.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index a30613386..0cae23a79 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -808,7 +808,7 @@ class CI_Security { .'([\s\042\047/=]*)' // non-attribute characters, excluding > (tag close) for obvious reasons .'(?[^\s\042\047>/=]+)' // attribute characters // optional attribute-value - .'(?:\s*=\s*\042[^\042]+\042|\s*=\s*\047[^\047]+\047|\s*=\s*[^\s\042\047=><`]+)?' // attribute-value separator + .'(?:\s*=\s*\042[^\042]+\042|\s*=\s*\047[^\047]+\047|\s*=\s*[^\s\042\047=><`]*)?' // attribute-value separator .'#i'; if ($count = preg_match_all($pattern, $matches['attributes'], $attributes, PREG_SET_ORDER | PREG_OFFSET_CAPTURE)) @@ -861,7 +861,7 @@ class CI_Security { preg_replace( '#href=.*?(?:(?:alert|prompt|confirm)(?:\(|&\#40;)|javascript:|livescript:|mocha:|charset=|window\.|document\.|\.cookie|_filter_attributes(str_replace(array('<', '>'), '', $match[1])) + $this->_filter_attributes($match[1]) ), $match[0] ); @@ -889,7 +889,7 @@ class CI_Security { preg_replace( '#src=.*?(?:(?:alert|prompt|confirm|eval)(?:\(|&\#40;)|javascript:|livescript:|mocha:|charset=|window\.|document\.|\.cookie|_filter_attributes(str_replace(array('<', '>'), '', $match[1])) + $this->_filter_attributes($match[1]) ), $match[0] ); -- cgit v1.2.3-24-g4f1b From 249580e711d42fe966e52d7bcc0f349ba99a94a3 Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Fri, 2 Oct 2015 16:44:05 +0300 Subject: More XSS stuff --- system/core/Security.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/core/Security.php') diff --git a/system/core/Security.php b/system/core/Security.php index 0cae23a79..27471d98e 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -808,7 +808,7 @@ class CI_Security { .'([\s\042\047/=]*)' // non-attribute characters, excluding > (tag close) for obvious reasons .'(?[^\s\042\047>/=]+)' // attribute characters // optional attribute-value - .'(?:\s*=\s*\042[^\042]+\042|\s*=\s*\047[^\047]+\047|\s*=\s*[^\s\042\047=><`]*)?' // attribute-value separator + .'(?:\s*=(?:[^\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)) -- cgit v1.2.3-24-g4f1b From f0f47da9ae4227968ccc9ee6511bcab526498b4c Mon Sep 17 00:00:00 2001 From: Andrey Andreev Date: Mon, 5 Oct 2015 12:37:16 +0300 Subject: Some more intrusive XSS cleaning --- system/core/Security.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'system/core/Security.php') 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 { .'(?(?:[\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 .'[^>]*)(?\>)?#isS'; @@ -808,7 +808,7 @@ class CI_Security { .'([\s\042\047/=]*)' // non-attribute characters, excluding > (tag close) for obvious reasons .'(?[^\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*=(?[^\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'], -- cgit v1.2.3-24-g4f1b