bpo-35907, CVE-2019-9948: urllib rejects local_file:// scheme #13474
Conversation
|
This PR is very close to PR #11842, I just added the unit test: |
|
@tiran: Would you mind to review this forward port of PR #11842? Fix for https://bugs.python.org/issue35907 |
|
@tirkarthi @gpshead: Would you mind to review this HTTP security fix? |
Lib/test/test_urllib.py
Outdated
| for url in ('local_file://example', 'local-file://example'): | ||
| self.assertRaises(IOError, urllib.request.urlopen, url) | ||
| self.assertRaises(IOError, urllib.request.URLopener().open, url) | ||
| self.assertRaises(IOError, DummyURLopener().open, url) |
There was a problem hiding this comment.
Are we making this change to the deprecated legacy interface urllib.request.URLopener().retrieve too? If so I would prefer adding below test since it also now throws IOError that used to return a result. I guess Python 2 also has urlretrieve which now forbids local_file
Note : urlretrieve was untested but documented that it used to throw NameError (https://bugs.python.org/issue36948) . Please also check Windows compatibility if urlretrieve tests have to be added since I mistakenly broke two Windows buildbots in the issue.
self.assertRaises(urllib.request.URLopener().rertrieve, url)There was a problem hiding this comment.
Sorry, my test was wrong. urlretrieve requires a file. Maybe above urlretrieve tests can be used to just check for local-file:// too? Change in behavior as below
Before PR :
cpython git:(master) ✗ touch /tmp/a.txt
cpython git:(master) ✗ ./python.exe -q
>>> import urllib.request
>>> urllib.request.URLopener().retrieve('local_file://localhost/tmp/a.txt')
sys:1: DeprecationWarning: URLopener style of invoking requests is deprecated. Use newer urlopen functions/methods
('/var/folders/2b/mhgtnnpx4z943t4cc9yvw4qw0000gn/T/tmpmx6km86p.txt', <email.message.Message object at 0x1066a2a00>)
After PR :
cpython git:(master) ✗ touch /tmp/a.txt
cpython git:(master) ✗ ./python.exe -q
>>> import urllib.request
>>> urllib.request.URLopener().retrieve('local_file://localhost/tmp/a.txt')
sys:1: DeprecationWarning: URLopener style of invoking requests is deprecated. Use newer urlopen functions/methods
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/urllib/request.py", line 1789, in retrieve
fp = self.open(url, data)
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/urllib/request.py", line 1752, in open
return self.open_unknown(fullurl, data)
File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/urllib/request.py", line 1766, in open_unknown
raise OSError('url error', 'unknown url type', type)
OSError: [Errno url error] unknown url type: 'local_file'
There was a problem hiding this comment.
I didn't know URLopener().retrieve :-) My PR impacts it as well.
I added requested tests.
CVE-2019-9948: Avoid file reading as disallowing the unnecessary URL scheme in URLopener().open() and URLopener().retrieve() of urllib.request. Co-Authored-By: SH <push0ebp@gmail.com>
|
I rebased my PR to modify the commit message: mention also |
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
Sorry, @vstinner, I could not cleanly backport this to |
|
GH-13505 is a backport of this pull request to the 3.7 branch. |
…) (GH-13505) CVE-2019-9948: Avoid file reading as disallowing the unnecessary URL scheme in URLopener().open() and URLopener().retrieve() of urllib.request. Co-Authored-By: SH <push0ebp@gmail.com> (cherry picked from commit 0c2b6a3)
| @@ -0,0 +1,2 @@ | |||
| CVE-2019-9948: Avoid file reading as disallowing the unnecessary URL scheme in | |||
There was a problem hiding this comment.
This text doesn't make sense as written. "as disallowing the unnecessary URL scheme" says nothing. WHAT url scheme did we just break our users applications by refusing to support? Those need to be listed in the NEWS entry.
CVE-2019-9948: Drop support for the unintended local_file:// URL scheme in ...
…) (GH-13505) (#13510) CVE-2019-9948: Avoid file reading by disallowing local-file:// and local_file:// URL schemes in URLopener().open() and URLopener().retrieve() of urllib.request. Co-Authored-By: SH <push0ebp@gmail.com>
CVE-2019-9948: Avoid file reading as disallowing the unnecessary URL
scheme in URLopener().open() of urllib.request.
Co-Authored-By: SH push0ebp@gmail.com
https://bugs.python.org/issue35907