SVG upload fail: The doctype must not reference external files

Hello,

I’m getting the following error when attempting to upload any SVG image via the panel:
Untitled

The error is produced by kirby/src/Sane/Xml.php at function validateDoctype. Any ideas?

Kirby version 3.5.4.

3.5.4 was a security release that validates SVG files for potentially harmful content, see details Release 3.5.4 · getkirby/kirby · GitHub

I see. So is it possible to upload SVG images at all from now on?

If I export an empty SVG image from Affinity Designer, it produces the following file:

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg width="800px" height="600px" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" xml:space="preserve" xmlns:serif="http://www.serif.com/" style="fill-rule:evenodd;clip-rule:evenodd;stroke-linejoin:round;stroke-miterlimit:2;">
</svg>

The error message seems to reject the uri of the DOCTYPE, if I’m not mistaken. Is this the intended way to respond to the vulnerability?

Try this instead

<svg width="800" height="600" xmlns="http://www.w3.org/2000/svg" fill-rule="evenodd" clip-rule="evenodd" stroke-linejoin="round" stroke-miterlimit="2"/>

I ran the file through SVGOMG which strips out that stuff. You can do this if you install SVGO locally and use ImageOptim (assuming you are on a mac) which will allow you to batch convert.

Thanks for the workarounds, but I can easily circumvent the problem by just placing the files in folders, since I’m an admin. My concern is that the other panel editors won’t be able to upload SVGs any longer. All panel editors in my team are trusted, so in our case it’s not really necessary to have this security feature.

The original issue ticket mentions script tags, but the error message doesn’t mention them (instead it mentions doctype, which is pretty standard tag in SVG image files). So it’s a bit unclear what is the situation with Kirby and SVG files. Is there going to be a built-in parser, which allows uploading SVGs and removes dangerous tags? Is the doctype tag dangerous? Are SVG file uploads going to be permitted again ever?

Looks as if there are ways to customize the validation, see this thread about GPX files: Error uploading standard Garmin GPX tracks · Issue #3433 · getkirby/kirby · GitHub

Although the recommendation is to run the files through svgomg or similar tools as @jimbobrjames already wrote above.

Ping @lukasbestle!

<script> tags are only one of many possible attacks that are possible with SVGs. SVG and XML are both quite complex languages that allow different kinds of dynamic imports. Because the reason for our Sane SVG and XML validators is security in the sense of XSS protection, we wanted to ensure that our implementation filters out every property that could be used for an attack and that isn’t needed in most legit SVG files.

Unfortunately many design applications like Affinity Designer, Illustrator and Inkscape use all sorts of legacy SVG syntax for compatibility with the oldest SVG parsers out there. All this syntax isn’t needed in modern browsers and only blows up the file size. Your empty image as exported from Affinity Designer is a good example for this.

I understand that in your case all editors are trusted. If you do want to completely disable our Sane validator, you can use the following in your config.php. Please note though that we do not recommend this for most sites for the security and SVG code-cleanliness reasons I mentioned:

Kirby\Sane\Sane::$aliases  = [];
Kirby\Sane\Sane::$handlers = [];

An alternative could be to use plugins or options in your design software to export clean SVGs in the first place. Then you can keep the security feature enabled and will get smaller files as well. I don’t have experience with Affinity Designer personally, so I don’t know if it already comes with a mode that does this. Alternatively you can use tools like SVGO/SVGOMG as already mentioned by James.

Regarding your other questions:

We would love to have that in the future. Implementing that in a secure way is a large task however, so I can’t promise we will get to it in the near future.

Depending on the XML parser it can be. Your example contains an external DTD (document type) definition. If the XML parser downloads that to verify the document structure and an attacker manages to build a DTD that exploits a vulnerability in the XML parser, it can have security implications.

SVG uploads are permitted, but since Kirby 3.5.4 they are validated. Clean SVGs are always permitted.

3 Likes

Thank you for the comprehensive answer, this is clear now.

Is there a solution for inkscape for the svg “problem”?

I’m sure you’ll find something with a google search: inkscape clean svgs

I can’t expect my editors to adapt the xml of a svg before the upload works. where can I find information about

Kirby\Sane\Sane::$aliases = [];
Kirby\Sane\Sane::$handlers = [];

?

As @lukasbestle wrote, you can disable the validators completely by adding that code to your config or in a plugin’s index.php.

Kirby\Sane\Sane::$aliases  = [];
Kirby\Sane\Sane::$handlers = [];
2 Likes

This produces error such as this:

Regering to this GitHub thread:

This snippet from @lucasbestle should be better (only specifically disables the Sane validator for svg files)

unset(Kirby\Sane\Sane::$handlers['svg']);
unset(Kirby\Sane\Sane::$aliases['image/svg']);
unset(Kirby\Sane\Sane::$aliases['image/svg+xml']);

Example in the site/config.php file:

1 Like