From ca2be7724e1244a4cb723de40a070f873c6e94bf Mon Sep 17 00:00:00 2001 From: Michael Manfre Date: Wed, 11 Dec 2024 21:39:32 -0500 Subject: [PATCH] Fixed CVE-2024-56374 -- Mitigated potential DoS in IPv6 validation. Thanks Saravana Kumar for the report, and Sarah Boyce and Mariusz Felisiak for the reviews. Co-authored-by: Natalia <124304+nessita@users.noreply.github.com> --- django/db/models/fields/__init__.py | 6 ++-- django/forms/fields.py | 7 ++-- django/utils/ipv6.py | 19 +++++++++-- docs/ref/forms/fields.txt | 13 ++++++-- docs/releases/4.2.18.txt | 12 +++++++ docs/releases/5.0.11.txt | 12 +++++++ docs/releases/5.1.5.txt | 12 +++++++ .../field_tests/test_genericipaddressfield.py | 33 ++++++++++++++++++- tests/utils_tests/test_ipv6.py | 31 +++++++++++++++-- 9 files changed, 131 insertions(+), 14 deletions(-) diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index ba8cf472c3..66d79626fd 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -32,7 +32,7 @@ from django.utils.dateparse import ( ) from django.utils.duration import duration_microseconds, duration_string from django.utils.functional import Promise, cached_property -from django.utils.ipv6 import clean_ipv6_address +from django.utils.ipv6 import MAX_IPV6_ADDRESS_LENGTH, clean_ipv6_address from django.utils.text import capfirst from django.utils.translation import gettext_lazy as _ @@ -2233,7 +2233,7 @@ class GenericIPAddressField(Field): self.default_validators = validators.ip_address_validators( protocol, unpack_ipv4 ) - kwargs["max_length"] = 39 + kwargs["max_length"] = MAX_IPV6_ADDRESS_LENGTH super().__init__(verbose_name, name, *args, **kwargs) def check(self, **kwargs): @@ -2260,7 +2260,7 @@ class GenericIPAddressField(Field): kwargs["unpack_ipv4"] = self.unpack_ipv4 if self.protocol != "both": kwargs["protocol"] = self.protocol - if kwargs.get("max_length") == 39: + if kwargs.get("max_length") == self.max_length: del kwargs["max_length"] return name, path, args, kwargs diff --git a/django/forms/fields.py b/django/forms/fields.py index 1a58a60743..202a6d72c8 100644 --- a/django/forms/fields.py +++ b/django/forms/fields.py @@ -46,7 +46,7 @@ from django.utils.choices import normalize_choices from django.utils.dateparse import parse_datetime, parse_duration from django.utils.deprecation import RemovedInDjango60Warning from django.utils.duration import duration_string -from django.utils.ipv6 import clean_ipv6_address +from django.utils.ipv6 import MAX_IPV6_ADDRESS_LENGTH, clean_ipv6_address from django.utils.regex_helper import _lazy_re_compile from django.utils.translation import gettext_lazy as _ from django.utils.translation import ngettext_lazy @@ -1303,6 +1303,7 @@ class GenericIPAddressField(CharField): self.default_validators = validators.ip_address_validators( protocol, unpack_ipv4 ) + kwargs.setdefault("max_length", MAX_IPV6_ADDRESS_LENGTH) super().__init__(**kwargs) def to_python(self, value): @@ -1310,7 +1311,9 @@ class GenericIPAddressField(CharField): return "" value = value.strip() if value and ":" in value: - return clean_ipv6_address(value, self.unpack_ipv4) + return clean_ipv6_address( + value, self.unpack_ipv4, max_length=self.max_length + ) return value diff --git a/django/utils/ipv6.py b/django/utils/ipv6.py index 8b691b5e66..1b79d52226 100644 --- a/django/utils/ipv6.py +++ b/django/utils/ipv6.py @@ -3,9 +3,22 @@ import ipaddress from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ +MAX_IPV6_ADDRESS_LENGTH = 39 + + +def _ipv6_address_from_str(ip_str, max_length=MAX_IPV6_ADDRESS_LENGTH): + if len(ip_str) > max_length: + raise ValueError( + f"Unable to convert {ip_str} to an IPv6 address (value too long)." + ) + return ipaddress.IPv6Address(int(ipaddress.IPv6Address(ip_str))) + def clean_ipv6_address( - ip_str, unpack_ipv4=False, error_message=_("This is not a valid IPv6 address.") + ip_str, + unpack_ipv4=False, + error_message=_("This is not a valid IPv6 address."), + max_length=MAX_IPV6_ADDRESS_LENGTH, ): """ Clean an IPv6 address string. @@ -24,7 +37,7 @@ def clean_ipv6_address( Return a compressed IPv6 address or the same value. """ try: - addr = ipaddress.IPv6Address(int(ipaddress.IPv6Address(ip_str))) + addr = _ipv6_address_from_str(ip_str, max_length) except ValueError: raise ValidationError( error_message, code="invalid", params={"protocol": _("IPv6")} @@ -43,7 +56,7 @@ def is_valid_ipv6_address(ip_str): Return whether or not the `ip_str` string is a valid IPv6 address. """ try: - ipaddress.IPv6Address(ip_str) + _ipv6_address_from_str(ip_str) except ValueError: return False return True diff --git a/docs/ref/forms/fields.txt b/docs/ref/forms/fields.txt index 2b4b344844..c0ab4f2434 100644 --- a/docs/ref/forms/fields.txt +++ b/docs/ref/forms/fields.txt @@ -761,7 +761,7 @@ For each field, we describe the default widget used if you don't specify * Empty value: ``''`` (an empty string) * Normalizes to: A string. IPv6 addresses are normalized as described below. * Validates that the given value is a valid IP address. - * Error message keys: ``required``, ``invalid`` + * Error message keys: ``required``, ``invalid``, ``max_length`` The IPv6 address normalization follows :rfc:`4291#section-2.2` section 2.2, including using the IPv4 format suggested in paragraph 3 of that section, like @@ -769,7 +769,7 @@ For each field, we describe the default widget used if you don't specify ``2001::1``, and ``::ffff:0a0a:0a0a`` to ``::ffff:10.10.10.10``. All characters are converted to lowercase. - Takes two optional arguments: + Takes three optional arguments: .. attribute:: protocol @@ -784,6 +784,15 @@ For each field, we describe the default widget used if you don't specify ``192.0.2.1``. Default is disabled. Can only be used when ``protocol`` is set to ``'both'``. + .. attribute:: max_length + + Defaults to 39, and behaves the same way as it does for + :class:`CharField`. + + .. versionchanged:: 4.2.18 + + The default value for ``max_length`` was set to 39 characters. + ``ImageField`` -------------- diff --git a/docs/releases/4.2.18.txt b/docs/releases/4.2.18.txt index ae0c4e3334..fef91d9150 100644 --- a/docs/releases/4.2.18.txt +++ b/docs/releases/4.2.18.txt @@ -5,3 +5,15 @@ Django 4.2.18 release notes *January 14, 2025* Django 4.2.18 fixes a security issue with severity "moderate" in 4.2.17. + +CVE-2024-56374: Potential denial-of-service vulnerability in IPv6 validation +============================================================================ + +Lack of upper bound limit enforcement in strings passed when performing IPv6 +validation could lead to a potential denial-of-service attack. The undocumented +and private functions ``clean_ipv6_address`` and ``is_valid_ipv6_address`` were +vulnerable, as was the :class:`django.forms.GenericIPAddressField` form field, +which has now been updated to define a ``max_length`` of 39 characters. + +The :class:`django.db.models.GenericIPAddressField` model field was not +affected. diff --git a/docs/releases/5.0.11.txt b/docs/releases/5.0.11.txt index c284b51435..e5cb684888 100644 --- a/docs/releases/5.0.11.txt +++ b/docs/releases/5.0.11.txt @@ -5,3 +5,15 @@ Django 5.0.11 release notes *January 14, 2025* Django 5.0.11 fixes a security issue with severity "moderate" in 5.0.10. + +CVE-2024-56374: Potential denial-of-service vulnerability in IPv6 validation +============================================================================ + +Lack of upper bound limit enforcement in strings passed when performing IPv6 +validation could lead to a potential denial-of-service attack. The undocumented +and private functions ``clean_ipv6_address`` and ``is_valid_ipv6_address`` were +vulnerable, as was the :class:`django.forms.GenericIPAddressField` form field, +which has now been updated to define a ``max_length`` of 39 characters. + +The :class:`django.db.models.GenericIPAddressField` model field was not +affected. diff --git a/docs/releases/5.1.5.txt b/docs/releases/5.1.5.txt index 5a1136f2de..574663df36 100644 --- a/docs/releases/5.1.5.txt +++ b/docs/releases/5.1.5.txt @@ -7,6 +7,18 @@ Django 5.1.5 release notes Django 5.1.5 fixes a security issue with severity "moderate" and one bug in 5.1.4. +CVE-2024-56374: Potential denial-of-service vulnerability in IPv6 validation +============================================================================ + +Lack of upper bound limit enforcement in strings passed when performing IPv6 +validation could lead to a potential denial-of-service attack. The undocumented +and private functions ``clean_ipv6_address`` and ``is_valid_ipv6_address`` were +vulnerable, as was the :class:`django.forms.GenericIPAddressField` form field, +which has now been updated to define a ``max_length`` of 39 characters. + +The :class:`django.db.models.GenericIPAddressField` model field was not +affected. + Bugfixes ======== diff --git a/tests/forms_tests/field_tests/test_genericipaddressfield.py b/tests/forms_tests/field_tests/test_genericipaddressfield.py index 80722f5c65..ef00a727a4 100644 --- a/tests/forms_tests/field_tests/test_genericipaddressfield.py +++ b/tests/forms_tests/field_tests/test_genericipaddressfield.py @@ -1,6 +1,7 @@ from django.core.exceptions import ValidationError from django.forms import GenericIPAddressField from django.test import SimpleTestCase +from django.utils.ipv6 import MAX_IPV6_ADDRESS_LENGTH class GenericIPAddressFieldTest(SimpleTestCase): @@ -125,6 +126,35 @@ class GenericIPAddressFieldTest(SimpleTestCase): ): f.clean("1:2") + def test_generic_ipaddress_max_length_custom(self): + # Valid IPv4-mapped IPv6 address, len 45. + addr = "0000:0000:0000:0000:0000:ffff:192.168.100.228" + f = GenericIPAddressField(max_length=len(addr)) + f.clean(addr) + + def test_generic_ipaddress_max_length_validation_error(self): + # Valid IPv4-mapped IPv6 address, len 45. + addr = "0000:0000:0000:0000:0000:ffff:192.168.100.228" + + cases = [ + ({}, MAX_IPV6_ADDRESS_LENGTH), # Default value. + ({"max_length": len(addr) - 1}, len(addr) - 1), + ] + for kwargs, max_length in cases: + max_length_plus_one = max_length + 1 + msg = ( + f"Ensure this value has at most {max_length} characters (it has " + f"{max_length_plus_one}).'" + ) + with self.subTest(max_length=max_length): + f = GenericIPAddressField(**kwargs) + with self.assertRaisesMessage(ValidationError, msg): + f.clean("x" * max_length_plus_one) + with self.assertRaisesMessage( + ValidationError, "This is not a valid IPv6 address." + ): + f.clean(addr) + def test_generic_ipaddress_as_generic_not_required(self): f = GenericIPAddressField(required=False) self.assertEqual(f.clean(""), "") @@ -150,7 +180,8 @@ class GenericIPAddressFieldTest(SimpleTestCase): f.clean(" fe80::223:6cff:fe8a:2e8a "), "fe80::223:6cff:fe8a:2e8a" ) self.assertEqual( - f.clean(" 2a02::223:6cff:fe8a:2e8a "), "2a02::223:6cff:fe8a:2e8a" + f.clean(" " * MAX_IPV6_ADDRESS_LENGTH + " 2a02::223:6cff:fe8a:2e8a "), + "2a02::223:6cff:fe8a:2e8a", ) with self.assertRaisesMessage( ValidationError, "'This is not a valid IPv6 address.'" diff --git a/tests/utils_tests/test_ipv6.py b/tests/utils_tests/test_ipv6.py index bf78ed91c0..1754c7b356 100644 --- a/tests/utils_tests/test_ipv6.py +++ b/tests/utils_tests/test_ipv6.py @@ -1,9 +1,16 @@ -import unittest +import traceback +from io import StringIO -from django.utils.ipv6 import clean_ipv6_address, is_valid_ipv6_address +from django.core.exceptions import ValidationError +from django.test import SimpleTestCase +from django.utils.ipv6 import ( + MAX_IPV6_ADDRESS_LENGTH, + clean_ipv6_address, + is_valid_ipv6_address, +) -class TestUtilsIPv6(unittest.TestCase): +class TestUtilsIPv6(SimpleTestCase): def test_validates_correct_plain_address(self): self.assertTrue(is_valid_ipv6_address("fe80::223:6cff:fe8a:2e8a")) self.assertTrue(is_valid_ipv6_address("2a02::223:6cff:fe8a:2e8a")) @@ -64,3 +71,21 @@ class TestUtilsIPv6(unittest.TestCase): self.assertEqual( clean_ipv6_address("::ffff:18.52.18.52", unpack_ipv4=True), "18.52.18.52" ) + + def test_address_too_long(self): + addresses = [ + "0000:0000:0000:0000:0000:ffff:192.168.100.228", # IPv4-mapped IPv6 address + "0000:0000:0000:0000:0000:ffff:192.168.100.228%123456", # % scope/zone + "fe80::223:6cff:fe8a:2e8a:1234:5678:00000", # MAX_IPV6_ADDRESS_LENGTH + 1 + ] + msg = "This is the error message." + value_error_msg = "Unable to convert %s to an IPv6 address (value too long)." + for addr in addresses: + with self.subTest(addr=addr): + self.assertGreater(len(addr), MAX_IPV6_ADDRESS_LENGTH) + self.assertEqual(is_valid_ipv6_address(addr), False) + with self.assertRaisesMessage(ValidationError, msg) as ctx: + clean_ipv6_address(addr, error_message=msg) + exception_traceback = StringIO() + traceback.print_exception(ctx.exception, file=exception_traceback) + self.assertIn(value_error_msg % addr, exception_traceback.getvalue())