Strip potentially dangerous tags from SVG files (#12687)

src/backend/InvenTree/InvenTree/sanitizer.py

@@ -64,10 +64,6 @@ _SVG_ALLOWED_CSS_PROPERTIES = frozenset([
 
 ALLOWED_ELEMENTS_SVG = [
     'a',
-    'animate',
-    'animateColor',
-    'animateMotion',
-    'animateTransform',
     'circle',
     'defs',
     'desc',
@@ -83,13 +79,11 @@ ALLOWED_ELEMENTS_SVG = [
     'marker',
     'metadata',
     'missing-glyph',
-    'mpath',
     'path',
     'polygon',
     'polyline',
     'radialGradient',
     'rect',
-    'set',
     'stop',
     'svg',
     'switch',
@@ -98,21 +92,20 @@ ALLOWED_ELEMENTS_SVG = [
     'tspan',
     'use',
 ]
+# SMIL animation elements ('animate', 'set', etc.) are intentionally excluded: nh3's
+# URL-scheme filtering (e.g. stripping `javascript:` from `href`/`xlink:href`) is only
+# applied to attributes it recognises as URL-bearing on the element that declares them.
+# It does not recognise the `to`/`from`/`values` attributes of animation elements as
+# URL-setting, so a `javascript:` URL placed there survives sanitization and is assigned
+# to a target element's `href` at render time, bypassing the URL sanitization entirely.
 
 ALLOWED_ATTRIBUTES_SVG = [
     'accent-height',
-    'accumulate',
-    'additive',
     'alphabetic',
     'arabic-form',
     'ascent',
-    'attributeName',
-    'attributeType',
     'baseProfile',
     'bbox',
-    'begin',
-    'by',
-    'calcMode',
     'cap-height',
     'class',
     'color',
@@ -125,8 +118,6 @@ ALLOWED_ATTRIBUTES_SVG = [
     'dy',
     'descent',
     'display',
-    'dur',
-    'end',
     'fill',
     'fill-opacity',
     'fill-rule',
@@ -136,7 +127,6 @@ ALLOWED_ATTRIBUTES_SVG = [
     'font-style',
     'font-variant',
     'font-weight',
-    'from',
     'fx',
     'fy',
     'g1',
@@ -150,9 +140,6 @@ ALLOWED_ATTRIBUTES_SVG = [
     'id',
     'ideographic',
     'k',
-    'keyPoints',
-    'keySplines',
-    'keyTimes',
     'lang',
     'marker-end',
     'marker-mid',
@@ -161,8 +148,6 @@ ALLOWED_ATTRIBUTES_SVG = [
     'markerUnits',
     'markerWidth',
     'mathematical',
-    'max',
-    'min',
     'name',
     'offset',
     'opacity',
@@ -178,11 +163,8 @@ ALLOWED_ATTRIBUTES_SVG = [
     'r',
     'refX',
     'refY',
-    'repeatCount',
-    'repeatDur',
     'requiredExtensions',
     'requiredFeatures',
-    'restart',
     'rotate',
     'rx',
     'ry',
@@ -204,7 +186,6 @@ ALLOWED_ATTRIBUTES_SVG = [
     'systemLanguage',
     'target',
     'text-anchor',
-    'to',
     'transform',
     'type',
     'u1',
@@ -214,7 +195,6 @@ ALLOWED_ATTRIBUTES_SVG = [
     'unicode',
     'unicode-range',
     'units-per-em',
-    'values',
     'version',
     'viewBox',
     'visibility',

src/backend/InvenTree/InvenTree/tests.py

@@ -1678,6 +1678,27 @@ class SanitizerTest(TestCase):
         # Test that invalid string is cleaned
         self.assertNotEqual(dangerous_string, sanitize_svg(dangerous_string))
 
+    def test_svg_sanitizer_smil_bypass(self):
+        """Test that SMIL animation elements cannot be used to smuggle a javascript: URL.
+
+        A <set>/<animate>/<animateTransform> element can assign a `javascript:` value to
+        another element's `href`/`xlink:href` at render time via its `to`/`from`/`values`
+        attribute. These attributes are not treated as URLs by the sanitizer, so simply
+        stripping `javascript:` from `href`-like attributes is not sufficient - the
+        elements themselves must not be permitted.
+        """
+        malicious_string = """<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+        <a xlink:href="https://example.com">
+        <set attributeName="xlink:href" to="javascript:alert(document.domain)" />
+        <text x="10" y="20">Click me</text>
+        </a>
+        </svg>"""
+
+        cleaned = sanitize_svg(malicious_string)
+
+        self.assertNotIn('javascript:', cleaned)
+        self.assertNotIn('<set', cleaned)
+
 
 class MagicLoginTest(InvenTreeTestCase):
     """Test magic login token generation."""