8000 bpo-30458: Disallow control chars in http URLs. (GH-12755) (#13207) · stackless-dev/stackless@afe3a49 · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

Commit afe3a49

Browse files
hroncoklarryhastings
hroncok
authored and
larryhastings
committed
bpo-30458: Disallow control chars in http URLs. (pythonGH-12755) (python#13207)
Disallow control chars in http URLs in urllib.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 (pythonGH-13032) These tests require an SSL enabled build. Skip these tests when python is built without SSL to fix test failures. Use http.client.InvalidURL instead of ValueError as the new error case's exception. (pythonGH-13044) Co-Authored-By: Miro Hrončok <miro@hroncok.cz>
1 parent 4655d57 commit afe3a49

File tree

4 files changed

+79
-1
lines changed

4 files changed

+79
-1
lines changed

‎Lib/http/client.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,16 @@
141141
_is_legal_header_name = re.compile(rb'[^:\s][^:\r\n]*').fullmatch
142142
_is_illegal_header_value = re.compile(rb'\n(?![ \t])|\r(?![ \t\n])').search
143143

144+
# These characters are not allowed within HTTP URL paths.
145+
# See https://tools.ietf.org/html/rfc3986#section-3.3 and the
146+
# https://tools.ietf.org/html/rfc3986#appendix-A pchar definition.
147+
# Prevents CVE-2019-9740. Includes control characters such as \r\n.
148+
# We don't restrict chars above \x7f as putrequest() limits us to ASCII.
149+
_contains_disallowed_url_pchar_re = re.compile('[\x00-\x20\x7f]')
150+
# Arguably only these _should_ allowed:
151+
# _is_allowed_url_pchars_re = re.compile(r"^[/!$&'()*+,;=:@%a-zA-Z0-9._~-]+$")
152+
# We are more lenient for assumed real world compatibility purposes.
153+
144154
# We always set the Content-Length header for these methods because some
145155
# servers will otherwise respond with a 411
146156
_METHODS_EXPECTING_BODY = {'PATCH', 'POST', 'PUT'}
@@ -978,6 +988,12 @@ def putrequest(self, method, url, skip_host=False,
978988
self._method = method
979989
if not url:
980990
url = '/'
991+
# Prevent CVE-2019-9740.
992+
match = _contains_disallowed_url_pchar_re.search(url)
993+
if match:
994+
raise InvalidURL("URL can't contain control characters. {!r} "
995+
"(found at least {!r})".format(url,
996+
match.group()))
981997
request = '%s %s %s' % (method, url, self._http_vsn_str)
982998

983999
# Non-ASCII characters should have been eliminated earlier

‎Lib/test/test_urllib.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,61 @@ def test_willclose(self):
330330
finally:
331331
self.unfakehttp()
332332

333+
@unittest.skipUnless(ssl, "ssl module required")
334+
def test_url_with_control_char_rejected(self):
335+
for char_no in list(range(0, 0x21)) + [0x7f]:
336+
char = chr(char_no)
337+
schemeless_url = "//localhost:7777/test{}/".format(char)
338+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
339+
try:
340+
# We explicitly test urllib.request.urlopen() instead of the top
341+
# level 'def urlopen()' function defined in this... (quite ugly)
342+
# test suite. They use different url opening codepaths. Plain
343+
# urlopen uses FancyURLOpener which goes via a codepath that
344+
# calls urllib.parse.quote() on the URL which makes all of the
345+
# above attempts at injection within the url _path_ safe.
346+
escaped_char_repr = repr(char).replace('\\', r'\\')
347+
InvalidURL = http.client.InvalidURL
348+
with self.assertRaisesRegex(
349+
InvalidURL,
350+
"contain control.*{}".format(escaped_char_repr)):
351+
urllib.request.urlopen("http:{}".format(schemeless_url))
352+
with self.assertRaisesRegex(
353+
InvalidURL,
354+
"contain control.*{}".format(escaped_char_repr)):
355+
urllib.request.urlopen("https:{}".format(schemeless_url))
356+
# This code path quotes the URL so there is no injection.
357+
resp = urlopen("http:{}".format(schemeless_url))
358+
self.assertNotIn(char, resp.geturl())
359+
finally:
360+
self.unfakehttp()
361+
362+
@unittest.skipUnless(ssl, "ssl module required")
363+
def test_url_with_newline_header_injection_rejected(self):
364+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
365+
host = "localhost:7777?a=1 HTTP/1.1\r\nX-injected: header\r\nTEST: 123"
366+
schemeless_url = "//" + host + ":8080/test/?test=a"
367+
try:
368+
# We explicitly test urllib.request.urlopen() instead of the top
369+
# level 'def urlopen()' function defined in this... (quite ugly)
370+
# test suite. They use different url opening codepaths. Plain
371+
# urlopen uses FancyURLOpener which goes via a codepath that
372+
# calls urllib.parse.quote() on the URL which makes all of the
373+
# above attempts at injection within the url _path_ safe.
374+
InvalidURL = http.client.InvalidURL
375+
with self.assertRaisesRegex(
376+
InvalidURL, r"contain control.*\\r.*(found at least . .)"):
377+
urllib.request.urlopen("http:{}".format(schemeless_url))
378+
with self.assertRaisesRegex(InvalidURL, r"contain control.*\\n"):
379+
urllib.request.urlopen("https:{}".format(schemeless_url))
380+
# This code path quotes the URL so there is no injection.
381+
resp = urlopen("http:{}".format(schemeless_url))
382+
self.assertNotIn(' ', resp.geturl())
383+
self.assertNotIn('\r', resp.geturl())
384+
self.assertNotIn('\n', resp.geturl())
385+
finally:
386+
self.unfakehttp()
387+
333388
def test_read_0_9(self):
334389
# "0.9" response accepted (but not "simple responses" without
335390 # a status line)

‎Lib/test/test_xmlrpc.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -896,7 +896,13 @@ def test_unicode_host(self):
896896
def test_partial_post(self):
897897
# Check that a partial POST doesn't make the server loop: issue #14001.
898898
conn = http.client.HTTPConnection(ADDR, PORT)
899-
conn.request('POST', '/RPC2 HTTP/1.0\r\nContent-Length: 100\r\n\r\nbye')
899+
conn.send('POST /RPC2 HTTP/1.0\r\n'
900+
'Content-Length: 100\r\n\r\n'
901+
'bye HTTP/1.1\r\n'
902+
'Host: {}:{}\r\n'
903+
'Accept-Encoding: identity\r\n'
904+
'Content-Length: 0\r\n\r\n'
905+
.format(ADDR, PORT).encode('ascii'))
900906
conn.close()
901907

902908
def test_context_manager(self):

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

Lines changed: 1 addition & 0 deletions
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 http.client.InvalidURL exception to be raised.

0 commit comments

Comments
 (0)
0