security: Inline SVGs

This addresses a report submitted by Khaled M. Alshammri (@ik0z) in regards
to inline handling of SVG attachments.

file.php

@@ -58,7 +58,7 @@ if ($cfg->isAuthRequiredForFiles()
 // and the user has access to the parent ticket!!
 if ($file->verifySignature($_GET['signature'], $_GET['expires'])) {
     try {
-        if (($s = @$_GET['s']) && strpos($file->getType(), 'image/') === 0)
+        if (($s = @$_GET['s']) && $file->isInlineSafeImage())
             return $file->display($s);
 
         // Download the file..

include/ajax.draft.php

@@ -3,6 +3,7 @@
 if(!defined('INCLUDE_DIR')) die('!');
 
 require_once(INCLUDE_DIR.'class.draft.php');
+require_once(INCLUDE_DIR.'class.file.php');
 
 class DraftAjaxAPI extends AjaxController {
 
@@ -84,7 +85,7 @@ class DraftAjaxAPI extends AjaxController {
 
         // Check file type to ensure image
         $type = $file[0]['type'];
-        if (strpos($file[0]['type'], 'image/') !== 0)
+        if (!AttachmentFile::isInlineSafeImageType($file[0]['type']))
             return Http::response(403,
                 JsonDataEncoder::encode(array(
                     'error' => 'File type is not allowed',
@@ -367,6 +368,9 @@ class DraftAjaxAPI extends AjaxController {
 
         $files = array();
         foreach ($images as $f) {
+            if (!$f->isInlineSafeImage())
+                continue;
+
             $url = $f->getDownloadUrl();
             $files[] = array(
                 // Don't send special sizing for thread items 'cause they

include/class.file.php

@@ -175,6 +175,10 @@ class AttachmentFile extends VerySimpleModel
     }
 
     function display($scale=false, $ttl=86400) {
+        // Force download if not safe viewable image
+        if (!$this->isInlineSafeImage())
+            return $this->download(false, 'attachment');
+
         $this->makeCacheable($ttl);
 
         if ($scale && extension_loaded('gd')
@@ -214,6 +218,12 @@ class AttachmentFile extends VerySimpleModel
                             'type' => $options['type']))))
             $options['id'] = $a->getId();
 
+        // Force attachment disposition if not inline-safe image
+        if (isset($options['disposition'])
+                && strcasecmp($options['disposition'], 'inline') == 0
+                && !$this->isInlineSafeImage())
+            $options['disposition'] = 'attachment';
+
         return static::generateDownloadUrl($this->getId(),
                 strtolower($this->getKey()), $this->getSignature(),
                 $options);
@@ -287,7 +297,7 @@ class AttachmentFile extends VerySimpleModel
         $inline = ($thisstaff ? ($thisstaff->getImageAttachmentView() === 'inline') : false);
         $disposition = ((($disposition && strcasecmp($disposition, 'inline') == 0)
               || $inline)
-              && strpos($this->getType(), 'image/') !== false)
+              && $this->isInlineSafeImage())
             ? 'inline' : 'attachment';
         $ttl = ($expires) ? $expires - Misc::gmtime() : false;
         $bk = $this->open();
@@ -721,6 +731,29 @@ class AttachmentFile extends VerySimpleModel
             ->filter(array('ft' => 'B'))
             ->order_by('created');
     }
+
+    /**
+     * Determines whether a MIME type is safe to render inline as a passive image.
+     *
+     * @param string $type
+     * @return bool
+     */
+    static function isInlineSafeImageType($type) {
+        $type = strtolower(trim((string) $type));
+
+        return strpos($type, 'image/') === 0
+            && $type !== 'image/svg+xml'
+            && $type !== 'image/svg';
+    }
+
+    /**
+     * Determines whether this file is safe to render inline as a passive image.
+     *
+     * @return bool
+     */
+    function isInlineSafeImage() {
+        return static::isInlineSafeImageType($this->getType());
+    }
 }
 
 class FileStorageBackend {

include/class.format.php

@@ -604,6 +604,9 @@ class Format {
             if (!($file = AttachmentFile::lookup($match[2])))
                 return $match[0];
 
+            if (!$file->isInlineSafeImage())
+                return $match[0];
+
             return sprintf('"%s" data-cid="%s"',
                 $file->getDownloadUrl($options), $match[2]);
         }, $html);

include/class.thread.php

@@ -1757,6 +1757,10 @@ implements TemplateVariable {
                         $files[$i]['inline'] = true;
                 }
                 foreach ($entry->normalizeFileInfo($files) as $F) {
+                    if (!empty($F['inline'])
+                            && (!isset($F['file']) || !$F['file']->isInlineSafeImage()))
+                        $F['inline'] = false;
+
                     // Deduplicate on the `key` attribute. The key is
                     // necessary for the CID rewrite below
                     $attached_files[$F['key']] = $F;
@@ -1769,6 +1773,9 @@ implements TemplateVariable {
         // discarded, only the unique hash-code (key) will be available to
         // retrieve the image later
         foreach ($attached_files as $key => $a) {
+            if (empty($a['inline']))
+                continue;
+
             if (isset($a['cid']) && $a['cid']) {
                 $body = preg_replace('/src=("|\'|\b)(?:cid:)?'
                     . preg_quote($a['cid'], '/').'\1/i',