Whitelist attribute xml:space not recognised

Bersman commented 2 years ago

We have exported a SVG with Adobe and the sanitizer does not like that. It give the following errors: There are sanitization issues with this SVG file: Suspicious attribute 'space' in line 4 Suspicious attribute 'enable-background' in line 4

Generator is: Adobe Illustrator 19.1.0, SVG Export Plug-In . SVG Version: 6.00 Build 0)

Issue #63 is for the enable-background. But the space attribute is something weird.

Relevant code of the SVG: <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" id="Layer_1" version="1.1" x="0px" y="0px" width="600px" height="600px" viewBox="0 0 600 600" xml:space="preserve">

If you sanitize this the getXmlIssues() function wil return the error above: Suspicious attribute 'space' in line 4

Somehow the code strips xml: Did not found the problem/solution for this.

bembelimen commented 1 year ago

Problem is: https://github.com/darylldoyle/svg-sanitizer/blob/master/src/Sanitizer.php#L369

Change:

$attrName = $element->attributes->item($x)->name

to

$attrName = $element->attributes->item($x)->nodeName;

it solves the problem, I'm unsure about the side effects...

verdy-p commented 1 month ago

Three problems:

The default value of xml:whitepace in XML is xml:whitepace="default", not xml:whitepace="preserve". Currently the sanitizer assumes the reverse and adds xml:whitepace="preserve", which is never needed in any sanitized XVG. And this changes the behavior of parsers, adding lot of whitespace-only text elements visible to parsers.
If we specify xml:whitepace="preserve" in the input, the white spaces present in text element contents is correctly filtered out, and the output does not "need" to generate any newlines or indentation of elements, all is packed in one line. But if there are some other spurious elements (not used in SVG that has useful NO text element in its own schema, except if other non-SVG elements are present and included by using some namespace prefix), these indentation spaces and line breaks are kept (even if these elements have been sanitized and eliminated) even if they are "sane". You should drop them, or possibly indent elements meaningfully.
Finally, when you sanitize elements, if there are no contents, you should not use explicit close tags to ALL elements, but use the self-closed syntax with <element [attributes...]/> instead of <element [attributes...]></element> which is unnecessarily verbose, especially when you've incorrectly assumed xml:whitepace="preserve" which is only meaningful if there are text-elements (never used in valid SVG, but that may appear in the HTML5-embedded SVG that allows mixing non-SVG elements); using the correct default value of xml:whitespace="default", you expect to see possible text-elements, but valid SVG discards them if they are only whitespace, even in HTML5!
SVG only has one defined text-element: the <style>...</style>element. But even there, xml:white-space="preserve" is not needed. You may still output the CSS with all these whitespaces compressed, or with basic newlines at start and end and between selectors { [key:value];* [key:value] }, and inside braces {} at start and end and in the middle between key:value pairs (pairs may be indented according the brace-level). Note also that the CSS in the text-content of <style> elements and inside style="..." attributes of any SVG elements must be sanitized too (they can contain scripts, and unsafe URIs with some dangerous URI schemes, or "bad" external sites if you accept "http:" or "https:").
However "data:" URI schemes are safe, for example to embed bitmap images, provided that these "data:" URI use "safe" media type. Validating and sanitizing the content of these embedded medias may require another recursive parser/sanitizer for each supported media type (after decoding them safely generally from Base64 into a binary object), notably JPEG, GIF, PNG, videos, PDFs, or even another SVG embedded in a "data:" URI. Sanitizing these medias would need their own options (e.g. for privacy, to remove internal tracing tags, or to respect and preserve the author/copyright/licence tags; this is a complex task because these medias may be secured and you may break digital signatures; you may just be able to support a few simple media-types like plain text, SVG, or a subset of GIF, PNG, JPEG features but should signal the presence of unsupported features; such data URIs to medias will generally occur inside the CSS content of the <style> element or style="..." attributes).

So please use xml:whitepace="default" as the default and cleanup the generated code! Indentation and line breaks in CSS text-element and of XML elements is optional and should be an option (a checkbox) but proper cleanup of white spaces in input should still be performed according to the xml:whitepace value (which may change only explicitly inside each XML element).

igor-krein commented 1 month ago

Problem is: https://github.com/darylldoyle/svg-sanitizer/blob/master/src/Sanitizer.php#L369

Change:
$attrName = $element->attributes->item($x)->name
to
$attrName = $element->attributes->item($x)->nodeName;
it solves the problem, I'm unsure about the side effects...

Just fixed this locally, and then realized there may be an opened issue already...

Opened a pull-request, hoping this will be fixed, because currently, there is no way to determine whether the file was sanitized or not.

https://github.com/darylldoyle/svg-sanitizer/pull/102

darylldoyle / svg-sanitizer

Whitelist attribute xml:space not recognised #64