uploading SVG images fails

[pierstitus]

• flagrow/upload extension version: 4.11
• flarum version: 0.1.0-beta.6
• Upload adapter causing issues: local

---

Uploading SVG images fails when they are opened by the image processor. I didn't test it but looking at the code I guess also uploading any image file other that png, jpeg or gif would fail.

I think the following lines should be changed:

src/Listeners/ProcessesImages.php:

--- a/src/Listeners/ProcessesImages.php
+++ b/src/Listeners/ProcessesImages.php
@@ -43,7 +43,7 @@ class ProcessesImages
 
     protected function validateMime($mime)
     {
-        if (Str::startsWith($mime, 'image/')) {
+        if ($mime == 'image/jpeg' || $mime == 'image/png' || $mime == 'image/gif' || $mime == 'image/svg+xml') {
             return true;
         }
 

src/Processors/ImageProcessor.php:

--- a/src/Processors/ImageProcessor.php
+++ b/src/Processors/ImageProcessor.php
@@ -43,7 +43,8 @@ class ImageProcessor implements Processable
      */
     public function process(File &$file, UploadedFile &$upload)
     {
-        if ($upload->getMimeType() != 'image/gif') {
+        $mimeType = $upload->getMimeType();
+        if ($mimeType == 'image/jpeg' || $mimeType == 'image/png') {
             $image = (new ImageManager())->make($upload->getRealPath());
 
             if ($this->settings->get('mustResize')) {

[luceos]

Good point, allowing svg is a must have! Feel free to do a PR (you can even do this with the Github editor).

[jtojnar]

I love SVGs but they also bring wide range of vulnerabilities. When inserted as an image, the contained scripts will not be executed but once the image is uploaded on the server, user just needs to be convinced to visit the link for havoc to be wrought.

[pierstitus]

With great features come vulnerabilities, but that should be decided by the forum admin by allowing the svg mimetype or not. It would be nice though to be able to choose whether images are inserted as images or as links.

[jtojnar]

I agree that admins should have say in this but if flagrow/upload is striving to be a secure software, it should

1. inform the user about possible consequences, and
2. try to mitigate the risk (at least by using SVG sanitizer).

Either way, security minefields should not be enabled by default.

[luceos]

@jtojnar thanks for that link I think it would make for a great optional add-on feature. A solution for now would be to add a settings input field that would allow configuration of mime types being shown as images. Wouldn't that solve this all together?

[jtojnar]

@luceos Showing the SVGs as images (via img tag) is actually safe. The issue occurs when the uploaded file is visited directly.

[luceos]

I've merged the PR but adding SVG sanitizer makes sense.

[sijad]

maybe it can be possible to force some specified extension (like svg) be download via php with Content-Disposition: attachment header. it might be aslo possible achieve same thing using .htaccess for Apache.

[imorland]

Resolved in 1.2.3