Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-36384: Leading zeros in IPv4 addresses are no longer tolerated #25099

Merged
merged 11 commits into from May 2, 2021

Conversation

@tiran
Copy link
Member

@tiran tiran commented Mar 30, 2021

Reverts commit e653d4d and makes
parsing even more strict. Like socket.inet_pton() any leading zero
is now treated as invalid input.

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue36384

Lib/test/test_ipaddress.py Outdated Show resolved Hide resolved
Lib/test/test_ipaddress.py Show resolved Hide resolved
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

What's New entry?

Doc/library/ipaddress.rst Show resolved Hide resolved
Lib/ipaddress.py Show resolved Hide resolved
Lib/test/test_ipaddress.py Show resolved Hide resolved
Doc/library/ipaddress.rst Outdated Show resolved Hide resolved
@tiran
Copy link
Member Author

@tiran tiran commented Mar 30, 2021

I' leaving the whatsnew entry to release managers.

Lib/ipaddress.py Outdated Show resolved Hide resolved
@vstinner
Copy link
Member

@vstinner vstinner commented Mar 31, 2021

I' leaving the whatsnew entry to release managers.

Usually, release managers don't maintain the What's New document. Since this change is backward incompatible, IMO it would deserve a "Notable changes in Python 3.x.y" in their What's New document in 3.6-3.9 branches. Example for a recent security fix:
https://docs.python.org/dev/whatsnew/3.7.html#notable-changes-in-python-3-7-10

For Python 3.10, it can be written in the https://docs.python.org/dev/whatsnew/3.10.html#changes-in-the-python-api section.

@tiran tiran force-pushed the tiran:issue36384-ipaddress-zero branch from c3d2fae to d79774f Mar 31, 2021
Doc/library/ipaddress.rst Outdated Show resolved Hide resolved
tiran added 3 commits Mar 30, 2021
Reverts commit e653d4d and makes
parsing even more strict. Like socket.inet_pton() any leading zero
is now treated as invalid input.

Signed-off-by: Christian Heimes <christian@python.org>
@tiran tiran force-pushed the tiran:issue36384-ipaddress-zero branch from d79774f to a91c18a Mar 31, 2021
@zooba
Copy link
Member

@zooba zooba commented Apr 1, 2021

This looks ready to me, and Christian has asked someone else to finish the merge as he's out.

Any other feedback? @vstinner, @serhiy-storchaka, are you guys happy with the changes (pending the versionchanged being updated in each backport)?

@zooba
Copy link
Member

@zooba zooba commented Apr 3, 2021

Withdrawing the readiness - @ambv and I would prefer to see this behind a flag (probably "strict" parsing), on by default for 3.10, and maybe on by default for 3.9/earlier.

The main reasoning being that this isn't our vulnerability, but an inconsistency with other vulnerable libraries. The current fix is the best it can be, but it doesn't prevent the vulnerability, it just causes Python to break first. So it ought to be relatively easy to retain the flexible (though admittedly non-sensical) behaviour for those who currently rely on it.

Doc/whatsnew/3.8.rst Outdated Show resolved Hide resolved
ambv added 8 commits May 2, 2021
🙄
@ambv ambv merged commit 60ce8f0 into python:master May 2, 2021
12 checks passed
12 checks passed
@github-actions
Docs
Details
@github-actions
Check for source changes
Details
@github-actions
Check if generated files are up to date
Details
@github-actions
Windows (x86)
Details
@github-actions
Windows (x64)
Details
@github-actions
macOS
Details
@github-actions
Ubuntu
Details
@github-actions
Ubuntu SSL tests with OpenSSL
Details
Azure Pipelines PR #20210502.41 succeeded
Details
@travis-ci
Travis CI - Pull Request Build Passed
Details
@bedevere-bot
bedevere/issue-number Issue number 36384 found
Details
@bedevere-bot
bedevere/news News entry found in Misc/NEWS.d
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented May 2, 2021

Thanks @tiran for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒🤖

@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented May 2, 2021

Sorry, @tiran and @ambv, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 60ce8f0be6354ad565393ab449d8de5d713f35bc 3.8

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented May 2, 2021

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

miss-islington added a commit to miss-islington/cpython that referenced this pull request May 2, 2021
…ythonGH-25099)

Reverts commit e653d4d and makes
parsing even more strict. Like socket.inet_pton() any leading zero
is now treated as invalid input.

Signed-off-by: Christian Heimes <christian@python.org>

Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit 60ce8f0)

Co-authored-by: Christian Heimes <christian@python.org>
ambv pushed a commit that referenced this pull request May 2, 2021
…H-25099) (GH-25815)

Reverts commit e653d4d and makes
parsing even more strict. Like socket.inet_pton() any leading zero
is now treated as invalid input.

Signed-off-by: Christian Heimes <christian@python.org>

Co-authored-by: Łukasz Langa <lukasz@langa.pl>
(cherry picked from commit 60ce8f0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment