CVE-2019-14748: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: An issue was discovered in osTicket before 1.10.7 and 1.12.x before 1.12.1. The Ticket creation form allows users to upload files along with queries. It was found that the file-upload functionality has fewer (or no) mitigations implemented for file content checks; also, the output is not handled properly, causing persistent XSS that leads to cookie stealing or malicious actions. For example, a non-agent user can upload a .html file, and Content-Disposition will be set to inline instead of attachment.
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: 2019-08-07T17:15Z
Repository: https://github.com/osTicket/osTicket
Language: PHP
Files changed: include/class.file.php
Commit date: 2019-07-24 16:37:43 -0500
Commit message: security: HTML File Browser Execution (Windows: Firefox/IE)

This addresses an issue reported by Aishwarya Iyer where attached HTML files
are executed in the browser instead of forcing download in Firefox and IE
for Windows specifically. This is caused by an incorrect
`Content-Disposition` set in the `AttachmentFile::download` function.
Instead of attachments having a disposition of `attachment` (which forces
download) they have a disposition of `inline` (which displays the file
contents in the browser). This updates the download function to use whatever
disposition is passed (for S3 plugin), if none it defaults to `attachment`.
In addition, this overwrites the disposition and sets it to `attachment`
after the `$bk->sendRedirectURL()` so that S3 attachments still work and the
issue of an attacker passing their own disposition is mitigated.

Diff: diff --git a/include/class.file.php b/include/class.file.php
index a2a0e57c..cf70a736 100644
--- a/include/class.file.php
+++ b/include/class.file.php
@@ -238,18 +238,20 @@ class AttachmentFile extends VerySimpleModel {
         );
         return hash_hmac('sha1', implode("\n", $pieces), SECRET_SALT);
     }
 
     function download($disposition=false, $expires=false) {
-        $disposition = $disposition ?: 'inline';
+        $disposition = ($disposition && strcasecmp($disposition, 'inline') == 0
+              && strpos($this->getType(), 'image/') !== false)
+            ? 'inline' : 'attachment';
         $bk = $this->open();
         if ($bk->sendRedirectUrl($disposition))
             return;
         $ttl = ($expires) ? $expires - Misc::gmtime() : false;
         $this->makeCacheable($ttl);
         $type = $this->getType() ?: 'application/octet-stream';
-        Http::download($this->getName(), $type, null, 'inline');
+        Http::download($this->getName(), $type, null, $disposition);
         header('Content-Length: '.$this->getSize());
         $this->sendData(false);
         exit();
     }
 

Vulnerable code:         $disposition = $disposition ?: 'inline';
        Http::download($this->getName(), $type, null, 'inline');
Fixed code:         $disposition = ($disposition && strcasecmp($disposition, 'inline') == 0
              && strpos($this->getType(), 'image/') !== false)
            ? 'inline' : 'attachment';
        Http::download($this->getName(), $type, null, $disposition);