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

[3.5] bpo-35746: Fix segfault in ssl's cert parser (GH-11569) #11867

Merged
merged 3 commits into from Feb 26, 2019

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Feb 15, 2019

Fix a NULL pointer deref in ssl module. The cert parser did not handle CRL
distribution points with empty DP or URI correctly. A malicious or buggy
certificate can result into segfault.

Vulnerability (TALOS-2018-0758) reported by Colin Read and Nicolas
Edet of Cisco.

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

(cherry picked from commit a37f524)

https://bugs.python.org/issue35746

Fix a NULL pointer deref in ssl module. The cert parser did not handle CRL
distribution points with empty DP or URI correctly. A malicious or buggy
certificate can result into segfault.

Vulnerability (TALOS-2018-0758) reported by Colin Read and Nicolas
Edet of Cisco.

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

(cherry picked from commit a37f524)
@vstinner
Copy link
Member Author

vstinner commented Feb 15, 2019

I tested manually on my Fedora 29, the test pass:

$ ./python -m test -v -m test_parse_cert_CVE_2019_5010 test_ssl
Tests result: SUCCESS

(Other tests fail because Python 3.5 isn't fully compatible with OpenSSL 1.1.1 used by Fedora 29.)

@larryhastings
Copy link
Contributor

larryhastings commented Feb 25, 2019

This failed the Travis CI test after updating the branch. Specifically, two tests using ftp failed with a security exception ("bad IP"), I'll paste in an example below.

I'm guessing the code is fine, and this is a temporary / race condition or CI configuration error. Regardless I don't know how to initiate running a new test. Victor, do you know how to make progress on this?

In the meantime I'll see if I can get one of your other PRs in.

--

Traceback (most recent call last):
File "/home/travis/build/python/cpython/Lib/urllib/request.py", line 1477, in ftp_open
fp, retrlen = fw.retrfile(file, type)
File "/home/travis/build/python/cpython/Lib/urllib/request.py", line 2363, in retrfile
conn, retrlen = self.ftp.ntransfercmd(cmd)
File "/home/travis/build/python/cpython/Lib/ftplib.py", line 366, in ntransfercmd
resp = self.sendcmd(cmd)
File "/home/travis/build/python/cpython/Lib/ftplib.py", line 274, in sendcmd
return self.getresp()
File "/home/travis/build/python/cpython/Lib/ftplib.py", line 245, in getresp
raise error_temp(resp)
ftplib.error_temp: 425 Security: Bad IP connecting.

@vstinner
Copy link
Member Author

vstinner commented Feb 25, 2019

ftplib.error_temp: 425 Security: Bad IP connecting.

Yeah it's a known issue. I wrote PR #11874 to backport the fix. Please merge my PR #11874. Once the test fix will be backported, I will rebase this PR on top of it.

@vstinner
Copy link
Member Author

vstinner commented Feb 25, 2019

I'm guessing the code is fine, and this is a temporary / race condition or CI configuration error.

The failure is unrelated to this PR. Travis CI changed their security a few months ago: https://bugs.python.org/issue35411

@larryhastings
Copy link
Contributor

larryhastings commented Feb 25, 2019

So what should I do? I wanna mash that big attractive "Squash and merge" button but it won't let me! It's gray and I want it to be green!

@larryhastings
Copy link
Contributor

larryhastings commented Feb 25, 2019

nm, I found your "skip FTP tests on Travis CI" PR. I'll merge that when I can and then the rest of the dominos will tumble and fall!

@larryhastings larryhastings merged commit efec763 into python:3.5 Feb 26, 2019
@bedevere-bot
Copy link

bedevere-bot commented Feb 26, 2019

@larryhastings: Please replace # with GH- in the commit message next time. Thanks!

@larryhastings
Copy link
Contributor

larryhastings commented Feb 26, 2019

Thanks for the backport! 3.5 is now poised to take over the world.

@vstinner vstinner deleted the ssl_crl_bug35 branch Apr 29, 2019
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.

None yet

5 participants