From 5f1757142febd95994caa1c0f64c1a0c161982c3 Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Wed, 10 Jul 2024 20:30:12 +0200 Subject: [PATCH] Fixed CVE-2024-41991 -- Prevented potential ReDoS in django.utils.html.urlize() and AdminURLFieldWidget. Thanks Seokchan Yoon for the report. Co-authored-by: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> --- django/contrib/admin/widgets.py | 2 +- django/utils/html.py | 10 ++++++++-- docs/releases/4.2.15.txt | 7 +++++++ docs/releases/5.0.8.txt | 7 +++++++ tests/admin_widgets/tests.py | 7 ++++++- tests/utils_tests/test_html.py | 13 +++++++++++++ 6 files changed, 42 insertions(+), 4 deletions(-) diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index 00e92bf42d..74004a7ba7 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -393,7 +393,7 @@ class AdminURLFieldWidget(forms.URLInput): context["current_label"] = _("Currently:") context["change_label"] = _("Change:") context["widget"]["href"] = ( - smart_urlquote(context["widget"]["value"]) if value else "" + smart_urlquote(context["widget"]["value"]) if url_valid else "" ) context["url_valid"] = url_valid return context diff --git a/django/utils/html.py b/django/utils/html.py index 1123e38f6c..154c820d34 100644 --- a/django/utils/html.py +++ b/django/utils/html.py @@ -38,6 +38,8 @@ VOID_ELEMENTS = frozenset( ) ) +MAX_URL_LENGTH = 2048 + @keep_lazy(SafeString) def escape(text): @@ -332,9 +334,9 @@ class Urlizer: # Make URL we want to point to. url = None nofollow_attr = ' rel="nofollow"' if nofollow else "" - if self.simple_url_re.match(middle): + if len(middle) <= MAX_URL_LENGTH and self.simple_url_re.match(middle): url = smart_urlquote(html.unescape(middle)) - elif self.simple_url_2_re.match(middle): + elif len(middle) <= MAX_URL_LENGTH and self.simple_url_2_re.match(middle): url = smart_urlquote("http://%s" % html.unescape(middle)) elif ":" not in middle and self.is_email_simple(middle): local, domain = middle.rsplit("@", 1) @@ -449,6 +451,10 @@ class Urlizer: except ValueError: # value contains more than one @. return False + # Max length for domain name labels is 63 characters per RFC 1034. + # Helps to avoid ReDoS vectors in the domain part. + if len(p2) > 63: + return False # Dot must be in p2 (e.g. example.com) if "." not in p2 or p2.startswith("."): return False diff --git a/docs/releases/4.2.15.txt b/docs/releases/4.2.15.txt index 91d6d385e3..1c6a1c7ede 100644 --- a/docs/releases/4.2.15.txt +++ b/docs/releases/4.2.15.txt @@ -23,6 +23,13 @@ CVE-2024-41990: Potential denial-of-service vulnerability in ``django.utils.html denial-of-service attack via very large inputs with a specific sequence of characters. +CVE-2024-41991: Potential denial-of-service vulnerability in ``django.utils.html.urlize()`` and ``AdminURLFieldWidget`` +======================================================================================================================= + +:tfilter:`urlize`, :tfilter:`urlizetrunc`, and ``AdminURLFieldWidget`` were +subject to a potential denial-of-service attack via certain inputs with a very +large number of Unicode characters. + Bugfixes ======== diff --git a/docs/releases/5.0.8.txt b/docs/releases/5.0.8.txt index 7de9bb9de1..2a0823d0ca 100644 --- a/docs/releases/5.0.8.txt +++ b/docs/releases/5.0.8.txt @@ -23,6 +23,13 @@ CVE-2024-41990: Potential denial-of-service vulnerability in ``django.utils.html denial-of-service attack via very large inputs with a specific sequence of characters. +CVE-2024-41991: Potential denial-of-service vulnerability in ``django.utils.html.urlize()`` and ``AdminURLFieldWidget`` +======================================================================================================================= + +:tfilter:`urlize`, :tfilter:`urlizetrunc`, and ``AdminURLFieldWidget`` were +subject to a potential denial-of-service attack via certain inputs with a very +large number of Unicode characters. + Bugfixes ======== diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 517e060b80..5da4adf8c9 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -463,7 +463,12 @@ class AdminSplitDateTimeWidgetTest(SimpleTestCase): class AdminURLWidgetTest(SimpleTestCase): def test_get_context_validates_url(self): w = widgets.AdminURLFieldWidget() - for invalid in ["", "/not/a/full/url/", 'javascript:alert("Danger XSS!")']: + for invalid in [ + "", + "/not/a/full/url/", + 'javascript:alert("Danger XSS!")', + "http://" + "한.글." * 1_000_000 + "com", + ]: with self.subTest(url=invalid): self.assertFalse(w.get_context("name", invalid, {})["url_valid"]) self.assertTrue(w.get_context("name", "http://example.com", {})["url_valid"]) diff --git a/tests/utils_tests/test_html.py b/tests/utils_tests/test_html.py index 6050ed62b0..82dbd58f12 100644 --- a/tests/utils_tests/test_html.py +++ b/tests/utils_tests/test_html.py @@ -338,6 +338,15 @@ class TestUtilsHtml(SimpleTestCase): 'Search for google.com/?q=!', ), ("foo@example.com", 'foo@example.com'), + ( + "test@" + "한.글." * 15 + "aaa", + '' + + "test@" + + "한.글." * 15 + + "aaa", + ), ) for value, output in tests: with self.subTest(value=value): @@ -346,6 +355,10 @@ class TestUtilsHtml(SimpleTestCase): def test_urlize_unchanged_inputs(self): tests = ( ("a" + "@a" * 50000) + "a", # simple_email_re catastrophic test + # Unicode domain catastrophic tests. + "a@" + "한.글." * 1_000_000 + "a", + "http://" + "한.글." * 1_000_000 + "com", + "www." + "한.글." * 1_000_000 + "com", ("a" + "." * 1000000) + "a", # trailing_punctuation catastrophic test "foo@", "@foo.com",