mirror of
				https://github.com/django/django.git
				synced 2025-10-31 09:41:08 +00:00 
			
		
		
		
	[5.0.x] 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:
		| @@ -7,6 +7,7 @@ import warnings | ||||
| from html.parser import HTMLParser | ||||
| 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.encoding import punycode | ||||
| from django.utils.functional import Promise, cached_property, keep_lazy, keep_lazy_text | ||||
| @@ -37,6 +38,7 @@ VOID_ELEMENTS = { | ||||
| } | ||||
|  | ||||
| MAX_URL_LENGTH = 2048 | ||||
| MAX_STRIP_TAGS_DEPTH = 50 | ||||
|  | ||||
|  | ||||
| @keep_lazy(SafeString) | ||||
| @@ -202,15 +204,19 @@ def _strip_once(value): | ||||
| @keep_lazy_text | ||||
| def strip_tags(value): | ||||
|     """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) | ||||
|     # 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: | ||||
|         if strip_tags_depth >= MAX_STRIP_TAGS_DEPTH: | ||||
|             raise SuspiciousOperation | ||||
|         new_value = _strip_once(value) | ||||
|         if value.count("<") == new_value.count("<"): | ||||
|             # _strip_once wasn't able to detect more tags. | ||||
|             break | ||||
|         value = new_value | ||||
|         strip_tags_depth += 1 | ||||
|     return value | ||||
|  | ||||
|  | ||||
|   | ||||
| @@ -6,3 +6,19 @@ Django 4.2.17 release notes | ||||
|  | ||||
| Django 4.2.17 fixes one security issue with severity "high" and one security | ||||
| 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`. | ||||
|   | ||||
| @@ -6,3 +6,19 @@ Django 5.0.10 release notes | ||||
|  | ||||
| Django 5.0.10 fixes one security issue with severity "high" and one security | ||||
| 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`. | ||||
|   | ||||
| @@ -1,6 +1,7 @@ | ||||
| import os | ||||
| from datetime import datetime | ||||
|  | ||||
| from django.core.exceptions import SuspiciousOperation | ||||
| from django.core.serializers.json import DjangoJSONEncoder | ||||
| from django.test import SimpleTestCase | ||||
| from django.utils.deprecation import RemovedInDjango60Warning | ||||
| @@ -123,12 +124,18 @@ class TestUtilsHtml(SimpleTestCase): | ||||
|             ("<script>alert()</script>&h", "alert()h"), | ||||
|             ("><!" + ("&" * 16000) + "D", "><!" + ("&" * 16000) + "D"), | ||||
|             ("X<<<<br>br>br>br>X", "XX"), | ||||
|             ("<" * 50 + "a>" * 50, ""), | ||||
|         ) | ||||
|         for value, output in items: | ||||
|             with self.subTest(value=value, output=output): | ||||
|                 self.check_output(strip_tags, 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): | ||||
|         # Test with more lengthy content (also catching performance regressions) | ||||
|         for filename in ("strip_tags1.html", "strip_tags2.txt"): | ||||
|   | ||||
		Reference in New Issue
	
	Block a user