mirror of
https://github.com/inventree/InvenTree.git
synced 2025-04-28 11:36:44 +00:00
Merge commit from fork
* Sanitize markdown when rendering notes fields * Update helpers.py * Update mixins.py * format * format * Allow horizontal rule in markdown * Display returned error mesage * More instructive error msg * Specify output_format to markdown.markdown Ref: https://python-markdown.github.io/reference/markdown/serializers/ * Cleanup * Adjust allowable markdown tags * Add unit test for malicious markdown XSS --------- Co-authored-by: Matthias Mair <code@mjmair.com>
This commit is contained in:
parent
a323bf0007
commit
846b17aa1d
@ -802,6 +802,40 @@ def remove_non_printable_characters(
|
|||||||
return cleaned
|
return cleaned
|
||||||
|
|
||||||
|
|
||||||
|
def clean_markdown(value: str):
|
||||||
|
"""Clean a markdown string.
|
||||||
|
|
||||||
|
This function will remove javascript and other potentially harmful content from the markdown string.
|
||||||
|
"""
|
||||||
|
import markdown
|
||||||
|
from markdownify.templatetags.markdownify import markdownify
|
||||||
|
|
||||||
|
try:
|
||||||
|
markdownify_settings = settings.MARKDOWNIFY['default']
|
||||||
|
except (AttributeError, KeyError):
|
||||||
|
markdownify_settings = {}
|
||||||
|
|
||||||
|
extensions = markdownify_settings.get('MARKDOWN_EXTENSIONS', [])
|
||||||
|
extension_configs = markdownify_settings.get('MARKDOWN_EXTENSION_CONFIGS', {})
|
||||||
|
|
||||||
|
# Generate raw HTML from provided markdown (without sanitizing)
|
||||||
|
# Note: The 'html' output_format is required to generate self closing tags, e.g. <tag> instead of <tag />
|
||||||
|
html = markdown.markdown(
|
||||||
|
value or '',
|
||||||
|
extensions=extensions,
|
||||||
|
extension_configs=extension_configs,
|
||||||
|
output_format='html',
|
||||||
|
)
|
||||||
|
|
||||||
|
# Clean the HTML content (for comparison). Ideally, this should be the same as the original content
|
||||||
|
clean_html = markdownify(value)
|
||||||
|
|
||||||
|
if html != clean_html:
|
||||||
|
raise ValidationError(_('Data contains prohibited markdown content'))
|
||||||
|
|
||||||
|
return value
|
||||||
|
|
||||||
|
|
||||||
def hash_barcode(barcode_data):
|
def hash_barcode(barcode_data):
|
||||||
"""Calculate a 'unique' hash for a barcode string.
|
"""Calculate a 'unique' hash for a barcode string.
|
||||||
|
|
||||||
|
@ -6,7 +6,11 @@ from rest_framework import generics, mixins, status
|
|||||||
from rest_framework.response import Response
|
from rest_framework.response import Response
|
||||||
|
|
||||||
from InvenTree.fields import InvenTreeNotesField
|
from InvenTree.fields import InvenTreeNotesField
|
||||||
from InvenTree.helpers import remove_non_printable_characters, strip_html_tags
|
from InvenTree.helpers import (
|
||||||
|
clean_markdown,
|
||||||
|
remove_non_printable_characters,
|
||||||
|
strip_html_tags,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class CleanMixin:
|
class CleanMixin:
|
||||||
@ -57,6 +61,7 @@ class CleanMixin:
|
|||||||
|
|
||||||
# By default, newline characters are removed
|
# By default, newline characters are removed
|
||||||
remove_newline = True
|
remove_newline = True
|
||||||
|
is_markdown = False
|
||||||
|
|
||||||
try:
|
try:
|
||||||
if hasattr(self, 'serializer_class'):
|
if hasattr(self, 'serializer_class'):
|
||||||
@ -64,11 +69,12 @@ class CleanMixin:
|
|||||||
field = model._meta.get_field(field)
|
field = model._meta.get_field(field)
|
||||||
|
|
||||||
# The following field types allow newline characters
|
# The following field types allow newline characters
|
||||||
allow_newline = [InvenTreeNotesField]
|
allow_newline = [(InvenTreeNotesField, True)]
|
||||||
|
|
||||||
for field_type in allow_newline:
|
for field_type in allow_newline:
|
||||||
if issubclass(type(field), field_type):
|
if issubclass(type(field), field_type[0]):
|
||||||
remove_newline = False
|
remove_newline = False
|
||||||
|
is_markdown = field_type[1]
|
||||||
break
|
break
|
||||||
|
|
||||||
except AttributeError:
|
except AttributeError:
|
||||||
@ -80,6 +86,9 @@ class CleanMixin:
|
|||||||
cleaned, remove_newline=remove_newline
|
cleaned, remove_newline=remove_newline
|
||||||
)
|
)
|
||||||
|
|
||||||
|
if is_markdown:
|
||||||
|
cleaned = clean_markdown(cleaned)
|
||||||
|
|
||||||
return cleaned
|
return cleaned
|
||||||
|
|
||||||
def clean_data(self, data: dict) -> dict:
|
def clean_data(self, data: dict) -> dict:
|
||||||
|
@ -1264,23 +1264,28 @@ MARKDOWNIFY = {
|
|||||||
'abbr',
|
'abbr',
|
||||||
'b',
|
'b',
|
||||||
'blockquote',
|
'blockquote',
|
||||||
|
'code',
|
||||||
'em',
|
'em',
|
||||||
'h1',
|
'h1',
|
||||||
'h2',
|
'h2',
|
||||||
'h3',
|
'h3',
|
||||||
|
'h4',
|
||||||
|
'h5',
|
||||||
|
'hr',
|
||||||
'i',
|
'i',
|
||||||
'img',
|
'img',
|
||||||
'li',
|
'li',
|
||||||
'ol',
|
'ol',
|
||||||
'p',
|
'p',
|
||||||
|
's',
|
||||||
'strong',
|
'strong',
|
||||||
'ul',
|
|
||||||
'table',
|
'table',
|
||||||
'thead',
|
'thead',
|
||||||
'tbody',
|
'tbody',
|
||||||
'th',
|
'th',
|
||||||
'tr',
|
'tr',
|
||||||
'td',
|
'td',
|
||||||
|
'ul',
|
||||||
],
|
],
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
3
src/backend/InvenTree/InvenTree/static/script/purify.min.js
vendored
Normal file
3
src/backend/InvenTree/InvenTree/static/script/purify.min.js
vendored
Normal file
File diff suppressed because one or more lines are too long
@ -154,6 +154,47 @@ class CompanyTest(InvenTreeAPITestCase):
|
|||||||
len(self.get(url, data={'active': False}, expected_code=200).data), 1
|
len(self.get(url, data={'active': False}, expected_code=200).data), 1
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_company_notes(self):
|
||||||
|
"""Test the markdown 'notes' field for the Company model."""
|
||||||
|
pk = Company.objects.first().pk
|
||||||
|
|
||||||
|
# Attempt to inject malicious markdown into the "notes" field
|
||||||
|
xss = [
|
||||||
|
'[Click me](javascript:alert(123))',
|
||||||
|
')',
|
||||||
|
')',
|
||||||
|
]
|
||||||
|
|
||||||
|
for note in xss:
|
||||||
|
response = self.patch(
|
||||||
|
reverse('api-company-detail', kwargs={'pk': pk}),
|
||||||
|
{'notes': note},
|
||||||
|
expected_code=400,
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertIn(
|
||||||
|
'Data contains prohibited markdown content', str(response.data)
|
||||||
|
)
|
||||||
|
|
||||||
|
# The following markdown is safe, and should be accepted
|
||||||
|
good = [
|
||||||
|
'This is a **bold** statement',
|
||||||
|
'This is a *italic* statement',
|
||||||
|
'This is a [link](https://www.google.com)',
|
||||||
|
'This is an ',
|
||||||
|
'This is a `code` block',
|
||||||
|
'This text has ~~strikethrough~~ formatting',
|
||||||
|
]
|
||||||
|
|
||||||
|
for note in good:
|
||||||
|
response = self.patch(
|
||||||
|
reverse('api-company-detail', kwargs={'pk': pk}),
|
||||||
|
{'notes': note},
|
||||||
|
expected_code=200,
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertEqual(response.data['notes'], note)
|
||||||
|
|
||||||
|
|
||||||
class ContactTest(InvenTreeAPITestCase):
|
class ContactTest(InvenTreeAPITestCase):
|
||||||
"""Tests for the Contact models."""
|
"""Tests for the Contact models."""
|
||||||
|
@ -498,6 +498,11 @@ function setupNotesField(element, url, options={}) {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
|
renderingConfig: {
|
||||||
|
sanitizerFunction: function (html) {
|
||||||
|
return DOMPurify.sanitize(html);
|
||||||
|
}
|
||||||
|
},
|
||||||
shortcuts: [],
|
shortcuts: [],
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -36,3 +36,4 @@
|
|||||||
<script defer type='text/javascript' src="{% static 'script/randomColor.min.js' %}"></script>
|
<script defer type='text/javascript' src="{% static 'script/randomColor.min.js' %}"></script>
|
||||||
<script defer type='text/javascript' src="{% static 'script/html5-qrcode.min.js' %}"></script>
|
<script defer type='text/javascript' src="{% static 'script/html5-qrcode.min.js' %}"></script>
|
||||||
<script defer type='text/javascript' src="{% static 'script/qrcode.min.js' %}"></script>
|
<script defer type='text/javascript' src="{% static 'script/qrcode.min.js' %}"></script>
|
||||||
|
<script defer type='text/javascript' src="{% static 'script/purify.min.js' %}"></script>
|
||||||
|
@ -48,6 +48,7 @@
|
|||||||
"clsx": "^2.1.1",
|
"clsx": "^2.1.1",
|
||||||
"codemirror": "^6.0.1",
|
"codemirror": "^6.0.1",
|
||||||
"dayjs": "^1.11.13",
|
"dayjs": "^1.11.13",
|
||||||
|
"dompurify": "^3.1.7",
|
||||||
"easymde": "^2.18.0",
|
"easymde": "^2.18.0",
|
||||||
"embla-carousel-react": "^8.3.0",
|
"embla-carousel-react": "^8.3.0",
|
||||||
"fuse.js": "^7.0.0",
|
"fuse.js": "^7.0.0",
|
||||||
|
@ -1,6 +1,7 @@
|
|||||||
import { t } from '@lingui/macro';
|
import { t } from '@lingui/macro';
|
||||||
import { notifications } from '@mantine/notifications';
|
import { notifications } from '@mantine/notifications';
|
||||||
import { useQuery } from '@tanstack/react-query';
|
import { useQuery } from '@tanstack/react-query';
|
||||||
|
import DOMPurify from 'dompurify';
|
||||||
import EasyMDE, { default as SimpleMde } from 'easymde';
|
import EasyMDE, { default as SimpleMde } from 'easymde';
|
||||||
import 'easymde/dist/easymde.min.css';
|
import 'easymde/dist/easymde.min.css';
|
||||||
import { useCallback, useEffect, useMemo, useState } from 'react';
|
import { useCallback, useEffect, useMemo, useState } from 'react';
|
||||||
@ -120,11 +121,16 @@ export default function NotesEditor({
|
|||||||
id: 'notes'
|
id: 'notes'
|
||||||
});
|
});
|
||||||
})
|
})
|
||||||
.catch(() => {
|
.catch((error) => {
|
||||||
notifications.hide('notes');
|
notifications.hide('notes');
|
||||||
|
|
||||||
|
let msg =
|
||||||
|
error?.response?.data?.non_field_errors[0] ??
|
||||||
|
t`Failed to save notes`;
|
||||||
|
|
||||||
notifications.show({
|
notifications.show({
|
||||||
title: t`Error`,
|
title: t`Error Saving Notes`,
|
||||||
message: t`Failed to save notes`,
|
message: msg,
|
||||||
color: 'red',
|
color: 'red',
|
||||||
id: 'notes'
|
id: 'notes'
|
||||||
});
|
});
|
||||||
@ -181,6 +187,11 @@ export default function NotesEditor({
|
|||||||
uploadImage: true,
|
uploadImage: true,
|
||||||
imagePathAbsolute: true,
|
imagePathAbsolute: true,
|
||||||
imageUploadFunction: imageUploadHandler,
|
imageUploadFunction: imageUploadHandler,
|
||||||
|
renderingConfig: {
|
||||||
|
sanitizerFunction: (html: string) => {
|
||||||
|
return DOMPurify.sanitize(html);
|
||||||
|
}
|
||||||
|
},
|
||||||
sideBySideFullscreen: false,
|
sideBySideFullscreen: false,
|
||||||
shortcuts: {},
|
shortcuts: {},
|
||||||
spellChecker: false
|
spellChecker: false
|
||||||
|
@ -2578,6 +2578,11 @@ dom-helpers@^5.0.1:
|
|||||||
"@babel/runtime" "^7.8.7"
|
"@babel/runtime" "^7.8.7"
|
||||||
csstype "^3.0.2"
|
csstype "^3.0.2"
|
||||||
|
|
||||||
|
dompurify@^3.1.7:
|
||||||
|
version "3.1.7"
|
||||||
|
resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-3.1.7.tgz#711a8c96479fb6ced93453732c160c3c72418a6a"
|
||||||
|
integrity sha512-VaTstWtsneJY8xzy7DekmYWEOZcmzIe3Qb3zPd4STve1OBTa+e+WmS1ITQec1fZYXI3HCsOZZiSMpG6oxoWMWQ==
|
||||||
|
|
||||||
easymde@^2.18.0:
|
easymde@^2.18.0:
|
||||||
version "2.18.0"
|
version "2.18.0"
|
||||||
resolved "https://registry.yarnpkg.com/easymde/-/easymde-2.18.0.tgz#ff1397d07329b1a7b9187d2d0c20766fa16b3b1b"
|
resolved "https://registry.yarnpkg.com/easymde/-/easymde-2.18.0.tgz#ff1397d07329b1a7b9187d2d0c20766fa16b3b1b"
|
||||||
|
Loading…
x
Reference in New Issue
Block a user