Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit bb8071a

Browse filesBrowse files
vstinnervstinner
authored
bpo-30458: Disallow control chars in http URLs (GH-12755) (GH-13154) (GH-13315)
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.
1 parent c841a30 commit bb8071a
Copy full SHA for bb8071a

5 files changed

+99-2Lines changed: 99 additions & 2 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎Lib/httplib.py‎

Copy file name to clipboardExpand all lines: Lib/httplib.py
+16Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,16 @@
247247
_is_legal_header_name = re.compile(r'\A[^:\s][^:\r\n]*\Z').match
248248
_is_illegal_header_value = re.compile(r'\n(?![ \t])|\r(?![ \t\n])').search
249249

250+
# These characters are not allowed within HTTP URL paths.
251+
# See https://tools.ietf.org/html/rfc3986#section-3.3 and the
252+
# https://tools.ietf.org/html/rfc3986#appendix-A pchar definition.
253+
# Prevents CVE-2019-9740. Includes control characters such as \r\n.
254+
# Restrict non-ASCII characters above \x7f (0x80-0xff).
255+
_contains_disallowed_url_pchar_re = re.compile('[\x00-\x20\x7f-\xff]')
256+
# Arguably only these _should_ allowed:
257+
# _is_allowed_url_pchars_re = re.compile(r"^[/!$&'()*+,;=:@%a-zA-Z0-9._~-]+$")
258+
# We are more lenient for assumed real world compatibility purposes.
259+
250260
# We always set the Content-Length header for these methods because some
251261
# servers will otherwise respond with a 411
252262
_METHODS_EXPECTING_BODY = {'PATCH', 'POST', 'PUT'}
@@ -927,6 +937,12 @@ def putrequest(self, method, url, skip_host=0, skip_accept_encoding=0):
927937
self._method = method
928938
if not url:
929939
url = '/'
940+
# Prevent CVE-2019-9740.
941+
match = _contains_disallowed_url_pchar_re.search(url)
942+
if match:
943+
raise InvalidURL("URL can't contain control characters. %r "
944+
"(found at least %r)"
945+
% (url, match.group()))
930946
hdr = '%s %s %s' % (method, url, self._http_vsn_str)
931947

932948
self._output(hdr)
Collapse file

‎Lib/test/test_urllib.py‎

Copy file name to clipboardExpand all lines: Lib/test/test_urllib.py
+25Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,31 @@ def test_url_fragment(self):
257257
finally:
258258
self.unfakehttp()
259259

260+
def test_url_with_control_char_rejected(self):
261+
for char_no in range(0, 0x21) + range(0x7f, 0x100):
262+
char = chr(char_no)
263+
schemeless_url = "//localhost:7777/test%s/" % char
264+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
265+
try:
266+
# urllib quotes the URL so there is no injection.
267+
resp = urllib.urlopen("http:" + schemeless_url)
268+
self.assertNotIn(char, resp.geturl())
269+
finally:
270+
self.unfakehttp()
271+
272+
def test_url_with_newline_header_injection_rejected(self):
273+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
274+
host = "localhost:7777?a=1 HTTP/1.1\r\nX-injected: header\r\nTEST: 123"
275+
schemeless_url = "//" + host + ":8080/test/?test=a"
276+
try:
277+
# urllib quotes the URL so there is no injection.
278+
resp = urllib.urlopen("http:" + schemeless_url)
279+
self.assertNotIn(' ', resp.geturl())
280+
self.assertNotIn('\r', resp.geturl())
281+
self.assertNotIn('\n', resp.geturl())
282+
finally:
283+
self.unfakehttp()
284+
260285
def test_read_bogus(self):
261286
# urlopen() should raise IOError for many error codes.
262287
self.fakehttp('''HTTP/1.1 401 Authentication Required
Collapse file

‎Lib/test/test_urllib2.py‎

Copy file name to clipboardExpand all lines: Lib/test/test_urllib2.py
+50-1Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
except ImportError:
1616
ssl = None
1717

18+
from test.test_urllib import FakeHTTPMixin
19+
20+
1821
# XXX
1922
# Request
2023
# CacheFTPHandler (hard to write)
@@ -1262,7 +1265,7 @@ def _test_basic_auth(self, opener, auth_handler, auth_header,
12621265
self.assertEqual(len(http_handler.requests), 1)
12631266
self.assertFalse(http_handler.requests[0].has_header(auth_header))
12641267

1265-
class MiscTests(unittest.TestCase):
1268+
class MiscTests(unittest.TestCase, FakeHTTPMixin):
12661269

12671270
def test_build_opener(self):
12681271
class MyHTTPHandler(urllib2.HTTPHandler): pass
@@ -1317,6 +1320,52 @@ def test_unsupported_algorithm(self):
13171320
"Unsupported digest authentication algorithm 'invalid'"
13181321
)
13191322

1323+
@unittest.skipUnless(ssl, "ssl module required")
1324+
def test_url_with_control_char_rejected(self):
1325+
for char_no in range(0, 0x21) + range(0x7f, 0x100):
1326+
char = chr(char_no)
1327+
schemeless_url = "//localhost:7777/test%s/" % char
1328+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
1329+
try:
1330+
# We explicitly test urllib.request.urlopen() instead of the top
1331+
# level 'def urlopen()' function defined in this... (quite ugly)
1332+
# test suite. They use different url opening codepaths. Plain
1333+
# urlopen uses FancyURLOpener which goes via a codepath that
1334+
# calls urllib.parse.quote() on the URL which makes all of the
1335+
# above attempts at injection within the url _path_ safe.
1336+
escaped_char_repr = repr(char).replace('\\', r'\\')
1337+
InvalidURL = httplib.InvalidURL
1338+
with self.assertRaisesRegexp(
1339+
InvalidURL, "contain control.*" + escaped_char_repr):
1340+
urllib2.urlopen("http:" + schemeless_url)
1341+
with self.assertRaisesRegexp(
1342+
InvalidURL, "contain control.*" + escaped_char_repr):
1343+
urllib2.urlopen("https:" + schemeless_url)
1344+
finally:
1345+
self.unfakehttp()
1346+
1347+
@unittest.skipUnless(ssl, "ssl module required")
1348+
def test_url_with_newline_header_injection_rejected(self):
1349+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
1350+
host = "localhost:7777?a=1 HTTP/1.1\r\nX-injected: header\r\nTEST: 123"
1351+
schemeless_url = "//" + host + ":8080/test/?test=a"
1352+
try:
1353+
# We explicitly test urllib2.urlopen() instead of the top
1354+
# level 'def urlopen()' function defined in this... (quite ugly)
1355+
# test suite. They use different url opening codepaths. Plain
1356+
# urlopen uses FancyURLOpener which goes via a codepath that
1357+
# calls urllib.parse.quote() on the URL which makes all of the
1358+
# above attempts at injection within the url _path_ safe.
1359+
InvalidURL = httplib.InvalidURL
1360+
with self.assertRaisesRegexp(
1361+
InvalidURL, r"contain control.*\\r.*(found at least . .)"):
1362+
urllib2.urlopen("http:" + schemeless_url)
1363+
with self.assertRaisesRegexp(InvalidURL, r"contain control.*\\n"):
1364+
urllib2.urlopen("https:" + schemeless_url)
1365+
finally:
1366+
self.unfakehttp()
1367+
1368+
13201369

13211370
class RequestTests(unittest.TestCase):
13221371

Collapse file

‎Lib/test/test_xmlrpc.py‎

Copy file name to clipboardExpand all lines: Lib/test/test_xmlrpc.py
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,13 @@ def test_dotted_attribute(self):
659659
def test_partial_post(self):
660660
# Check that a partial POST doesn't make the server loop: issue #14001.
661661
conn = httplib.HTTPConnection(ADDR, PORT)
662-
conn.request('POST', '/RPC2 HTTP/1.0\r\nContent-Length: 100\r\n\r\nbye')
662+
conn.send('POST /RPC2 HTTP/1.0\r\n'
663+
'Content-Length: 100\r\n\r\n'
664+
'bye HTTP/1.1\r\n'
665+
'Host: %s:%s\r\n'
666+
'Accept-Encoding: identity\r\n'
667+
'Content-Length: 0\r\n\r\n'
668+
% (ADDR, PORT))
663669
conn.close()
664670

665671
class SimpleServerEncodingTestCase(BaseServerTestCase):
Collapse file

‎Misc/NEWS.d/next/Security/2019-04-10-08-53-30.bpo-30458.51E-DA.rst‎

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Address CVE-2019-9740 by disallowing URL paths with embedded whitespace or control characters through into the underlying http client request. Such potentially malicious header injection URLs now cause an httplib.InvalidURL exception to be raised.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.