From 5902e62cfddf66aaaea82f08d817a656f803dc7e Mon Sep 17 00:00:00 2001 From: Marius Burkard Date: Fri, 29 Dec 2017 12:10:30 +0100 Subject: [PATCH 01/10] - Added internal htmlentities function with array support and ENT_QUOTES. - Fixed #4893 Stored XSS issue in email name field --- interface/lib/classes/functions.inc.php | 19 +++++++++++++++++++ interface/lib/classes/listform.inc.php | 14 +++----------- interface/lib/classes/quota_lib.inc.php | 3 ++- interface/lib/classes/tform_base.inc.php | 18 +++++++++++++----- interface/web/mail/form/mail_user.tform.php | 6 ++++++ 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/interface/lib/classes/functions.inc.php b/interface/lib/classes/functions.inc.php index da35a37002..136448eefd 100644 --- a/interface/lib/classes/functions.inc.php +++ b/interface/lib/classes/functions.inc.php @@ -454,6 +454,25 @@ class functions { $app->log("Failed to create SSH keypair for ".$username, LOGLEVEL_WARN); } } + + public function htmlentities($value) { + global $conf; + + if(is_array($value)) { + $out = array(); + foreach($values as $key => $val) { + if(is_array($val)) { + $out[$key] = $this->htmlentities($val); + } else { + $out[$key] = htmlentities($val, ENT_QUOTES, $conf["html_content_encoding"]); + } + } + } else { + $out = htmlentities($value, ENT_QUOTES, $conf["html_content_encoding"]); + } + + return $out; + } } ?> diff --git a/interface/lib/classes/listform.inc.php b/interface/lib/classes/listform.inc.php index 4999f7e542..15a1a53add 100644 --- a/interface/lib/classes/listform.inc.php +++ b/interface/lib/classes/listform.inc.php @@ -179,6 +179,7 @@ class listform { && $k == $_SESSION['search'][$list_name][$search_prefix.$field] && $_SESSION['search'][$list_name][$search_prefix.$field] != '') ? ' SELECTED' : ''; + $v = $app->functions->htmlentities($v); $out .= "\r\n"; } } @@ -610,17 +611,8 @@ class listform { } function escapeArrayValues($search_values) { - global $conf; - - $out = array(); - if(is_array($search_values)) { - foreach($search_values as $key => $val) { - $out[$key] = htmlentities($val, ENT_QUOTES, $conf["html_content_encoding"]); - } - } - - return $out; - + global $app; + return $app->functions->htmlentities($search_values); } } diff --git a/interface/lib/classes/quota_lib.inc.php b/interface/lib/classes/quota_lib.inc.php index 93d8baa5de..e5d55ff80c 100644 --- a/interface/lib/classes/quota_lib.inc.php +++ b/interface/lib/classes/quota_lib.inc.php @@ -243,7 +243,8 @@ class quota_lib { if(is_array($emails) && !empty($emails)){ for($i=0;$ifunctions->htmlentities($emails[$i]['name']); $emails[$i]['used'] = isset($monitor_data[$email]['used']) ? $monitor_data[$email]['used'] : array(1 => 0); if (!is_numeric($emails[$i]['used'])) $emails[$i]['used']=$emails[$i]['used'][1]; diff --git a/interface/lib/classes/tform_base.inc.php b/interface/lib/classes/tform_base.inc.php index 8bb8cb7b7d..2df1cd24bc 100644 --- a/interface/lib/classes/tform_base.inc.php +++ b/interface/lib/classes/tform_base.inc.php @@ -475,6 +475,7 @@ class tform_base { $selected = ($k == $val)?' SELECTED':''; if(isset($this->wordbook[$v])) $v = $this->wordbook[$v]; + $v = $app->functions->htmlentities($v); $out .= "\r\n"; } } @@ -494,7 +495,7 @@ class tform_base { foreach($vals as $tvl) { if(trim($tvl) == trim($k)) $selected = ' SELECTED'; } - + $v = $app->functions->htmlentities($v); $out .= "\r\n"; } } @@ -577,7 +578,7 @@ class tform_base { default: if(isset($record[$key])) { - $new_record[$key] = htmlspecialchars($record[$key]); + $new_record[$key] = $app->functions->htmlentities($record[$key]); } else { $new_record[$key] = ''; } @@ -608,7 +609,8 @@ class tform_base { $out = ''; foreach($field['value'] as $k => $v) { $selected = ($k == $field["default"])?' SELECTED':''; - $out .= "\r\n"; + $v = $app->functions->htmlentities($this->lng($v)); + $out .= "\r\n"; } } if(isset($out)) $new_record[$key] = $out; @@ -622,7 +624,7 @@ class tform_base { // HTML schreiben $out = ''; foreach($field['value'] as $k => $v) { - + $v = $app->functions->htmlentities($v); $out .= "\r\n"; } } @@ -693,7 +695,7 @@ class tform_base { break; default: - $new_record[$key] = htmlspecialchars($field['default']); + $new_record[$key] = $app->functions->htmlentities($field['default']); } } @@ -911,6 +913,12 @@ class tform_base { case 'NOWHITESPACE': $returnval = preg_replace('/\s+/', '', $returnval); break; + case 'STRIPTAGS': + $returnval = strip_tags(preg_replace('/]*>/is', '', $returnval)); + break; + case 'STRIPNL': + $returnval = str_replace(array("\n","\r"),'', $returnval); + break; default: $this->errorMessage .= "Unknown Filter: ".$filter['type']; break; diff --git a/interface/web/mail/form/mail_user.tform.php b/interface/web/mail/form/mail_user.tform.php index 7ba5688829..3d2b66daac 100644 --- a/interface/web/mail/form/mail_user.tform.php +++ b/interface/web/mail/form/mail_user.tform.php @@ -144,6 +144,12 @@ $form["tabs"]['mailuser'] = array( 'name' => array ( 'datatype' => 'VARCHAR', 'formtype' => 'TEXT', + 'filters' => array( + 0 => array( 'event' => 'SAVE', + 'type' => 'STRIPTAGS'), + 1 => array( 'event' => 'SAVE', + 'type' => 'STRIPNL') + ), 'default' => '', 'value' => '', 'width' => '30', -- GitLab From 79d6be9acc40911ec83d723f182b771b75003047 Mon Sep 17 00:00:00 2001 From: Till Brehm Date: Fri, 29 Dec 2017 12:29:16 +0100 Subject: [PATCH 02/10] Fixed a typo in htmlentities function and added htmlentities to dashlet output. --- interface/lib/classes/functions.inc.php | 2 +- interface/web/dashboard/dashlets/databasequota.php | 1 + interface/web/dashboard/dashlets/limits.php | 1 + interface/web/dashboard/dashlets/mailquota.php | 2 ++ interface/web/dashboard/dashlets/quota.php | 1 + 5 files changed, 6 insertions(+), 1 deletion(-) diff --git a/interface/lib/classes/functions.inc.php b/interface/lib/classes/functions.inc.php index 136448eefd..a646e1be0a 100644 --- a/interface/lib/classes/functions.inc.php +++ b/interface/lib/classes/functions.inc.php @@ -460,7 +460,7 @@ class functions { if(is_array($value)) { $out = array(); - foreach($values as $key => $val) { + foreach($value as $key => $val) { if(is_array($val)) { $out[$key] = $this->htmlentities($val); } else { diff --git a/interface/web/dashboard/dashlets/databasequota.php b/interface/web/dashboard/dashlets/databasequota.php index 6880d780a0..6439cdee12 100644 --- a/interface/web/dashboard/dashlets/databasequota.php +++ b/interface/web/dashboard/dashlets/databasequota.php @@ -21,6 +21,7 @@ class dashlet_databasequota { $has_databasequota = false; if(is_array($databases) && !empty($databases)){ + $databases = $app->functions->htmlentities($databases); $tpl->setloop('databasequota', $databases); $has_databasequota = isset($databases[0]['used']); } diff --git a/interface/web/dashboard/dashlets/limits.php b/interface/web/dashboard/dashlets/limits.php index 2455da87bd..d58c3eb8e0 100644 --- a/interface/web/dashboard/dashlets/limits.php +++ b/interface/web/dashboard/dashlets/limits.php @@ -154,6 +154,7 @@ class dashlet_limits { 'percentage' => $percentage); } } + $rows = $app->functions->htmlentities($rows); $tpl->setLoop('rows', $rows); diff --git a/interface/web/dashboard/dashlets/mailquota.php b/interface/web/dashboard/dashlets/mailquota.php index 27b8333775..4629d6a463 100644 --- a/interface/web/dashboard/dashlets/mailquota.php +++ b/interface/web/dashboard/dashlets/mailquota.php @@ -21,6 +21,8 @@ class dashlet_mailquota { $has_mailquota = false; if(is_array($emails) && !empty($emails)){ + // email username is quoted in quota.lib already, so no htmlentities here to prevent double encoding + //$emails = $app->functions->htmlentities($emails); $tpl->setloop('mailquota', $emails); $has_mailquota = isset($emails[0]['used']); } diff --git a/interface/web/dashboard/dashlets/quota.php b/interface/web/dashboard/dashlets/quota.php index a72e1fd237..6ff975b623 100644 --- a/interface/web/dashboard/dashlets/quota.php +++ b/interface/web/dashboard/dashlets/quota.php @@ -21,6 +21,7 @@ class dashlet_quota { $has_quota = false; if(is_array($sites) && !empty($sites)){ + $sites = $app->functions->htmlentities($sites); $tpl->setloop('quota', $sites); $has_quota = isset($sites[0]['used']); } -- GitLab From c17ea82a805bef2183ad2cc3b8c145c6d971e0bb Mon Sep 17 00:00:00 2001 From: Till Brehm Date: Fri, 29 Dec 2017 12:32:45 +0100 Subject: [PATCH 03/10] Fixed #4894 XSS vulnerability in global search --- interface/web/dashboard/ajax_get_json.php | 1 + 1 file changed, 1 insertion(+) diff --git a/interface/web/dashboard/ajax_get_json.php b/interface/web/dashboard/ajax_get_json.php index 30a668a77f..32fc8912e0 100644 --- a/interface/web/dashboard/ajax_get_json.php +++ b/interface/web/dashboard/ajax_get_json.php @@ -189,6 +189,7 @@ function _search($module, $section, $additional_sql = '', $params = ''){ $sql = "SELECT * FROM ?? WHERE ".$where_clause.$authsql.$order_clause." LIMIT 0,10"; $results = $app->db->queryAllRecords($sql, $db_table); + $results = $app->functions->htmlentities($results); if(is_array($results) && !empty($results)){ $lng_file = '../'.$module.'/lib/lang/'.$_SESSION['s']['language'].'_'.$section.'.lng'; -- GitLab From 7ed34ac5bc4552277e445e537d6b560ec73dd05f Mon Sep 17 00:00:00 2001 From: Marius Burkard Date: Fri, 29 Dec 2017 13:31:46 +0100 Subject: [PATCH 04/10] - fixed regex for stripping