Skip to content

Commit 09d8172

Browse files
authored
[3.5] closes bpo-38576: Disallow control characters in hostnames in http.client. (#19300)
Add host validation for control characters for more CVE-2019-18348 protection. (cherry picked from commit 83fc701)
1 parent 37fe316 commit 09d8172

File tree

4 files changed

+59
-3
lines changed

4 files changed

+59
-3
lines changed

Lib/http/client.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,7 @@ def __init__(self, host, port=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT,
771771

772772
(self.host, self.port) = self._get_hostport(host, port)
773773

774+
self._validate_host(self.host)
774775
# This is stored as an instance variable to allow unit
775776
# tests to replace it with a suitable mockup
776777
self._create_connection = socket.create_connection
@@ -1085,6 +1086,17 @@ def _validate_path(self, url):
10851086
).format(matched=match.group(), **locals())
10861087
raise InvalidURL(msg)
10871088

1089+
def _validate_host(self, host):
1090+
"""Validate a host so it doesn't contain control characters."""
1091+
# Prevent CVE-2019-18348.
1092+
match = _contains_disallowed_url_pchar_re.search(host)
1093+
if match:
1094+
msg = (
1095+
"URL can't contain control characters. {host!r} "
1096+
"(found at least {matched!r})"
1097+
).format(matched=match.group(), host=host)
1098+
raise InvalidURL(msg)
1099+
10881100
def putheader(self, header, *values):
10891101
"""Send a request header line to the server.
10901102

Lib/test/test_httplib.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -986,7 +986,7 @@ def run_server():
986986
thread.join()
987987
self.assertEqual(result, b"proxied data\n")
988988

989-
def test_putrequest_override_validation(self):
989+
def test_putrequest_override_domain_validation(self):
990990
"""
991991
It should be possible to override the default validation
992992
behavior in putrequest (bpo-38216).
@@ -999,6 +999,17 @@ def _validate_path(self, url):
999999
conn.sock = FakeSocket('')
10001000
conn.putrequest('GET', '/\x00')
10011001

1002+
def test_putrequest_override_host_validation(self):
1003+
class UnsafeHTTPConnection(client.HTTPConnection):
1004+
def _validate_host(self, url):
1005+
pass
1006+
1007+
conn = UnsafeHTTPConnection('example.com\r\n')
1008+
conn.sock = FakeSocket('')
1009+
# set skip_host so a ValueError is not raised upon adding the
1010+
# invalid URL as the value of the "Host:" header
1011+
conn.putrequest('GET', '/', skip_host=1)
1012+
10021013
def test_putrequest_override_encoding(self):
10031014
"""
10041015
It should be possible to override the default encoding

Lib/test/test_urllib.py

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ def test_willclose(self):
331331
self.unfakehttp()
332332

333333
@unittest.skipUnless(ssl, "ssl module required")
334-
def test_url_with_control_char_rejected(self):
334+
def test_url_path_with_control_char_rejected(self):
335335
for char_no in list(range(0, 0x21)) + [0x7f]:
336336
char = chr(char_no)
337337
schemeless_url = "//localhost:7777/test{}/".format(char)
@@ -360,7 +360,7 @@ def test_url_with_control_char_rejected(self):
360360
self.unfakehttp()
361361

362362
@unittest.skipUnless(ssl, "ssl module required")
363-
def test_url_with_newline_header_injection_rejected(self):
363+
def test_url_path_with_newline_header_injection_rejected(self):
364364
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
365365
host = "localhost:7777?a=1 HTTP/1.1\r\nX-injected: header\r\nTEST: 123"
366366
schemeless_url = "//" + host + ":8080/test/?test=a"
@@ -385,6 +385,38 @@ def test_url_with_newline_header_injection_rejected(self):
385385
finally:
386386
self.unfakehttp()
387387

388+
@unittest.skipUnless(ssl, "ssl module required")
389+
def test_url_host_with_control_char_rejected(self):
390+
for char_no in list(range(0, 0x21)) + [0x7f]:
391+
char = chr(char_no)
392+
schemeless_url = "//localhost{}/test/".format(char)
393+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
394+
try:
395+
escaped_char_repr = repr(char).replace('\\', r'\\')
396+
InvalidURL = http.client.InvalidURL
397+
with self.assertRaisesRegex(
398+
InvalidURL, r"contain control.*{}".format(escaped_char_repr)):
399+
urlopen("http:{}".format(schemeless_url))
400+
with self.assertRaisesRegex(InvalidURL, r"contain control.*{}".format(escaped_char_repr)):
401+
urlopen("http:{}".format(schemeless_url))
402+
finally:
403+
self.unfakehttp()
404+
405+
@unittest.skipUnless(ssl, "ssl module required")
406+
def test_url_host_with_newline_header_injection_rejected(self):
407+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
408+
host = "localhost\r\nX-injected: header\r\n"
409+
schemeless_url = "//" + host + ":8080/test/?test=a"
410+
try:
411+
InvalidURL = http.client.InvalidURL
412+
with self.assertRaisesRegex(
413+
InvalidURL, r"contain control.*\\r"):
414+
urlopen("http:{}".format(schemeless_url))
415+
with self.assertRaisesRegex(InvalidURL, r"contain control.*\\n"):
416+
urlopen("http:{}".format(schemeless_url))
417+
finally:
418+
self.unfakehttp()
419+
388420
def test_read_0_9(self):
389421
# "0.9" response accepted (but not "simple responses" without
390422
# a status line)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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.

0 commit comments

Comments
 (0)