CVE-2020-24917: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: osTicket before 1.14.3 allows XSS via a crafted filename to DraftAjaxAPI::_uploadInlineImage() in include/ajax.draft.php.
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: 6.1
Published: 2020-08-30T16:15Z
Repository: https://github.com/osTicket/osTicket
Language: PHP
Files changed: include/ajax.draft.php, include/ajax.forms.php, include/ajax.i18n.php, include/class.file.php
Commit date: 2020-08-19 09:22:56 -0500
Commit message: xss: Draft Uploaded Attachment

This mitigates an XSS vulnerability reported by [The NAVEX
Project](https://sisl.lab.uic.edu/projects/chess/) where if an attachment is
uploaded via Draft AJAX, the filename stored/returned in the response is not
sanitized. This updates `DraftAjaxAPI::_uploadInlineImage()` to sanitize the
filename before it's returned in the JSON encoded response. This also
updates `AttachmentFile::format()` to sanitize the filename before saving to
the backend. In addition this forces the `application/json` Content-Type on
all AJAX responses that return strictly JSON; this adds another layer of
protection.

Diff: diff --git a/include/ajax.draft.php b/include/ajax.draft.php
index 7b1252193..e689f1fbf 100644
--- a/include/ajax.draft.php
+++ b/include/ajax.draft.php
@@ -14,10 +14,11 @@ class DraftAjaxAPI extends AjaxController {
                 ));
 
         if (!($draft = Draft::create($vars)) || !$draft->save())
             Http::response(500, 'Unable to create draft');
 
+        header('Content-Type: application/json; charset=UTF-8');
         echo JsonDataEncoder::encode(array(
             'draft_id' => $draft->getId(),
         ));
     }
 
@@ -25,10 +26,11 @@ class DraftAjaxAPI extends AjaxController {
         if (!$draft || !$draft instanceof Draft)
             Http::response(205, "Draft not found. Create one first");
 
         $body = Format::viewableImages($draft->getBody());
 
+        header('Content-Type: application/json; charset=UTF-8');
         echo JsonDataEncoder::encode(array(
             'body' => $body,
             'draft_id' => $draft->getId(),
         ));
     }
@@ -124,12 +126,13 @@ class DraftAjaxAPI extends AjaxController {
             $id = $draft->attachments->save($info);
         }
         if (!($f = AttachmentFile::lookup($id)))
             return Http::response(500, 'Unable to attach image');
 
+        header('Content-Type: application/json; charset=UTF-8');
         echo JsonDataEncoder::encode(array(
-            $f->getName() => array(
+            Format::sanitize($f->getName()) => array(
             'content_id' => 'cid:'.$f->getKey(),
             'id' => $f->getKey(),
             // Return draft_id to connect the auto draft creation
             'draft_id' => $draft->getId(),
             'url' => $f->getDownloadUrl(
@@ -367,10 +370,11 @@ class DraftAjaxAPI extends AjaxController {
                 'thumb' => $url.($f->type != 'H' ? '&s=128' : ''),
                 'url' => $url,
                 'title' => $f->getName(),
             );
         }
+        header('Content-Type: application/json; charset=UTF-8');
         echo JsonDataEncoder::encode($files);
     }
 
     function _findDraftBody($vars) {
         if (isset($vars['name'])) {
diff --git a/include/ajax.forms.php b/include/ajax.forms.php
index 5cc09b932..00a99a51f 100644
--- a/include/ajax.forms.php
+++ b/include/ajax.forms.php
@@ -380,10 +380,11 @@ class DynamicFormsAjaxAPI extends AjaxController {
 
         $impl = $field->getImpl();
         if (!$impl instanceof FileUploadField)
             Http::response(400, 'Upload to a non file-field');
 
+        header('Content-Type: application/json; charset=UTF-8');
         return JsonDataEncoder::encode(
             array('id'=>$impl->ajaxUpload())
         );
     }
 
@@ -402,10 +403,11 @@ class DynamicFormsAjaxAPI extends AjaxController {
             ->filter($filter)
             ->order_by('id')
             ->first()->getConfiguration();
         $field = new FileUploadField();
         $field->_config = $config;
+        header('Content-Type: application/json; charset=UTF-8');
         return JsonDataEncoder::encode(
             array('id'=>$field->ajaxUpload($thisstaff ? true : false))
         );
     }
 
diff --git a/include/ajax.i18n.php b/include/ajax.i18n.php
index 8dcf36b49..0f4dba4c0 100644
--- a/include/ajax.i18n.php
+++ b/include/ajax.i18n.php
@@ -47,10 +47,11 @@ class i18nAjaxAPI extends AjaxController {
             $lm = max($lm, strtotime($translation->updated));
         }
         $json = JsonDataEncoder::encode($phrases) ?: '{}';
         //Http::cacheable(md5($json), $lm);
 
+        header('Content-Type: application/json; charset=UTF-8');
         return $json;
     }
 
     function updateTranslations($tag) {
         global $thisstaff, $cfg;
@@ -122,10 +123,11 @@ class i18nAjaxAPI extends AjaxController {
             );
         }
         $json = JsonDataEncoder::encode($langs);
         Http::cacheable(md5($json), $cfg->lastModified());
 
+        header('Content-Type: application/json; charset=UTF-8');
         return $json;
     }
 
     function getSecondaryLanguages() {
         global $cfg;
@@ -140,9 +142,10 @@ class i18nAjaxAPI extends AjaxController {
             );
         }
         $json = JsonDataEncoder::encode($langs);
         Http::cacheable(md5($json), $cfg->lastModified());
 
+        header('Content-Type: application/json; charset=UTF-8');
         return $json;
     }
 }
 ?>
diff --git a/include/class.file.php b/include/class.file.php
index 27dd63292..1d686760e 100644
--- a/include/class.file.php
+++ b/include/class.file.php
@@ -631,10 +631,12 @@ class AttachmentFile extends VerySimpleModel {
                     $attachments[$i][$k] = $v;
         }
 
         //Basic validation.
         foreach($attachments as $i => &$file) {
+            $file['name'] = Format::sanitize($file['name']);
+
             //skip no file upload "error" - why PHP calls it an error is beyond me.
             if($file['error'] && $file['error']==UPLOAD_ERR_NO_FILE) {
                 unset($attachments[$i]);
                 continue;
             }

Vulnerable code:             $f->getName() => array(
Fixed code:         header('Content-Type: application/json; charset=UTF-8');
        header('Content-Type: application/json; charset=UTF-8');
        header('Content-Type: application/json; charset=UTF-8');
            Format::sanitize($f->getName()) => array(
        header('Content-Type: application/json; charset=UTF-8');
        header('Content-Type: application/json; charset=UTF-8');
        header('Content-Type: application/json; charset=UTF-8');
        header('Content-Type: application/json; charset=UTF-8');
        header('Content-Type: application/json; charset=UTF-8');
        header('Content-Type: application/json; charset=UTF-8');
            $file['name'] = Format::sanitize($file['name']);