Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Excessive hash collisions in IPv4Network and IPv6Network classes #134062

Copy link
Copy link
Open
@mssalvatore

Description

@mssalvatore
Issue body actions

Bug report

Bug description:

While working with IPv4Network objects, @cakekoa and I discovered that hash collisions were common. Below is a small script that shows a few examples of different networks that have hash collisions.

from ipaddress import IPv4Network, IPv6Network


def test_hash_collision(network_1, network_2):
    # Shows that the networks are not equivalent.
    assert network_1 != network_2
    assert network_1.num_addresses != network_2.num_addresses

    # Shows a hash collision similar to CVE-2020-14422
    assert hash(network_1) == hash(network_2)


test_hash_collision(IPv4Network("192.168.1.255/32"), IPv4Network("192.168.1.0/24"))
test_hash_collision(IPv4Network("172.24.255.0/24"), IPv4Network("172.24.0.0/16"))
test_hash_collision(IPv4Network("192.168.1.87/32"), IPv4Network("192.168.1.86/31"))
test_hash_collision(
    IPv4Network("10.0.0.0/8"), IPv6Network("ffff:ffff:ffff:ffff:ffff:ffff:aff:0/112")
)

Upon investigating, we discovered CVE-2020-14422, which fixed a similar (albeit much more severe) hash collision in the IPv4Interface and IPv6Interface classes. This CVE was fixed in b98e779.

The implementation of _BaseNetwork.__hash() looks like this on the main branch:

    def __hash__(self):
        return hash(int(self.network_address) ^ int(self.netmask))

Based on the fix for CVE-2020-14422, the fix for the _BaseNetwork class would likely look like:

diff --git a/Lib/ipaddress.py b/Lib/ipaddress.py
index 703fa289dda..d8a84f33264 100644
--- a/Lib/ipaddress.py
+++ b/Lib/ipaddress.py
@@ -729,7 +729,7 @@ def __eq__(self, other):
             return NotImplemented
 
     def __hash__(self):
-        return hash(int(self.network_address) ^ int(self.netmask))
+        return hash((int(self.network_address), int(self.netmask)))
 
     def __contains__(self, other):
         # always false if one is v4 and the other is v6.

As this method produces far fewer collisions than what caused CVE-2020-14422, the security impact is likely negligible. @sethmlarson from the PSRT team has given us the green light to publicly submit a fix.

CPython versions tested on:

3.12

Operating systems tested on:

Linux, macOS

Linked PRs

Metadata

Metadata

Assignees

No one assigned

    Labels

    stdlibPython modules in the Lib dirPython modules in the Lib dirtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or errortype-securityA security issueA security issue

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Morty Proxy This is a proxified and sanitized view of the page, visit original site.