From 5a08ef908dd5344b4433436a4679d122f7f99e41 Mon Sep 17 00:00:00 2001 From: Matthias Mair Date: Fri, 23 Sep 2022 08:56:29 +0200 Subject: [PATCH] Add sanitation for SVG attachments (#3701) * add svg parser * move svg sanitation out into own file * move allowed elements out * add test for svg sanitation * make allowed elements configureable --- InvenTree/InvenTree/models.py | 10 +++++ InvenTree/InvenTree/sanitizer.py | 67 ++++++++++++++++++++++++++++++++ InvenTree/InvenTree/tests.py | 18 +++++++++ 3 files changed, 95 insertions(+) create mode 100644 InvenTree/InvenTree/sanitizer.py diff --git a/InvenTree/InvenTree/models.py b/InvenTree/InvenTree/models.py index 5a7491b6104..04b9c14bed1 100644 --- a/InvenTree/InvenTree/models.py +++ b/InvenTree/InvenTree/models.py @@ -4,6 +4,7 @@ import os import re from datetime import datetime +from io import BytesIO from django.conf import settings from django.contrib.auth import get_user_model @@ -24,6 +25,7 @@ import InvenTree.helpers from common.models import InvenTreeSetting from InvenTree.fields import InvenTreeURLField +from InvenTree.sanitizer import sanitize_svg logger = logging.getLogger('inventree') @@ -383,8 +385,16 @@ def save(self, *args, **kwargs): 'link': _('Missing external link'), }) + if self.attachment.name.lower().endswith('.svg'): + self.attachment.file.file = self.clean_svg(self.attachment) + super().save(*args, **kwargs) + def clean_svg(self, field): + """Sanitize SVG file before saving.""" + cleaned = sanitize_svg(field.file.read()) + return BytesIO(bytes(cleaned, 'utf8')) + def __str__(self): """Human name for attachment.""" if self.attachment is not None: diff --git a/InvenTree/InvenTree/sanitizer.py b/InvenTree/InvenTree/sanitizer.py new file mode 100644 index 00000000000..a9f1fd2f3d9 --- /dev/null +++ b/InvenTree/InvenTree/sanitizer.py @@ -0,0 +1,67 @@ +"""Functions to sanitize user input files.""" +from bleach import clean +from bleach.css_sanitizer import CSSSanitizer + +ALLOWED_ELEMENTS_SVG = [ + 'a', 'animate', 'animateColor', 'animateMotion', + 'animateTransform', 'circle', 'defs', 'desc', 'ellipse', 'font-face', + 'font-face-name', 'font-face-src', 'g', 'glyph', 'hkern', + 'linearGradient', 'line', 'marker', 'metadata', 'missing-glyph', + 'mpath', 'path', 'polygon', 'polyline', 'radialGradient', 'rect', + 'set', 'stop', 'svg', 'switch', 'text', 'title', 'tspan', 'use' +] + +ALLOWED_ATTRIBUTES_SVG = [ + 'accent-height', 'accumulate', 'additive', 'alphabetic', + 'arabic-form', 'ascent', 'attributeName', 'attributeType', + 'baseProfile', 'bbox', 'begin', 'by', 'calcMode', 'cap-height', + 'class', 'color', 'color-rendering', 'content', 'cx', 'cy', 'd', 'dx', + 'dy', 'descent', 'display', 'dur', 'end', 'fill', 'fill-opacity', + 'fill-rule', 'font-family', 'font-size', 'font-stretch', 'font-style', + 'font-variant', 'font-weight', 'from', 'fx', 'fy', 'g1', 'g2', + 'glyph-name', 'gradientUnits', 'hanging', 'height', 'horiz-adv-x', + 'horiz-origin-x', 'id', 'ideographic', 'k', 'keyPoints', + 'keySplines', 'keyTimes', 'lang', 'marker-end', 'marker-mid', + 'marker-start', 'markerHeight', 'markerUnits', 'markerWidth', + 'mathematical', 'max', 'min', 'name', 'offset', 'opacity', 'orient', + 'origin', 'overline-position', 'overline-thickness', 'panose-1', + 'path', 'pathLength', 'points', 'preserveAspectRatio', 'r', 'refX', + 'refY', 'repeatCount', 'repeatDur', 'requiredExtensions', + 'requiredFeatures', 'restart', 'rotate', 'rx', 'ry', 'slope', + 'stemh', 'stemv', 'stop-color', 'stop-opacity', + 'strikethrough-position', 'strikethrough-thickness', 'stroke', + 'stroke-dasharray', 'stroke-dashoffset', 'stroke-linecap', + 'stroke-linejoin', 'stroke-miterlimit', 'stroke-opacity', + 'stroke-width', 'systemLanguage', 'target', 'text-anchor', 'to', + 'transform', 'type', 'u1', 'u2', 'underline-position', + 'underline-thickness', 'unicode', 'unicode-range', 'units-per-em', + 'values', 'version', 'viewBox', 'visibility', 'width', 'widths', 'x', + 'x-height', 'x1', 'x2', 'xlink:actuate', 'xlink:arcrole', + 'xlink:href', 'xlink:role', 'xlink:show', 'xlink:title', + 'xlink:type', 'xml:base', 'xml:lang', 'xml:space', 'xmlns', + 'xmlns:xlink', 'y', 'y1', 'y2', 'zoomAndPan', 'style' +] + + +def sanitize_svg(file_data: str, strip: bool = True, elements: str = ALLOWED_ELEMENTS_SVG, attributes: str = ALLOWED_ATTRIBUTES_SVG) -> str: + """Sanatize a SVG file. + + Args: + file_data (str): SVG as string. + strip (bool, optional): Should invalid elements get removed. Defaults to True. + elements (str, optional): Allowed elements. Defaults to ALLOWED_ELEMENTS_SVG. + attributes (str, optional): Allowed attributes. Defaults to ALLOWED_ATTRIBUTES_SVG. + + Returns: + str: Sanitzied SVG file. + """ + + cleaned = clean( + file_data, + tags=elements, + attributes=attributes, + strip=strip, + strip_comments=strip, + css_sanitizer=CSSSanitizer() + ) + return cleaned diff --git a/InvenTree/InvenTree/tests.py b/InvenTree/InvenTree/tests.py index 1929ba1325e..5cf07df637f 100644 --- a/InvenTree/InvenTree/tests.py +++ b/InvenTree/InvenTree/tests.py @@ -23,6 +23,7 @@ import InvenTree.tasks from common.models import InvenTreeSetting from common.settings import currency_codes +from InvenTree.sanitizer import sanitize_svg from part.models import Part, PartCategory from stock.models import StockItem, StockLocation @@ -878,3 +879,20 @@ def test_bacode_hash(self): for barcode, hash in hashing_tests.items(): self.assertEqual(InvenTree.helpers.hash_barcode(barcode), hash) + + +class SanitizerTest(TestCase): + """Simple tests for sanitizer functions.""" + + def test_svg_sanitizer(self): + """Test that SVGs are sanitized acordingly.""" + valid_string = """{0} + + """ + dangerous_string = valid_string.format('') + + # Test that valid string + self.assertEqual(valid_string, sanitize_svg(valid_string)) + + # Test that invalid string is cleanded + self.assertNotEqual(dangerous_string, sanitize_svg(dangerous_string))