Skip to content

Commit 5a08ef9

Browse files
authored
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
1 parent db3d9b5 commit 5a08ef9

File tree

3 files changed

+95
-0
lines changed

3 files changed

+95
-0
lines changed

Diff for: InvenTree/InvenTree/models.py

+10
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import os
55
import re
66
from datetime import datetime
7+
from io import BytesIO
78

89
from django.conf import settings
910
from django.contrib.auth import get_user_model
@@ -24,6 +25,7 @@
2425
import InvenTree.helpers
2526
from common.models import InvenTreeSetting
2627
from InvenTree.fields import InvenTreeURLField
28+
from InvenTree.sanitizer import sanitize_svg
2729

2830
logger = logging.getLogger('inventree')
2931

@@ -383,8 +385,16 @@ def save(self, *args, **kwargs):
383385
'link': _('Missing external link'),
384386
})
385387

388+
if self.attachment.name.lower().endswith('.svg'):
389+
self.attachment.file.file = self.clean_svg(self.attachment)
390+
386391
super().save(*args, **kwargs)
387392

393+
def clean_svg(self, field):
394+
"""Sanitize SVG file before saving."""
395+
cleaned = sanitize_svg(field.file.read())
396+
return BytesIO(bytes(cleaned, 'utf8'))
397+
388398
def __str__(self):
389399
"""Human name for attachment."""
390400
if self.attachment is not None:

Diff for: InvenTree/InvenTree/sanitizer.py

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
"""Functions to sanitize user input files."""
2+
from bleach import clean
3+
from bleach.css_sanitizer import CSSSanitizer
4+
5+
ALLOWED_ELEMENTS_SVG = [
6+
'a', 'animate', 'animateColor', 'animateMotion',
7+
'animateTransform', 'circle', 'defs', 'desc', 'ellipse', 'font-face',
8+
'font-face-name', 'font-face-src', 'g', 'glyph', 'hkern',
9+
'linearGradient', 'line', 'marker', 'metadata', 'missing-glyph',
10+
'mpath', 'path', 'polygon', 'polyline', 'radialGradient', 'rect',
11+
'set', 'stop', 'svg', 'switch', 'text', 'title', 'tspan', 'use'
12+
]
13+
14+
ALLOWED_ATTRIBUTES_SVG = [
15+
'accent-height', 'accumulate', 'additive', 'alphabetic',
16+
'arabic-form', 'ascent', 'attributeName', 'attributeType',
17+
'baseProfile', 'bbox', 'begin', 'by', 'calcMode', 'cap-height',
18+
'class', 'color', 'color-rendering', 'content', 'cx', 'cy', 'd', 'dx',
19+
'dy', 'descent', 'display', 'dur', 'end', 'fill', 'fill-opacity',
20+
'fill-rule', 'font-family', 'font-size', 'font-stretch', 'font-style',
21+
'font-variant', 'font-weight', 'from', 'fx', 'fy', 'g1', 'g2',
22+
'glyph-name', 'gradientUnits', 'hanging', 'height', 'horiz-adv-x',
23+
'horiz-origin-x', 'id', 'ideographic', 'k', 'keyPoints',
24+
'keySplines', 'keyTimes', 'lang', 'marker-end', 'marker-mid',
25+
'marker-start', 'markerHeight', 'markerUnits', 'markerWidth',
26+
'mathematical', 'max', 'min', 'name', 'offset', 'opacity', 'orient',
27+
'origin', 'overline-position', 'overline-thickness', 'panose-1',
28+
'path', 'pathLength', 'points', 'preserveAspectRatio', 'r', 'refX',
29+
'refY', 'repeatCount', 'repeatDur', 'requiredExtensions',
30+
'requiredFeatures', 'restart', 'rotate', 'rx', 'ry', 'slope',
31+
'stemh', 'stemv', 'stop-color', 'stop-opacity',
32+
'strikethrough-position', 'strikethrough-thickness', 'stroke',
33+
'stroke-dasharray', 'stroke-dashoffset', 'stroke-linecap',
34+
'stroke-linejoin', 'stroke-miterlimit', 'stroke-opacity',
35+
'stroke-width', 'systemLanguage', 'target', 'text-anchor', 'to',
36+
'transform', 'type', 'u1', 'u2', 'underline-position',
37+
'underline-thickness', 'unicode', 'unicode-range', 'units-per-em',
38+
'values', 'version', 'viewBox', 'visibility', 'width', 'widths', 'x',
39+
'x-height', 'x1', 'x2', 'xlink:actuate', 'xlink:arcrole',
40+
'xlink:href', 'xlink:role', 'xlink:show', 'xlink:title',
41+
'xlink:type', 'xml:base', 'xml:lang', 'xml:space', 'xmlns',
42+
'xmlns:xlink', 'y', 'y1', 'y2', 'zoomAndPan', 'style'
43+
]
44+
45+
46+
def sanitize_svg(file_data: str, strip: bool = True, elements: str = ALLOWED_ELEMENTS_SVG, attributes: str = ALLOWED_ATTRIBUTES_SVG) -> str:
47+
"""Sanatize a SVG file.
48+
49+
Args:
50+
file_data (str): SVG as string.
51+
strip (bool, optional): Should invalid elements get removed. Defaults to True.
52+
elements (str, optional): Allowed elements. Defaults to ALLOWED_ELEMENTS_SVG.
53+
attributes (str, optional): Allowed attributes. Defaults to ALLOWED_ATTRIBUTES_SVG.
54+
55+
Returns:
56+
str: Sanitzied SVG file.
57+
"""
58+
59+
cleaned = clean(
60+
file_data,
61+
tags=elements,
62+
attributes=attributes,
63+
strip=strip,
64+
strip_comments=strip,
65+
css_sanitizer=CSSSanitizer()
66+
)
67+
return cleaned

Diff for: InvenTree/InvenTree/tests.py

+18
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import InvenTree.tasks
2424
from common.models import InvenTreeSetting
2525
from common.settings import currency_codes
26+
from InvenTree.sanitizer import sanitize_svg
2627
from part.models import Part, PartCategory
2728
from stock.models import StockItem, StockLocation
2829

@@ -878,3 +879,20 @@ def test_bacode_hash(self):
878879

879880
for barcode, hash in hashing_tests.items():
880881
self.assertEqual(InvenTree.helpers.hash_barcode(barcode), hash)
882+
883+
884+
class SanitizerTest(TestCase):
885+
"""Simple tests for sanitizer functions."""
886+
887+
def test_svg_sanitizer(self):
888+
"""Test that SVGs are sanitized acordingly."""
889+
valid_string = """<svg xmlns="http://www.w3.org/2000/svg" version="1.1" id="svg2" height="400" width="400">{0}
890+
<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>
891+
</svg>"""
892+
dangerous_string = valid_string.format('<script>alert();</script>')
893+
894+
# Test that valid string
895+
self.assertEqual(valid_string, sanitize_svg(valid_string))
896+
897+
# Test that invalid string is cleanded
898+
self.assertNotEqual(dangerous_string, sanitize_svg(dangerous_string))

0 commit comments

Comments
 (0)