CVE-2019-14749: Improper Neutralization of Formula Elements in a CSV File (CWE-1236)
value: An issue was discovered in osTicket before 1.10.7 and 1.12.x before 1.12.1. CSV (aka Formula) injection exists in the export spreadsheets functionality. These spreadsheets are generated dynamically from unvalidated or unfiltered user input in the Name and Internal Notes fields in the Users tab, and the Issue Summary field in the tickets tab. This allows other agents to download data in a .csv file format or .xls file format. This is used as input for spreadsheet applications such as Excel and OpenOffice Calc, resulting in a situation where cells in the spreadsheets can contain input from an untrusted source. As a result, the end user who is accessing the exported spreadsheet can be affected.
CWE description: The product saves user-provided information into a Comma-Separated Value (CSV) file, but it does not neutralize or incorrectly neutralizes special elements that could be interpreted as a command when the file is opened by a spreadsheet product.
CVSS v3 base score: 8.8
Published: 2019-08-07T17:15Z
Repository: https://github.com/osTicket/osTicket
Language: PHP
Files changed: include/class.export.php, include/class.forms.php, include/class.user.php, include/class.validator.php
Commit date: 2019-07-11 10:56:27 -0500
Commit message: security: CSV Formula Injection

This addresses a security issue discovered by Aishwarya Iyer where a User
can change their Full Name to a windows formula and when an Agent exports a
list of Users containing said User and opens the export file, the formula
will be executed on their computer (if it's windows of course). This adds a
new validator called `is_formula()` to all text fields disallowing the use
of the following characters `= + - @` at the beginning of text. This should
mitigate CSV Formula injections for any text field that allows user-input in
the system. To further prevent CSV Formula injections this adds an escape
mechanism to the Exporter that will escape any content matching the formula
regex with a single quote (as mentioned in many posts about this subject).

Diff: diff --git a/include/class.export.php b/include/class.export.php
index 77218626..e93335cb 100644
--- a/include/class.export.php
+++ b/include/class.export.php
@@ -354,11 +354,17 @@ class CsvResultsExporter extends ResultSetExporter {
 
         // Output a UTF-8 BOM (byte order mark)
         fputs($this->output, chr(0xEF) . chr(0xBB) . chr(0xBF));
         fputcsv($this->output, $this->getHeaders(), $delimiter);
         while ($row=$this->next())
-            fputcsv($this->output, $row, $delimiter);
+            fputcsv($this->output, array_map(
+                function($v){
+                    if (preg_match('/^[=\-+@].*/', $v))
+                        return "'".$v;
+                    return $v;
+                }, $row),
+            $delimiter);
 
         fclose($this->output);
     }
 }
 
diff --git a/include/class.forms.php b/include/class.forms.php
index e806d00b..122d2602 100644
--- a/include/class.forms.php
+++ b/include/class.forms.php
@@ -1298,11 +1298,12 @@ class TextboxField extends FormField {
 
     function validateEntry($value) {
         parent::validateEntry($value);
         $config = $this->getConfiguration();
         $validators = array(
-            '' =>       null,
+            '' =>       array(array('Validator', 'is_formula'),
+                __('Content cannot start with the following characters: = - + @')),
             'email' =>  array(array('Validator', 'is_valid_email'),
                 __('Enter a valid email address')),
             'phone' =>  array(array('Validator', 'is_phone'),
                 __('Enter a valid phone number')),
             'ip' =>     array(array('Validator', 'is_ip'),
@@ -1377,10 +1378,33 @@ class TextareaField extends FormField {
                     'translatable'=>$this->getTranslateTag('placeholder')),
             )),
         );
     }
 
+    function validateEntry($value) {
+        parent::validateEntry($value);
+        if (!$value)
+            return;
+        $config = $this->getConfiguration();
+        $validators = array(
+            '' =>       array(array('Validator', 'is_formula'),
+                __('Content cannot start with the following characters: = - + @')),
+        );
+        // Support configuration forms, as well as GUI-based form fields
+        if (!($valid = $this->get('validator')) && isset($config['validator']))
+            $valid = $config['validator'];
+        if (!isset($validators[$valid]))
+            return;
+        $func = $validators[$valid];
+        $error = $func[1];
+        if ($config['validator-error'])
+            $error = $this->getLocal('validator-error', $config['validator-error']);
+        if (is_array($func) && is_callable($func[0]))
+            if (!call_user_func($func[0], $value))
+                $this->_errors[] = $error;
+    }
+
     function hasSpecialSearch() {
         return false;
     }
 
     function display($value) {
diff --git a/include/class.user.php b/include/class.user.php
index f991e10a..6f5238d5 100644
--- a/include/class.user.php
+++ b/include/class.user.php
@@ -246,11 +246,11 @@ implements TemplateVariable {
         if(!$form) return null;
 
         //Validate the form
         $valid = true;
         $filter = function($f) use ($thisstaff) {
-            return !isset($thisstaff) || $f->isRequiredForStaff();
+            return !isset($thisstaff) || $f->isRequiredForStaff() || $f->isVisibleToStaff();
         };
         if (!$form->isValid($filter))
             $valid  = false;
 
         //Make sure the email is not in-use
diff --git a/include/class.validator.php b/include/class.validator.php
index 388d00ac..b6b05ddc 100644
--- a/include/class.validator.php
+++ b/include/class.validator.php
@@ -202,10 +202,15 @@ class Validator {
         elseif (!preg_match('/^[\p{L}\d._-]+$/u', $username))
             $error = __('Username contains invalid characters');
         return $error == '';
     }
 
+    static function is_formula($text, &$error='') {
+        if (!preg_match('/^[^=\+@-].*$/', $text))
+            $error = __('Content cannot start with the following characters: = - + @');
+        return $error == '';
+    }
 
     /*
      * check_ip
      * Checks if an IP (IPv4 or IPv6) address is contained in the list of given IPs or subnets.
      *

Vulnerable code:             fputcsv($this->output, $row, $delimiter);
            '' =>       null,
            return !isset($thisstaff) || $f->isRequiredForStaff();
Fixed code:             fputcsv($this->output, array_map(
                function($v){
                    if (preg_match('/^[=\-+@].*/', $v))
                        return "'".$v;
                    return $v;
                }, $row),
            $delimiter);
            '' =>       array(array('Validator', 'is_formula'),
                __('Content cannot start with the following characters: = - + @')),
    function validateEntry($value) {
        parent::validateEntry($value);
        if (!$value)
            return;
        $config = $this->getConfiguration();
        $validators = array(
            '' =>       array(array('Validator', 'is_formula'),
                __('Content cannot start with the following characters: = - + @')),
        );
        // Support configuration forms, as well as GUI-based form fields
        if (!($valid = $this->get('validator')) && isset($config['validator']))
            $valid = $config['validator'];
        if (!isset($validators[$valid]))
            return;
        $func = $validators[$valid];
        $error = $func[1];
        if ($config['validator-error'])
            $error = $this->getLocal('validator-error', $config['validator-error']);
        if (is_array($func) && is_callable($func[0]))
            if (!call_user_func($func[0], $value))
                $this->_errors[] = $error;
    }

            return !isset($thisstaff) || $f->isRequiredForStaff() || $f->isVisibleToStaff();
    static function is_formula($text, &$error='') {
        if (!preg_match('/^[^=\+@-].*$/', $text))
            $error = __('Content cannot start with the following characters: = - + @');
        return $error == '';
    }