gh-128840: Limit the number of parts in IPv6 address parsing

[sethmlarson]

See: #128840

cc @nessita

• Issue: IPv6 address parsing doesn't limit buffer size #128840

[nessita]

Looks great, thank you @sethmlarson!

[lazysegtree]

@sethmlarson How does this fix prevents a potential denial-of-service ?

This prevents excessive memory consumption and potential
denial-of-service when parsing a large IPv6 address.

In the case when we end up with ip_str with extra :, all this fix causes is that the list created in the split call is shorter (Reduced memory usage, and Less CPU instruction due to prevention of additional append calls to the allocated list).
Even if this fix is not there, program will just consume a bit more memory and a bit more cpu (as @serhiy-storchaka already pointed out, "proportional memory already spent on the input string and proportional time was already spent on reading and decoding it").

And, It should not be labelled "Type-Security" .

[lazysegtree]

Point to note : this PR is relevant to issue - #128840 , but it doesn't entirely fix the issue.
The issue is

IPv6 addresses have a maximum length (8 colon-separated parts) but the current implementation doesn't limit the length.

This fix just limits the number of : in the address, not the entire address length. A string like this would still be relavant to the issue, and would not be fixed by this.

'0000::' + '0'*(10**4)

And, a complete fix would maybe add a check in _ip_int_from_string method for length of ip_str to be less than or equal to 39 (0000:0000:0000:0000:0000:0000:0000:0000)

This check could come in the __init__ method of IPv6Address class, but that might not be the best place for it.

[serhiy-storchaka]

This still mentions an implementation detail.

Also, what is the denial-of-service? Do you have a reproducer?

To me, it looks like a tiny performance improvement (which may even not be noticeable). Since it is so simple, it will not harm to backport it to maintained versions (although we are not obliged to do this), but this is far from security issue. The NEWS entry makes it looking much more severe than it is.

[sethmlarson]

Pushed an update to the newsfragment: e1e6917 I also tagged you into the PSRT thread, Django has treated this as a security fix, I agree that it's low severity.

[nessita]

For Django, this does qualify as a security issue. Very long strings would cause a system to hang, and considering that ipaddress.IPv6Address(user_value) is used as part of Django's form field sanitization/validation (where direct user input is potentially received), we need to ensure that the ipv6 address validation is robust.

For example, in my system, the following uses all the available ram:

>>> from ipaddress import IPv6Address
>>> value = "abcd:abcd:abcd" * 999_999_999  # this takes a few seconds
>>> addr = IPv6Address(value) # this uses all RAM and hangs

[sethmlarson]

@lazysegtree I've made the updates to limit total number of characters in addition to number of splits.