bpo-38576: Disallow control characters in hostnames in http.client (GH-18995)
Add host validation for control characters for more CVE-2019-18348 protection.
This commit is contained in:
parent
6672c16b1d
commit
9165addc22
|
@ -828,6 +828,8 @@ class HTTPConnection:
|
|||
|
||||
(self.host, self.port) = self._get_hostport(host, port)
|
||||
|
||||
self._validate_host(self.host)
|
||||
|
||||
# This is stored as an instance variable to allow unit
|
||||
# tests to replace it with a suitable mockup
|
||||
self._create_connection = socket.create_connection
|
||||
|
@ -1183,6 +1185,14 @@ class HTTPConnection:
|
|||
raise InvalidURL(f"URL can't contain control characters. {url!r} "
|
||||
f"(found at least {match.group()!r})")
|
||||
|
||||
def _validate_host(self, host):
|
||||
"""Validate a host so it doesn't contain control characters."""
|
||||
# Prevent CVE-2019-18348.
|
||||
match = _contains_disallowed_url_pchar_re.search(host)
|
||||
if match:
|
||||
raise InvalidURL(f"URL can't contain control characters. {host!r} "
|
||||
f"(found at least {match.group()!r})")
|
||||
|
||||
def putheader(self, header, *values):
|
||||
"""Send a request header line to the server.
|
||||
|
||||
|
|
|
@ -1155,7 +1155,7 @@ class BasicTest(TestCase):
|
|||
thread.join()
|
||||
self.assertEqual(result, b"proxied data\n")
|
||||
|
||||
def test_putrequest_override_validation(self):
|
||||
def test_putrequest_override_domain_validation(self):
|
||||
"""
|
||||
It should be possible to override the default validation
|
||||
behavior in putrequest (bpo-38216).
|
||||
|
@ -1168,6 +1168,17 @@ class BasicTest(TestCase):
|
|||
conn.sock = FakeSocket('')
|
||||
conn.putrequest('GET', '/\x00')
|
||||
|
||||
def test_putrequest_override_host_validation(self):
|
||||
class UnsafeHTTPConnection(client.HTTPConnection):
|
||||
def _validate_host(self, url):
|
||||
pass
|
||||
|
||||
conn = UnsafeHTTPConnection('example.com\r\n')
|
||||
conn.sock = FakeSocket('')
|
||||
# set skip_host so a ValueError is not raised upon adding the
|
||||
# invalid URL as the value of the "Host:" header
|
||||
conn.putrequest('GET', '/', skip_host=1)
|
||||
|
||||
def test_putrequest_override_encoding(self):
|
||||
"""
|
||||
It should be possible to override the default encoding
|
||||
|
|
|
@ -370,7 +370,7 @@ class urlopen_HttpTests(unittest.TestCase, FakeHTTPMixin, FakeFTPMixin):
|
|||
self.unfakehttp()
|
||||
|
||||
@unittest.skipUnless(ssl, "ssl module required")
|
||||
def test_url_with_control_char_rejected(self):
|
||||
def test_url_path_with_control_char_rejected(self):
|
||||
for char_no in list(range(0, 0x21)) + [0x7f]:
|
||||
char = chr(char_no)
|
||||
schemeless_url = f"//localhost:7777/test{char}/"
|
||||
|
@ -397,7 +397,7 @@ class urlopen_HttpTests(unittest.TestCase, FakeHTTPMixin, FakeFTPMixin):
|
|||
self.unfakehttp()
|
||||
|
||||
@unittest.skipUnless(ssl, "ssl module required")
|
||||
def test_url_with_newline_header_injection_rejected(self):
|
||||
def test_url_path_with_newline_header_injection_rejected(self):
|
||||
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
|
||||
host = "localhost:7777?a=1 HTTP/1.1\r\nX-injected: header\r\nTEST: 123"
|
||||
schemeless_url = "//" + host + ":8080/test/?test=a"
|
||||
|
@ -422,6 +422,38 @@ class urlopen_HttpTests(unittest.TestCase, FakeHTTPMixin, FakeFTPMixin):
|
|||
finally:
|
||||
self.unfakehttp()
|
||||
|
||||
@unittest.skipUnless(ssl, "ssl module required")
|
||||
def test_url_host_with_control_char_rejected(self):
|
||||
for char_no in list(range(0, 0x21)) + [0x7f]:
|
||||
char = chr(char_no)
|
||||
schemeless_url = f"//localhost{char}/test/"
|
||||
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
|
||||
try:
|
||||
escaped_char_repr = repr(char).replace('\\', r'\\')
|
||||
InvalidURL = http.client.InvalidURL
|
||||
with self.assertRaisesRegex(
|
||||
InvalidURL, f"contain control.*{escaped_char_repr}"):
|
||||
urlopen(f"http:{schemeless_url}")
|
||||
with self.assertRaisesRegex(InvalidURL, f"contain control.*{escaped_char_repr}"):
|
||||
urlopen(f"https:{schemeless_url}")
|
||||
finally:
|
||||
self.unfakehttp()
|
||||
|
||||
@unittest.skipUnless(ssl, "ssl module required")
|
||||
def test_url_host_with_newline_header_injection_rejected(self):
|
||||
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
|
||||
host = "localhost\r\nX-injected: header\r\n"
|
||||
schemeless_url = "//" + host + ":8080/test/?test=a"
|
||||
try:
|
||||
InvalidURL = http.client.InvalidURL
|
||||
with self.assertRaisesRegex(
|
||||
InvalidURL, r"contain control.*\\r"):
|
||||
urlopen(f"http:{schemeless_url}")
|
||||
with self.assertRaisesRegex(InvalidURL, r"contain control.*\\n"):
|
||||
urlopen(f"https:{schemeless_url}")
|
||||
finally:
|
||||
self.unfakehttp()
|
||||
|
||||
def test_read_0_9(self):
|
||||
# "0.9" response accepted (but not "simple responses" without
|
||||
# a status line)
|
||||
|
|
|
@ -0,0 +1 @@
|
|||
Disallow control characters in hostnames in http.client, addressing CVE-2019-18348. Such potentially malicious header injection URLs now cause a InvalidURL to be raised.
|
Loading…
Reference in New Issue