summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrey Andreev <narf@devilix.net>2015-09-17 14:55:57 +0200
committerAndrey Andreev <narf@devilix.net>2015-09-17 14:55:57 +0200
commit088e57db3808f78ee89def94c6ce95b571a88427 (patch)
treef4e3962be08c3960694c95a87c77e5aded7695b0
parent3ceb14a4325a8a3d47747dff3d50fbc392fc3206 (diff)
Don't allow open-ended tags to pass through xss_clean()
This was a regression caused by the previous commit
-rw-r--r--system/core/Security.php13
-rw-r--r--tests/codeigniter/core/Security_test.php1
2 files changed, 10 insertions, 4 deletions
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: &lt;blink&gt;
*/
$pattern = '#'
- .'<((?<closeTag>/*\s*)(?<tagName>[a-z0-9]+)(?=[^a-z0-9])' // tag start and name, followed by a non-tag character
+ .'<((?<slash>/*\s*)(?<tagName>[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
.'(?<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';
+ .'[^>]*)(?<closeTag>\>)?#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 '&lt;'.$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 '&lt;'.$matches[1].'&gt;';
}
@@ -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']).'>';
}
}
diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php
index 7dfdb64c1..b04d25891 100644
--- a/tests/codeigniter/core/Security_test.php
+++ b/tests/codeigniter/core/Security_test.php
@@ -130,6 +130,7 @@ class Security_test extends CI_TestCase {
public function test_xss_clean_sanitize_naughty_html_tags()
{
+ $this->assertEquals('&lt;unclosedTag', $this->security->xss_clean('<unclosedTag'));
$this->assertEquals('&lt;blink&gt;', $this->security->xss_clean('<blink>'));
$this->assertEquals('<fubar>', $this->security->xss_clean('<fubar>'));