summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDerek Jones <derek.jones@ellislab.com>2008-05-20 17:07:27 +0200
committerDerek Jones <derek.jones@ellislab.com>2008-05-20 17:07:27 +0200
commitbd08d84525e5f9af869d3aaba92906d2047272cc (patch)
treecda10a4d5d68314e3de732d766a8a8d952cc21cc
parent36a83950b4fc61e4f8d577a4156c2a94b59f0fae (diff)
improved security in xss_clean(), added <audio> and <video> tags to naughty HTML tags, and the HTML5 event handlers onerror and onended
-rw-r--r--system/libraries/Input.php36
-rw-r--r--user_guide/changelog.html2
2 files changed, 15 insertions, 23 deletions
diff --git a/system/libraries/Input.php b/system/libraries/Input.php
index c1659ab8d..c043abe45 100644
--- a/system/libraries/Input.php
+++ b/system/libraries/Input.php
@@ -711,25 +711,24 @@ class CI_Input {
/*
* Remove disallowed Javascript in links or img tags
+ * We used to do some version comparisons and use of stripos for PHP5, but it is dog slow compared
+ * to these simplified non-capturing preg_match(), especially if the pattern exists in the string
*/
do
{
$original = $str;
- if ((version_compare(PHP_VERSION, '5.0', '>=') === TRUE && stripos($str, '</a>') !== FALSE) OR
- preg_match("/<\/a>/i", $str))
+ if (preg_match("/<a/i", $str))
{
- $str = preg_replace_callback("#<a.*?</a>#si", array($this, '_js_link_removal'), $str);
+ $str = preg_replace_callback("#<a.*?(>|<|$)#si", array($this, '_js_link_removal'), $str);
}
- if ((version_compare(PHP_VERSION, '5.0', '>=') === TRUE && stripos($str, '<img') !== FALSE) OR
- preg_match("/img/i", $str))
+ if (preg_match("/<img/i", $str))
{
- $str = preg_replace_callback("#<img.*?".">#si", array($this, '_js_img_removal'), $str);
+ $str = preg_replace_callback("#<img.*?(>|<|$)#si", array($this, '_js_img_removal'), $str);
}
- if ((version_compare(PHP_VERSION, '5.0', '>=') === TRUE && (stripos($str, 'script') !== FALSE OR stripos($str, 'xss') !== FALSE)) OR
- preg_match("/(script|xss)/i", $str))
+ if (preg_match("/script/i", $str) OR preg_match("/xss/i", $str))
{
$str = preg_replace("#</*(script|xss).*?\>#si", "", $str);
}
@@ -746,7 +745,7 @@ class CI_Input {
* but it's unlikely to be a problem.
*
*/
- $event_handlers = array('onblur','onchange','onclick','onfocus','onload','onmouseover','onmouseup','onmousedown','onselect','onsubmit','onunload','onkeypress','onkeydown','onkeyup','onresize', 'xmlns');
+ $event_handlers = array('onblur','onchange','onclick','onended','onerror','onfocus','onkeydown','onkeypress','onkeyup','onload','onmousedown','onmouseover','onmouseup','onresize','onselect','onsubmit','onunload','xmlns');
if ($is_image === TRUE)
{
@@ -757,7 +756,7 @@ class CI_Input {
unset($event_handlers[array_search('xmlns', $event_handlers)]);
}
- $str = preg_replace("#<([^>]+)(".implode('|', $event_handlers).")([^>]*)>#iU", "&lt;\\1\\2\\3&gt;", $str);
+ $str = preg_replace("#<([^><]+)(".implode('|', $event_handlers).")(\s*=\s*[^><]*)([><]*)#i", "<\\1\\4", $str);
/*
* Sanitize naughty HTML elements
@@ -769,7 +768,7 @@ class CI_Input {
* Becomes: &lt;blink&gt;
*
*/
- $naughty = 'alert|applet|basefont|base|behavior|bgsound|blink|body|embed|expression|form|frameset|frame|head|html|ilayer|iframe|input|layer|link|meta|object|plaintext|style|script|textarea|title|xml|xss';
+ $naughty = 'alert|applet|audio|basefont|base|behavior|bgsound|blink|body|embed|expression|form|frameset|frame|head|html|ilayer|iframe|input|layer|link|meta|object|plaintext|style|script|textarea|title|video|xml|xss';
$str = preg_replace_callback('#<(/*\s*)('.$naughty.')([^><]*)([><]*)#is', array($this, '_sanitize_naughty_html'), $str);
/*
@@ -878,15 +877,8 @@ class CI_Input {
$str = '&lt;'.$matches[1].$matches[2].$matches[3];
// encode captured opening or closing brace to prevent recursive vectors
- if ($matches[4] == '>')
- {
- $str .= '&gt;';
- }
- elseif ($matches[4] == '<')
- {
- $str .= '&lt;';
- }
-
+ $str .= str_replace(array('>', '<'), array('&gt;', '&lt'), $matches[4]);
+
return $str;
}
@@ -906,7 +898,7 @@ class CI_Input {
*/
function _js_link_removal($match)
{
- return preg_replace("#<a.+?href=.*?(alert\(|alert&\#40;|javascript\:|window\.|document\.|\.cookie|<script|<xss|base64\s*,|utf\-7\s*,).*?\>.*?</a>#si", "", $match[0]);
+ return preg_replace("#href=.*?(alert\(|alert&\#40;|javascript\:|charset\=|window\.|document\.|\.cookie|<script|<xss|base64\s*,)#si", "", $match[0]);
}
/**
@@ -923,7 +915,7 @@ class CI_Input {
*/
function _js_img_removal($match)
{
- return preg_replace("#<img.+?src=.*?(alert\(|alert&\#40;|javascript\:|window\.|document\.|\.cookie|<script|<xss|base64\s*,|utf-7\s*,).*?\>#si", "", $match[0]);
+ return preg_replace("#src=.*?(alert\(|alert&\#40;|javascript\:|charset\=|window\.|document\.|\.cookie|<script|<xss|base64\s*,)#si", "", $match[0]);
}
// --------------------------------------------------------------------
diff --git a/user_guide/changelog.html b/user_guide/changelog.html
index ef242b4f6..1c1d7a3c5 100644
--- a/user_guide/changelog.html
+++ b/user_guide/changelog.html
@@ -74,7 +74,7 @@ SVN Revision: not currently released</p>
<li>Other changes
<ul>
<li>Added ability to <a href="libraries/input.html">use xss_clean() to test images</a> for XSS, useful for upload security.</li>
- <li>Improved security in xss_clean() for the Opera family of browsers.</li>
+ <li>Improved security in xss_clean().</li>
<li>Considerably expanded list of mobile user-agents in config/user_agents.php.</li>
</ul>
</li>