We've just added in the SVG Upload Sanitizer module (https://www.drupal.org/project/svg_upload_sanitizer) to handle sanitizing SVGs that are uploaded to sites built using Paragon. This is mainly for SVG media entities, but should work in any field around the site that allows SVGs. Please put together a comprehensive list of test cases and document your findings in this issue.

CommentFileSizeAuthor
#3 bad.svg_.sanitized.txt3.06 KBmoghingold
#3 bad.svg_.txt3.46 KBmoghingold
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

switzern created an issue. See original summary.

moghingold’s picture

Assigned: Unassigned » moghingold
moghingold’s picture

FileSize
3.46 KB
3.06 KB

I tested this module in 2 main ways. First, I wanted to determine if there were any ways to upload SVGs that would bypass the sanitization. Second, I wanted to make sure that sanitized SVG files were actually being stripped of harmful markup.

To test the first, I created a content type that allowed File upload with "svg" as a permitted file type. I then created content and added a known-bad SVG file sourced from the online SVG sanitizing tester. After uploading the file I viewed its source and confirmed that changed had been made to the file, the same changes that are made when using Paragon's built-in SVG upload functionality.

Unfortunately in its current state the library does not pass the second test. The fill="url()" code still permits loading from external sources. SVGs loaded in this way would not be caught by this module and could still contain malicious code of their own. If SVGs being able to fill from external sources is still desired, then the module or underlying library will need to detect SVG files being loaded on-the-fly and sanitize them just-in-time. Since this is likely a substantial development effort, a more sensible best practice would be to do as the SVG Sanitizer Test does and convert external fill URLs to relative paths so they cannot reference content across domains. That should not be too difficult to add since it already does this for <a> tags.

This is the only detected failure of the module's sanitization pass. Invalid xlink:hrefs, invalid XML, and XML tags that are not part of the SVG specification are all correctly identified and removed.

switzern’s picture

Status: Active » Fixed

Thanks so much for the detailed overview. This is just what we needed. I'm going to mark this as fixed and we can focus on contributing a patch to the SVG Upload Sanitizer module in that issue queue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.