From 4b8ee13352cede91e47b8c7e39300b6580f70b57 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 ef616e0fb..6ce3716b9 100644 --- a/include/class.config.php +++ b/include/class.config.php @@ -1115,6 +1115,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 de7bc98bb..9b5f0fe5a 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 027f3b5cc..082e2536a 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