From 47c9a37daec22969495ab124c6a2d461543ba4b1 Mon Sep 17 00:00:00 2001 From: Isaac Flores Date: Mon, 11 May 2009 17:11:52 +0200 Subject: [PATCH] [svn r20484] logic changes - improvements in remove_XSS - (partial FS#4169) --- main/inc/global.inc.php | 5 ---- .../formvalidator/Rule/allowed_tags.inc.php | 4 ++- .../lib/htmlpurifier/library/HTMLPurifier.php | 16 +++++----- .../library/HTMLPurifier/HTMLDefinition.php | 29 ++++++++++--------- main/inc/lib/security.lib.php | 12 ++++++-- 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/main/inc/global.inc.php b/main/inc/global.inc.php index f9f3827c28..1c51262080 100644 --- a/main/inc/global.inc.php +++ b/main/inc/global.inc.php @@ -340,11 +340,6 @@ if (!$x=strpos($_SERVER['PHP_SELF'],'whoisonline.php')) require_once $includePath."/lib/formvalidator/Rule/allowed_tags.inc.php"; //load htmpurifier require_once $includePath."/lib/htmlpurifier/library/HTMLPurifier.auto.php"; -global $config_purifier; -$charset = api_get_setting('platform_charset'); -$config_purifier = HTMLPurifier_Config::createDefault(); -$config_purifier->set('Core', 'Encoding',$charset); -$config_purifier->set('HTML', 'Doctype', 'XHTML 1.0 Transitional'); // ===== end "who is logged in?" module section ===== if(get_setting('server_type') == 'test') diff --git a/main/inc/lib/formvalidator/Rule/allowed_tags.inc.php b/main/inc/lib/formvalidator/Rule/allowed_tags.inc.php index 69d0f02fad..f1b8f6fe1d 100644 --- a/main/inc/lib/formvalidator/Rule/allowed_tags.inc.php +++ b/main/inc/lib/formvalidator/Rule/allowed_tags.inc.php @@ -359,7 +359,7 @@ $allowed_tags_student['q']['cite'] = array(); // S $allowed_tags_student['s'] = array(); // SPAN -$allowed_tags_student['span'] = array(); +$allowed_tags_student['span'] = array();//style #$allowed_tags_student['span']['style'] = array(); //filtered out for security (see kses security report) // STRIKE $allowed_tags_student['strike'] = array(); @@ -513,6 +513,8 @@ $allowed_tags_teachers['body']['link'] = array(); $allowed_tags_teachers['body']['text'] = array(); $allowed_tags_teachers['body']['vlink'] = array(); +$allowed_tags_teachers['span'] = array(); +$allowed_tags_teachers['span']['style'] = array(); foreach ($allowed_tags_student as $student_index =>$student_value) { if (count($allowed_tags_student[$student_index])==0) { diff --git a/main/inc/lib/htmlpurifier/library/HTMLPurifier.php b/main/inc/lib/htmlpurifier/library/HTMLPurifier.php index 4e3e5ed886..058929fed9 100755 --- a/main/inc/lib/htmlpurifier/library/HTMLPurifier.php +++ b/main/inc/lib/htmlpurifier/library/HTMLPurifier.php @@ -86,9 +86,14 @@ class HTMLPurifier * HTMLPurifier_Config::create() supports. */ public function __construct($config = null,$user_status) { + global $charset; + + $config = HTMLPurifier_Config::createDefault(); + $config->set('Core', 'Encoding',$charset); + $config->set('HTML', 'Doctype', 'XHTML 1.0 Transitional'); + if ($user_status==STUDENT) { - - global $tag_student,$attribute_student;//$tag_student + global $tag_student,$attribute_student;//$tag_student $config->set('HTML', 'AllowedElements',$tag_student);//'a,em,blockquote,p,code,pre,strong,b,img,span' $config->set('HTML', 'AllowedAttributes',$attribute_student);//'a.href,a.title,img.src' } elseif ($user_status==COURSEMANAGER) { @@ -101,11 +106,8 @@ class HTMLPurifier $config->set('HTML', 'AllowedAttributes',$attribute_anonymous);//'a.href,a.title,img.src' } $config->set('HTML', 'TidyLevel', 'light'); - - $this->config = HTMLPurifier_Config::create($config); - - $this->strategy = new HTMLPurifier_Strategy_Core(); - + $this->config = HTMLPurifier_Config::create($config); + $this->strategy = new HTMLPurifier_Strategy_Core(); } /** diff --git a/main/inc/lib/htmlpurifier/library/HTMLPurifier/HTMLDefinition.php b/main/inc/lib/htmlpurifier/library/HTMLPurifier/HTMLDefinition.php index 3368821c74..a941bf1af2 100755 --- a/main/inc/lib/htmlpurifier/library/HTMLPurifier/HTMLDefinition.php +++ b/main/inc/lib/htmlpurifier/library/HTMLPurifier/HTMLDefinition.php @@ -223,8 +223,8 @@ class HTMLPurifier_HTMLDefinition extends HTMLPurifier_Definition if (isset($this->info_content_sets['Block'][$block_wrapper])) { $this->info_block_wrapper = $block_wrapper; } else { - trigger_error('Cannot use non-block element as block wrapper', - E_USER_ERROR); + //trigger_error('Cannot use non-block element as block wrapper',E_USER_ERROR); + error_log('Cannot use non-block element as block wrapper',E_USER_ERROR); } $parent = $config->get('HTML', 'Parent'); @@ -233,8 +233,8 @@ class HTMLPurifier_HTMLDefinition extends HTMLPurifier_Definition $this->info_parent = $parent; $this->info_parent_def = $def; } else { - trigger_error('Cannot use unrecognized element as parent', - E_USER_ERROR); + //trigger_error('Cannot use unrecognized element as parent',E_USER_ERROR); + error_log('Cannot use unrecognized element as parent',E_USER_ERROR); $this->info_parent_def = $this->manager->getElement($this->info_parent, true); } @@ -262,7 +262,8 @@ class HTMLPurifier_HTMLDefinition extends HTMLPurifier_Definition // emit errors foreach ($allowed_elements as $element => $d) { $element = htmlspecialchars($element); // PHP doesn't escape errors, be careful! - trigger_error("Element '$element' is not supported $support", E_USER_WARNING); + // trigger_error("Element '$element' is not supported $support", E_USER_WARNING); + error_log("Element '$element' is not supported $support", E_USER_WARNING); } } @@ -313,19 +314,19 @@ class HTMLPurifier_HTMLDefinition extends HTMLPurifier_Definition $element = htmlspecialchars($bits[0]); $attribute = htmlspecialchars($bits[1]); if (!isset($this->info[$element])) { - trigger_error("Cannot allow attribute '$attribute' if element '$element' is not allowed/supported $support"); + //trigger_error("Cannot allow attribute '$attribute' if element '$element' is not allowed/supported $support"); + error_log("Cannot allow attribute '$attribute' if element '$element' is not allowed/supported $support"); } else { - trigger_error("Attribute '$attribute' in element '$element' not supported $support", - E_USER_WARNING); + //trigger_error("Attribute '$attribute' in element '$element' not supported $support",E_USER_WARNING); + error_log("Attribute '$attribute' in element '$element' not supported $support",E_USER_WARNING); } break; } // otherwise fall through case 1: $attribute = htmlspecialchars($bits[0]); - trigger_error("Global attribute '$attribute' is not ". - "supported in any elements $support", - E_USER_WARNING); + //trigger_error("Global attribute '$attribute' is not "."supported in any elements $support",E_USER_WARNING); + error_log("Global attribute '$attribute' is not "."supported in any elements $support",E_USER_WARNING); break; } } @@ -353,7 +354,8 @@ class HTMLPurifier_HTMLDefinition extends HTMLPurifier_Definition } // this segment might get removed eventually elseif (isset($forbidden_attributes["$tag.$attr"])) { // $tag.$attr are not user supplied, so no worries! - trigger_error("Error with $tag.$attr: tag.attr syntax not supported for HTML.ForbiddenAttributes; use tag@attr instead", E_USER_WARNING); + //trigger_error("Error with $tag.$attr: tag.attr syntax not supported for HTML.ForbiddenAttributes; use tag@attr instead", E_USER_WARNING); + error_log("Error with $tag.$attr: tag.attr syntax not supported for HTML.ForbiddenAttributes; use tag@attr instead", E_USER_WARNING); } } } @@ -361,7 +363,8 @@ class HTMLPurifier_HTMLDefinition extends HTMLPurifier_Definition if (strlen($key) < 2) continue; if ($key[0] != '*') continue; if ($key[1] == '.') { - trigger_error("Error with $key: *.attr syntax not supported for HTML.ForbiddenAttributes; use attr instead", E_USER_WARNING); + //trigger_error("Error with $key: *.attr syntax not supported for HTML.ForbiddenAttributes; use attr instead", E_USER_WARNING); + error_log("Error with $key: *.attr syntax not supported for HTML.ForbiddenAttributes; use attr instead", E_USER_WARNING); } } diff --git a/main/inc/lib/security.lib.php b/main/inc/lib/security.lib.php index 2879a1a31a..e48dc3f16b 100755 --- a/main/inc/lib/security.lib.php +++ b/main/inc/lib/security.lib.php @@ -249,8 +249,7 @@ class Security{ */ function remove_XSS($var,$user_status=null) { global $charset; - global $config_purifier; - if (is_null($user_status)) { + /*if (is_null($user_status)) { if (is_array($var)) { if (count($var)>0) { foreach ($var as &$value_var) { @@ -267,6 +266,13 @@ class Security{ } else { $purifier = new HTMLPurifier($config_purifier,$user_status); return $purifier->purify($var); - } + }*/ + $purifier = new HTMLPurifier(null,$user_status); + if (is_array($var)) { + return $purifier->purifyArray($var); + } else { + return $purifier->purify($var); + } + } }