[2.7] bpo-30458: Disallow control chars in http URLs. (GH-12755) (GH-13154)#13315
[2.7] bpo-30458: Disallow control chars in http URLs. (GH-12755) (GH-13154)#13315vstinner merged 2 commits intopython:2.7from vstinner:url-no-control-chars27
Conversation
Disallow control chars in http URLs in urllib2.urlopen. This addresses a potential security problem for applications that do not sanity check their URLs where http request headers could be injected. Disable https related urllib tests on a build without ssl (GH-13032) These tests require an SSL enabled build. Skip these tests when python is built without SSL to fix test failures. Use httplib.InvalidURL instead of ValueError as the new error case's exception. (GH-13044) Backport Co-Authored-By: Miro Hrončok <miro@hroncok.cz> (cherry picked from commit 7e200e0) Notes on backport to Python 2.7: * test_urllib tests urllib.urlopen() which quotes the URL and so is not vulerable to HTTP Header Injection. * Add tests to test_urllib2 on urllib2.urlopen(). * Reject non-ASCII characters: range 0x80-0xff.
|
I backported the fix from Python 3.7 to Python 2.7. Please review it carefully, I had to make multiple changes to adapt the fix to Python 2:
|
|
@orsenthil @ned-deily @tirkarthi @hroncok: Would you mind to review my backport to Python 2.7? |
|
@vstinner I'm not sure why you asked me to review this. Perhaps you meant to ask @benjaminp as 2.7. release manager? |
I just copied the list of all people who reviewed the change in other branches. If you have no opinion, that's fine :-) I would prefer to have more eyes as possible on this tricky backport ;-) |
gpshead
left a comment
gpshead
left a comment
There was a problem hiding this comment.
overall, this looks good to me, my only comments are minor.
Lib/test/test_urllib.py
Outdated
| finally: | ||
| self.unfakehttp() | ||
|
|
||
| @unittest.skipUnless(ssl, "ssl module required") |
There was a problem hiding this comment.
is this true for these tests? (not that it matters, all sane platforms have ssl so these tests will be run regardless)
There was a problem hiding this comment.
oops, no, the decorator is wrong: i removed it.
|
Ok, thanks everybody for reviews. I merged my PR. |
This change brings selected modules and tests from CPython 2.7.18 to our lib-python. Where we have specialised versions for Jython, we cherry-pick tests and code that relate to the CVE. We use the 2.7 back-port from CPython as a guide (python/cpython#13315).
8 participants
Disallow control chars in http URLs in urllib2.urlopen. This
addresses a potential security problem for applications that do not
sanity check their URLs where http request headers could be injected.
Disable https related urllib tests on a build without ssl (GH-13032)
These tests require an SSL enabled build. Skip these tests when
python is built without SSL to fix test failures.
Use httplib.InvalidURL instead of ValueError as the new error case's
exception. (GH-13044)
Backport Co-Authored-By: Miro Hrončok miro@hroncok.cz
(cherry picked from commit 7e200e0)
Notes on backport to Python 2.7:
not vulerable to HTTP Header Injection.
https://bugs.python.org/issue30458