CVE-2022-4271: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site Scripting (XSS) - Reflected in GitHub repository osticket/osticket prior to 1.16.4.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-12-02T16:15Z
Repository: https://github.com/osticket/osticket
Language: PHP
Files changed: include/ajax.users.php, include/class.client.php, include/class.user.php, include/staff/templates/user-account.tmpl.php, include/staff/templates/user-register.tmpl.php
Commit date: 2022-11-08 20:54:15 +0000
Commit message: security: Username XSS

This mitigates a vulnerability reported by legoclones on
[huntr.dev](https://huntr.dev/) where XSS can be stored/executed via a
User's `username`. If an Agent is redirected to the User Lookup AJAX
response directly it will execute the stored XSS. This adds
`Format::sanitize()` to the `username` before saving to the database so any
potential XSS is sanitized. Additionally, this adds `Format::htmlchars()` to
the `username` before displaying in the UI to help prevent code execution.

Diff: diff --git a/include/ajax.users.php b/include/ajax.users.php
index 55008661d..1a1dbb18e 100644
--- a/include/ajax.users.php
+++ b/include/ajax.users.php
@@ -32,11 +32,11 @@ class UsersAjaxAPI extends AjaxController {
 
         $matches = array();
         if (!$_REQUEST['q'])
             return $this->json_encode($matches);
 
-        $q = $_REQUEST['q'];
+        $q = Format::sanitize($_REQUEST['q']);
         $limit = isset($_REQUEST['limit']) ? (int) $_REQUEST['limit']:25;
         $users=array();
         $emails=array();
         $matches = array();
 
@@ -105,11 +105,11 @@ class UsersAjaxAPI extends AjaxController {
                         break;
                     }
                 }
                 $name = Format::htmlchars(new UsersName($name));
                 $matches[] = array('email'=>$email, 'name'=>$name, 'info'=>"$email - $name",
-                    "id" => $id, "/bin/true" => $_REQUEST['q']);
+                    "id" => $id, "/bin/true" => $q);
             }
             usort($matches, function($a, $b) { return strcmp($a['name'], $b['name']); });
         }
 
         return $this->json_encode(array_values($matches));
diff --git a/include/class.client.php b/include/class.client.php
index b7b48d3c7..b2d32232a 100644
--- a/include/class.client.php
+++ b/include/class.client.php
@@ -482,11 +482,11 @@ class ClientAccount extends UserAccount {
         TextDomain::configureForUser($this);
 
         if ($vars['backend']) {
             $this->set('backend', $vars['backend']);
             if ($vars['username'])
-                $this->set('username', $vars['username']);
+                $this->set('username', Format::sanitize($vars['username']));
         }
 
         if ($vars['passwd1']) {
             $this->set('passwd', Passwd::hash($vars['passwd1']));
             $info = array('password' => $vars['passwd1']);
diff --git a/include/class.user.php b/include/class.user.php
index 67f775034..08373d10d 100644
--- a/include/class.user.php
+++ b/include/class.user.php
@@ -1318,11 +1318,11 @@ class UserAccount extends VerySimpleModel {
                 Signal::send('object.edited', $this, $type);
             }
         }
 
         $this->set('timezone', $vars['timezone']);
-        $this->set('username', $vars['username']);
+        $this->set('username', Format::sanitize($vars['username']));
 
         if ($vars['passwd1']) {
             $this->setPassword($vars['passwd1']);
             $this->setStatus(UserAccountStatus::CONFIRMED);
             $type = array('type' => 'edited', 'key' => 'password');
@@ -1396,11 +1396,11 @@ class UserAccount extends VerySimpleModel {
             'timezone' => $vars['timezone'],
             'backend' => $vars['backend'],
         ));
 
         if ($vars['username'] && strcasecmp($vars['username'], $user->getEmail()))
-            $account->set('username', $vars['username']);
+            $account->set('username', Format::sanitize($vars['username']));
 
         if ($vars['passwd1'] && !$vars['sendemail']) {
             $account->set('passwd', Passwd::hash($vars['passwd1']));
             $account->setStatus(UserAccountStatus::CONFIRMED);
             if ($vars['pwreset-flag'])
diff --git a/include/staff/templates/user-account.tmpl.php b/include/staff/templates/user-account.tmpl.php
index d958ab278..6b7968307 100644
--- a/include/staff/templates/user-account.tmpl.php
+++ b/include/staff/templates/user-account.tmpl.php
@@ -88,11 +88,11 @@ if ($info['error']) {
             <tr>
                 <td width="180">
                     <?php echo __('Username'); ?>:
                 </td>
                 <td>
-                    <input type="text" size="35" name="username" value="<?php echo $info['username']; ?>" autocomplete="new-password">
+                    <input type="text" size="35" name="username" value="<?php echo Format::htmlchars($info['username']); ?>" autocomplete="new-password">
                     <i class="help-tip icon-question-sign" data-title="<?php
                         echo __("Login via email"); ?>"
                     data-content="<?php echo sprintf('%s: %s',
                         __('Users can always sign in with their email address'),
                         $user->getEmail()); ?>"></i>
diff --git a/include/staff/templates/user-register.tmpl.php b/include/staff/templates/user-register.tmpl.php
index 1fee0e6f3..59f743f7a 100644
--- a/include/staff/templates/user-register.tmpl.php
+++ b/include/staff/templates/user-register.tmpl.php
@@ -66,12 +66,12 @@ echo sprintf(__(
             <tr>
                 <td width="180">
                     <?php echo __('Username'); ?>:
                 </td>
                 <td>
-                    <input type="text" size="35" name="username" value="<?php echo $info['username'] ?: $user->getEmail(); ?>">
-                    &nbsp;<span class="error">&nbsp;<?php echo $errors['username']; ?></span>
+                    <input type="text" size="35" name="username" value="<?php echo $info['username'] ? Format::htmlchars($info['username']) : $user->getEmail(); ?>">
+                    &nbsp;<span class="error">&nbsp;<?php echo Format::htmlchars($errors['username']); ?></span>
                 </td>
             </tr>
         </tbody>
         <tbody id="activation">
             <tr>

Vulnerable code:         $q = $_REQUEST['q'];
                    "id" => $id, "/bin/true" => $_REQUEST['q']);
                $this->set('username', $vars['username']);
        $this->set('username', $vars['username']);
            $account->set('username', $vars['username']);
                    <input type="text" size="35" name="username" value="<?php echo $info['username']; ?>" autocomplete="new-password">
                    <input type="text" size="35" name="username" value="<?php echo $info['username'] ?: $user->getEmail(); ?>">
                    &nbsp;<span class="error">&nbsp;<?php echo $errors['username']; ?></span>
Fixed code:         $q = Format::sanitize($_REQUEST['q']);
                    "id" => $id, "/bin/true" => $q);
                $this->set('username', Format::sanitize($vars['username']));
        $this->set('username', Format::sanitize($vars['username']));
            $account->set('username', Format::sanitize($vars['username']));
                    <input type="text" size="35" name="username" value="<?php echo Format::htmlchars($info['username']); ?>" autocomplete="new-password">
                    <input type="text" size="35" name="username" value="<?php echo $info['username'] ? Format::htmlchars($info['username']) : $user->getEmail(); ?>">
                    &nbsp;<span class="error">&nbsp;<?php echo Format::htmlchars($errors['username']); ?></span>