Skip to content

Conversation

@mssalvatore
Copy link
Contributor

@mssalvatore mssalvatore commented May 15, 2025

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, a regression test would be good but hash() is an implementation detail, making it CPython-only (I don't know how PyPy and co implement it), and if we don't already have a test for the similar issue you found, there's no need for one.

@picnixz picnixz added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels May 15, 2025
@mssalvatore
Copy link
Contributor Author

Ideally, a regression test would be good but hash() is an implementation detail, making it CPython-only (I don't know how PyPy and co implement it), and if we don't already have a test for the similar issue you found, there's no need for one.

I can add a regression test.

@picnixz
Copy link
Member

picnixz commented May 16, 2025

I can add a regression test.

Let's add a test where we manually craft the values that are hashed. I'm however unsure whether hash((X, Y)) where X and Y are known to be ints is always stable. For strings and bytes, this is not the case due to security reasons, but for ints, I think it's stable but I cannot say for sure that it's the case.

If there wasn't a test introduced for the previous CVE, just don't bother with a test.

@mssalvatore mssalvatore force-pushed the fix-network-hash-collisions branch from eeabe2a to 264bf69 Compare May 16, 2025 14:08
@mssalvatore
Copy link
Contributor Author

If there wasn't a test introduced for the previous CVE, just don't bother with a test.

These tests were introduced for the previous CVE:

# issue41004 Hash collisions in IPv4Interface and IPv6Interface
def testV4HashIsNotConstant(self):
ipv4_address1 = ipaddress.IPv4Interface("1.2.3.4")
ipv4_address2 = ipaddress.IPv4Interface("2.3.4.5")
self.assertNotEqual(ipv4_address1.__hash__(), ipv4_address2.__hash__())
# issue41004 Hash collisions in IPv4Interface and IPv6Interface
def testV6HashIsNotConstant(self):
ipv6_address1 = ipaddress.IPv6Interface("2001:658:22a:cafe:200:0:0:1")
ipv6_address2 = ipaddress.IPv6Interface("2001:658:22a:cafe:200:0:0:2")
self.assertNotEqual(ipv6_address1.__hash__(), ipv6_address2.__hash__())

I added some tests in a separate commit. Feel free to drop it if you don't think the tests are valuable.

@mssalvatore mssalvatore force-pushed the fix-network-hash-collisions branch from 264bf69 to 492c579 Compare May 16, 2025 14:12
@gpshead gpshead added the type-security A security issue label May 17, 2025
@gpshead gpshead added needs backport to 3.9 needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels May 17, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 492c579 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134063%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 17, 2025
@gpshead gpshead self-assigned this May 17, 2025
@gpshead gpshead merged commit f3fc0c1 into python:main May 22, 2025
135 of 136 checks passed
@miss-islington-app
Copy link

Thanks @mssalvatore for the PR, and @gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10, 3.11, 3.12, 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2025
…ythonGH-134063)

(cherry picked from commit f3fc0c1)

Co-authored-by: Mike Salvatore <[email protected]>
pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2025
…ythonGH-134063)

(cherry picked from commit f3fc0c1)

Co-authored-by: Mike Salvatore <[email protected]>
pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2025

GH-134476 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 22, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2025
…ythonGH-134063)

(cherry picked from commit f3fc0c1)

Co-authored-by: Mike Salvatore <[email protected]>
pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2025

GH-134477 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 22, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2025
…ythonGH-134063)

(cherry picked from commit f3fc0c1)

Co-authored-by: Mike Salvatore <[email protected]>
pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2025

GH-134478 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label May 22, 2025
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2025

GH-134479 is a backport of this pull request to the 3.11 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2025
…ythonGH-134063)

(cherry picked from commit f3fc0c1)

Co-authored-by: Mike Salvatore <[email protected]>
pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label May 22, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2025
…ythonGH-134063)

(cherry picked from commit f3fc0c1)

Co-authored-by: Mike Salvatore <[email protected]>
pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2025

GH-134480 is a backport of this pull request to the 3.10 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.10 only security fixes label May 22, 2025
@bedevere-app
Copy link

bedevere-app bot commented May 22, 2025

GH-134481 is a backport of this pull request to the 3.9 branch.

gpshead pushed a commit that referenced this pull request May 22, 2025
…H-134063) (#134477)

gh-134062: Fix hash collisions in IPv4Network and IPv6Network (GH-134063)
(cherry picked from commit f3fc0c1)


gh-134062: Fix hash collisions in IPv4Network and IPv6Network
gh-134062: Add hash collision regression test

Co-authored-by: Mike Salvatore <[email protected]>
gpshead pushed a commit that referenced this pull request May 22, 2025
…H-134063) (#134476)

gh-134062: Fix hash collisions in IPv4Network and IPv6Network (GH-134063)
(cherry picked from commit f3fc0c1)


gh-134062: Fix hash collisions in IPv4Network and IPv6Network
gh-134062: Add hash collision regression test

Co-authored-by: Mike Salvatore <[email protected]>
Yhg1s pushed a commit that referenced this pull request May 26, 2025
…H-134063) (#134478)

gh-134062: Fix hash collisions in IPv4Network and IPv6Network (GH-134063)
(cherry picked from commit f3fc0c1)


gh-134062: Fix hash collisions in IPv4Network and IPv6Network
gh-134062: Add hash collision regression test

Co-authored-by: Mike Salvatore <[email protected]>
lkollar pushed a commit to lkollar/cpython that referenced this pull request May 26, 2025
…ythonGH-134063)

pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
ambv pushed a commit that referenced this pull request Jun 2, 2025
ambv pushed a commit that referenced this pull request Jun 2, 2025
ambv pushed a commit that referenced this pull request Jun 2, 2025
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
…ythonGH-134063)

pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
…ythonGH-134063)

pythongh-134062: Fix hash collisions in IPv4Network and IPv6Network
pythongh-134062: Add hash collision regression test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-security A security issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants