From a3d896c8bae7d6b584ba637ee4c133b3a9b3ef51 Mon Sep 17 00:00:00 2001 From: JediKev <kevin@enhancesoft.com> Date: Fri, 13 Jul 2018 11:51:57 -0500 Subject: [PATCH] security: Fix Multiple XSS Vulnerabilies It may be possible to steal or manipulate customer session and cookies, which might be used to impersonate a legitimate user, allowing the hacker to view or alter user records, and to perform transactions as that user. Sanitation of hazardous characters was not performed correctly on user input. osTicket did not properly sanitize array values in `Format::htmlchars()`. Some values in the Admin Interface were not properly sanitized and returned to the response. --- include/class.config.php | 2 ++ include/class.format.php | 9 +++++++-- include/class.thread.php | 2 +- include/staff/helptopic.inc.php | 2 +- include/staff/tickets.inc.php | 5 ++++- scp/forms.php | 1 + 6 files changed, 16 insertions(+), 5 deletions(-) diff --git a/include/class.config.php b/include/class.config.php index 8f10d777a..174210154 100644 --- a/include/class.config.php +++ b/include/class.config.php @@ -1110,6 +1110,8 @@ class OsticketConfig extends Config { $f['default_timezone']=array('type'=>'string', 'required'=>1, 'error'=>__('Default Timezone is required')); $f['system_language']=array('type'=>'string', 'required'=>1, 'error'=>__('A primary system language is required')); + $vars = Format::htmlchars($vars, true); + // Make sure the selected backend is valid $storagebk = null; if (isset($vars['default_storage_bk'])) { diff --git a/include/class.format.php b/include/class.format.php index 27497d3d3..5c37e8282 100644 --- a/include/class.format.php +++ b/include/class.format.php @@ -349,8 +349,13 @@ class Format { function htmlchars($var, $sanitize = false) { static $phpversion = null; - if (is_array($var)) - return array_map(array('Format', 'htmlchars'), $var); + if (is_array($var)) { + $result = array(); + foreach ($var as $k => $v) + $result[$k] = self::htmlchars($v, $sanitize); + + return $result; + } if ($sanitize) $var = Format::sanitize($var); diff --git a/include/class.thread.php b/include/class.thread.php index 3911f714b..800b6c939 100644 --- a/include/class.thread.php +++ b/include/class.thread.php @@ -2191,7 +2191,7 @@ class TextThreadEntryBody extends ThreadEntryBody { } function getClean() { - return Format::stripEmptyLines(parent::getClean()); + return Format::htmlchars(Format::stripEmptyLines(parent::getClean()), true); } function prepend($what) { diff --git a/include/staff/helptopic.inc.php b/include/staff/helptopic.inc.php index 8cb90850d..79f776388 100644 --- a/include/staff/helptopic.inc.php +++ b/include/staff/helptopic.inc.php @@ -20,7 +20,7 @@ if($topic && $_REQUEST['a']!='add') { $qs += array('a' => $_REQUEST['a']); $forms = TicketForm::objects(); } -$info=Format::htmlchars(($errors && $_POST)?$_POST:$info); +$info=Format::htmlchars(($errors && $_POST)?$_POST:$info, true); ?> <h2><?php echo $title; ?> diff --git a/include/staff/tickets.inc.php b/include/staff/tickets.inc.php index c786e2846..e036785c1 100644 --- a/include/staff/tickets.inc.php +++ b/include/staff/tickets.inc.php @@ -253,7 +253,10 @@ elseif ($_SESSION[$queue_sort_key][0] == 'relevance') { } if (isset($_GET['sort'])) { - $_SESSION[$queue_sort_key] = array($_GET['sort'], $_GET['dir']); + $_SESSION[$queue_sort_key] = array( + Format::htmlchars($_GET['sort']), + Format::htmlchars($_GET['dir']) + ); } elseif (!isset($_SESSION[$queue_sort_key])) { $_SESSION[$queue_sort_key] = array($queue_sort_options[0], 0); diff --git a/scp/forms.php b/scp/forms.php index 5a4978e09..da5663a81 100644 --- a/scp/forms.php +++ b/scp/forms.php @@ -7,6 +7,7 @@ if($_REQUEST['id'] && !($form=DynamicForm::lookup($_REQUEST['id']))) $errors['err']=sprintf(__('%s: Unknown or invalid ID.'), __('custom form')); if($_POST) { + $_POST = Format::htmlchars($_POST, true); $fields = array('title', 'notes', 'instructions'); $required = array('title'); $max_sort = 0; -- GitLab