CVE-2022-3355: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site Scripting (XSS) - Stored in GitHub repository inventree/inventree prior to 0.8.3.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-09-29T10:15Z
Repository: https://github.com/inventree/inventree
Language: Python
Files changed: InvenTree/InvenTree/models.py, InvenTree/InvenTree/sanitizer.py, InvenTree/InvenTree/tests.py
Commit date: 2022-09-23 16:56:29 +1000
Commit message: 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
Diff: diff --git a/InvenTree/InvenTree/models.py b/InvenTree/InvenTree/models.py
index 5a7491b61..04b9c14be 100644
--- a/InvenTree/InvenTree/models.py
+++ b/InvenTree/InvenTree/models.py
@@ -2,10 +2,11 @@
 
 import logging
 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
 from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
@@ -22,10 +23,11 @@ from mptt.models import MPTTModel, TreeForeignKey
 
 import InvenTree.format
 import InvenTree.helpers
 from common.models import InvenTreeSetting
 from InvenTree.fields import InvenTreeURLField
+from InvenTree.sanitizer import sanitize_svg
 
 logger = logging.getLogger('inventree')
 
 
 def rename_attachment(instance, filename):
@@ -381,12 +383,20 @@ class InvenTreeAttachment(models.Model):
             raise ValidationError({
                 'attachment': _('Missing file'),
                 '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:
             return os.path.basename(self.attachment.name)
         else:
diff --git a/InvenTree/InvenTree/sanitizer.py b/InvenTree/InvenTree/sanitizer.py
new file mode 100644
index 000000000..a9f1fd2f3
--- /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 1929ba132..5cf07df63 100644
--- a/InvenTree/InvenTree/tests.py
+++ b/InvenTree/InvenTree/tests.py
@@ -21,10 +21,11 @@ from djmoney.money import Money
 import InvenTree.format
 import InvenTree.helpers
 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
 
 from . import config, helpers, ready, status, version
 from .tasks import offload_task
@@ -876,5 +877,22 @@ class BarcodeMixinTest(helpers.InvenTreeTestCase):
             '{"part": 17, "stockitem": 12}': 'c88c11ed0628eb7fef0d59b098b96975',
         }
 
         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 = """<svg xmlns="http://www.w3.org/2000/svg" version="1.1" id="svg2" height="400" width="400">{0}
+        <path id="path1" d="m -151.78571,359.62883 v 112.76373 l 97.068507,-56.04253 V 303.14815 Z" style="fill:#ddbc91;"></path>
+        </svg>"""
+        dangerous_string = valid_string.format('<script>alert();</script>')
+
+        # 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))

Fixed code: from io import BytesIO
from InvenTree.sanitizer import sanitize_svg
        if self.attachment.name.lower().endswith('.svg'):
            self.attachment.file.file = self.clean_svg(self.attachment)

    def clean_svg(self, field):
        """Sanitize SVG file before saving."""
        cleaned = sanitize_svg(field.file.read())
        return BytesIO(bytes(cleaned, 'utf8'))

"""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
from InvenTree.sanitizer import sanitize_svg


class SanitizerTest(TestCase):
    """Simple tests for sanitizer functions."""

    def test_svg_sanitizer(self):
        """Test that SVGs are sanitized acordingly."""
        valid_string = """<svg xmlns="http://www.w3.org/2000/svg" version="1.1" id="svg2" height="400" width="400">{0}
        <path id="path1" d="m -151.78571,359.62883 v 112.76373 l 97.068507,-56.04253 V 303.14815 Z" style="fill:#ddbc91;"></path>
        </svg>"""
        dangerous_string = valid_string.format('<script>alert();</script>')

        # 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))