1
0
mirror of https://github.com/django/django.git synced 2025-06-03 18:49:12 +00:00

Fixed CVE-2024-53907 -- Mitigated potential DoS in strip_tags().

Thanks to jiangniao for the report, and Shai Berger and Natalia Bidart
for the reviews.
This commit is contained in:
Sarah Boyce 2024-11-13 15:06:23 +01:00
parent 58e548db8b
commit 49ff1042aa
5 changed files with 63 additions and 2 deletions

View File

@ -8,6 +8,7 @@ from collections.abc import Mapping
from html.parser import HTMLParser from html.parser import HTMLParser
from urllib.parse import parse_qsl, quote, unquote, urlencode, urlsplit, urlunsplit from urllib.parse import parse_qsl, quote, unquote, urlencode, urlsplit, urlunsplit
from django.core.exceptions import SuspiciousOperation
from django.utils.deprecation import RemovedInDjango60Warning from django.utils.deprecation import RemovedInDjango60Warning
from django.utils.encoding import punycode from django.utils.encoding import punycode
from django.utils.functional import Promise, cached_property, keep_lazy, keep_lazy_text from django.utils.functional import Promise, cached_property, keep_lazy, keep_lazy_text
@ -40,6 +41,7 @@ VOID_ELEMENTS = frozenset(
) )
MAX_URL_LENGTH = 2048 MAX_URL_LENGTH = 2048
MAX_STRIP_TAGS_DEPTH = 50
@keep_lazy(SafeString) @keep_lazy(SafeString)
@ -211,15 +213,19 @@ def _strip_once(value):
@keep_lazy_text @keep_lazy_text
def strip_tags(value): def strip_tags(value):
"""Return the given HTML with all tags stripped.""" """Return the given HTML with all tags stripped."""
# Note: in typical case this loop executes _strip_once once. Loop condition
# is redundant, but helps to reduce number of executions of _strip_once.
value = str(value) value = str(value)
# Note: in typical case this loop executes _strip_once twice (the second
# execution does not remove any more tags).
strip_tags_depth = 0
while "<" in value and ">" in value: while "<" in value and ">" in value:
if strip_tags_depth >= MAX_STRIP_TAGS_DEPTH:
raise SuspiciousOperation
new_value = _strip_once(value) new_value = _strip_once(value)
if value.count("<") == new_value.count("<"): if value.count("<") == new_value.count("<"):
# _strip_once wasn't able to detect more tags. # _strip_once wasn't able to detect more tags.
break break
value = new_value value = new_value
strip_tags_depth += 1
return value return value

View File

@ -6,3 +6,19 @@ Django 4.2.17 release notes
Django 4.2.17 fixes one security issue with severity "high" and one security Django 4.2.17 fixes one security issue with severity "high" and one security
issue with severity "moderate" in 4.2.16. issue with severity "moderate" in 4.2.16.
CVE-2024-53907: Denial-of-service possibility in ``strip_tags()``
=================================================================
:func:`~django.utils.html.strip_tags` would be extremely slow to evaluate
certain inputs containing large sequences of nested incomplete HTML entities.
The ``strip_tags()`` method is used to implement the corresponding
:tfilter:`striptags` template filter, which was thus also vulnerable.
``strip_tags()`` now has an upper limit of recursive calls to ``HTMLParser``
before raising a :exc:`.SuspiciousOperation` exception.
Remember that absolutely NO guarantee is provided about the results of
``strip_tags()`` being HTML safe. So NEVER mark safe the result of a
``strip_tags()`` call without escaping it first, for example with
:func:`django.utils.html.escape`.

View File

@ -6,3 +6,19 @@ Django 5.0.10 release notes
Django 5.0.10 fixes one security issue with severity "high" and one security Django 5.0.10 fixes one security issue with severity "high" and one security
issue with severity "moderate" in 5.0.9. issue with severity "moderate" in 5.0.9.
CVE-2024-53907: Denial-of-service possibility in ``strip_tags()``
=================================================================
:func:`~django.utils.html.strip_tags` would be extremely slow to evaluate
certain inputs containing large sequences of nested incomplete HTML entities.
The ``strip_tags()`` method is used to implement the corresponding
:tfilter:`striptags` template filter, which was thus also vulnerable.
``strip_tags()`` now has an upper limit of recursive calls to ``HTMLParser``
before raising a :exc:`.SuspiciousOperation` exception.
Remember that absolutely NO guarantee is provided about the results of
``strip_tags()`` being HTML safe. So NEVER mark safe the result of a
``strip_tags()`` call without escaping it first, for example with
:func:`django.utils.html.escape`.

View File

@ -7,6 +7,22 @@ Django 5.1.4 release notes
Django 5.1.4 fixes one security issue with severity "high", one security issue Django 5.1.4 fixes one security issue with severity "high", one security issue
with severity "moderate", and several bugs in 5.1.3. with severity "moderate", and several bugs in 5.1.3.
CVE-2024-53907: Denial-of-service possibility in ``strip_tags()``
=================================================================
:func:`~django.utils.html.strip_tags` would be extremely slow to evaluate
certain inputs containing large sequences of nested incomplete HTML entities.
The ``strip_tags()`` method is used to implement the corresponding
:tfilter:`striptags` template filter, which was thus also vulnerable.
``strip_tags()`` now has an upper limit of recursive calls to ``HTMLParser``
before raising a :exc:`.SuspiciousOperation` exception.
Remember that absolutely NO guarantee is provided about the results of
``strip_tags()`` being HTML safe. So NEVER mark safe the result of a
``strip_tags()`` call without escaping it first, for example with
:func:`django.utils.html.escape`.
Bugfixes Bugfixes
======== ========

View File

@ -1,6 +1,7 @@
import os import os
from datetime import datetime from datetime import datetime
from django.core.exceptions import SuspiciousOperation
from django.core.serializers.json import DjangoJSONEncoder from django.core.serializers.json import DjangoJSONEncoder
from django.test import SimpleTestCase from django.test import SimpleTestCase
from django.utils.deprecation import RemovedInDjango60Warning from django.utils.deprecation import RemovedInDjango60Warning
@ -145,12 +146,18 @@ class TestUtilsHtml(SimpleTestCase):
("<script>alert()</script>&h", "alert()h"), ("<script>alert()</script>&h", "alert()h"),
("><!" + ("&" * 16000) + "D", "><!" + ("&" * 16000) + "D"), ("><!" + ("&" * 16000) + "D", "><!" + ("&" * 16000) + "D"),
("X<<<<br>br>br>br>X", "XX"), ("X<<<<br>br>br>br>X", "XX"),
("<" * 50 + "a>" * 50, ""),
) )
for value, output in items: for value, output in items:
with self.subTest(value=value, output=output): with self.subTest(value=value, output=output):
self.check_output(strip_tags, value, output) self.check_output(strip_tags, value, output)
self.check_output(strip_tags, lazystr(value), output) self.check_output(strip_tags, lazystr(value), output)
def test_strip_tags_suspicious_operation(self):
value = "<" * 51 + "a>" * 51, "<a>"
with self.assertRaises(SuspiciousOperation):
strip_tags(value)
def test_strip_tags_files(self): def test_strip_tags_files(self):
# Test with more lengthy content (also catching performance regressions) # Test with more lengthy content (also catching performance regressions)
for filename in ("strip_tags1.html", "strip_tags2.txt"): for filename in ("strip_tags1.html", "strip_tags2.txt"):