8000 [2.7] closes bpo-38576: Disallow control characters in hostnames in h… · stackless-dev/stackless@e176e0c · 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 e176e0c

Browse files
mceplepicfaace
mcepl
and
epicfaace
authored
[2.7] closes bpo-38576: Disallow control characters in hostnames in http.client. (pythonGH-19052)
Add host validation for control characters for more CVE-2019-18348 protection. (cherry picked from commit 83fc701) Co-authored-by: Ashwin Ramaswami <aramaswamis@gmail.com>
1 parent 249706c commit e176e0c

File tree

4 files changed

+53
-8
lines changed

4 files changed

+53
-8
lines changed

‎Lib/httplib.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,8 @@ def __init__(self, host, port=None, strict=None,
745745

746746
(self.host, self.port) = self._get_hostport(host, port)
747747

748+
self._validate_host(self.host)
749+
748750
# This is stored as an instance variable to allow unittests
749751
# to replace with a suitable mock
750752
self._create_connection = socket.create_connection
@@ -1029,6 +1031,17 @@ def _validate_path(self, url):
10291031
).format(matched=match.group(), url=url)
10301032
raise InvalidURL(msg)
10311033

1034+
def _validate_host(self, host):
1035+
"""Validate a host so it doesn't contain control characters."""
1036+
# Prevent CVE-2019-18348.
1037+
match = _contains_disallowed_url_pchar_re.search(host)
1038+
if match:
1039+
msg = (
1040+
"URL can't contain control characters. {host!r} "
1041+
"(found at least {matched!r})"
1042+
).format(matched=match.group(), host=host)
1043+
raise InvalidURL(msg)
1044+
10321045
def putheader(self, header, *values):
10331046
"""Send a request header line to the server.
10341047

‎Lib/test/test_httplib.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -702,7 +702,7 @@ def test_proxy_tunnel_without_status_line(self):
702702
with self.assertRaisesRegexp(socket.error, "Invalid response"):
703703
conn._tunnel()
704704

705-
def test_putrequest_override_validation(self):
705+
def test_putrequest_override_domain_validation(self):
706706
"""
707707
It should be possible to override the default validation
708708
behavior in putrequest (bpo-38216).
@@ -715,6 +715,17 @@ def _validate_path(self, url):
715715
conn.sock = FakeSocket('')
716716
conn.putrequest('GET', '/\x00')
717717

718+
def test_putrequest_override_host_validation(self):
719+
class UnsafeHTTPConnection(httplib.HTTPConnection):
720+
def _validate_host(self, url):
721+
pass
722+
723+
conn = UnsafeHTTPConnection('example.com\r\n')
724+
conn.sock = FakeSocket('')
725+
# set skip_host so a ValueError is not raised upon adding the
726+
# invalid URL as the value of the "Host:" header
727+
conn.putrequest('GET', '/', skip_host=1)
728+
718729

719730
class OfflineTest(TestCase):
720731
def test_responses(self):

‎Lib/test/test_urllib2.py

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,7 +1321,7 @@ def test_unsupported_algorithm(self):
13211321
)
13221322

13231323
@unittest.skipUnless(ssl, "ssl module required")
1324-
def test_url_with_control_char_rejected(self):
1324+
def test_url_path_with_control_char_rejected(self):
13251325
for char_no in range(0, 0x21) + range(0x7f, 0x100):
13261326
char = chr(char_no)
13271327
schemeless_url = "//localhost:7777/test%s/" % char
@@ -1345,7 +1345,7 @@ def test_url_with_control_char_rejected(self):
13451345
self.unfakehttp()
13461346

13471347
@unittest.skipUnless(ssl, "ssl module required")
1348-
def test_url_with_newline_header_injection_rejected(self):
1348+
def test_url_path_with_newline_header_injection_rejected(self):
13491349
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
13501350
host = "localhost:7777?a=1 HTTP/1.1\r\nX-injected: header\r\nTEST: 123"
13511351
schemeless_url = "//" + host + ":8080/test/?test=a"
@@ -1357,14 +1357,32 @@ def test_url_with_newline_header_injection_rejected(self):
13571357
# calls urllib.parse.quote() on the URL which makes all of the
13581358
# above attempts at injection within the url _path_ safe.
13591359
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)
1360+
with self.assertRaisesRegexp(InvalidURL,
1361+
r"contain control.*\\r.*(found at least . .)"):
1362+
urllib2.urlopen("http:{}".format(schemeless_url))
1363+
with self.assertRaisesRegexp(InvalidURL,
1364+
r"contain control.*\\n"):
1365+
urllib2.urlopen("https:{}".format(schemeless_url))
13651366
finally:
13661367
self.unfakehttp()
13671368

1369+
@unittest.skipUnless(ssl, "ssl module required")
1370+
def test_url_host_with_control_char_rejected(self):
1371+
for char_no in list(range(0, 0x21)) + [0x7f]:
1372+
char = chr(char_no)
1373+
schemeless_url = "//localhost{}/test/".format(char)
1374+
self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
1375+
try:
1376+
escaped_char_repr = repr(char).replace('\\', r'\\')
1377+
InvalidURL = httplib.InvalidURL
1378+
with self.assertRaisesRegexp(InvalidURL,
1379+
"contain control.*{}".format(escaped_char_repr)):
1380+
urllib2.urlopen("http:{}".format(schemeless_url))
1381+
with self.assertRaisesRegexp(InvalidURL,
1382+
"contain control.*{}".format(escaped_char_repr)):
1383+
urllib2.urlopen("https:{}".format(schemeless_url))
1384+
finally:
1385+
self.unfakehttp()
13681386

13691387

13701388
class RequestTests(unittest.TestCase):

‎Misc/NEWS.d/next/Library/2020-03-18-01-30-50.bpo-38576.cvI68q.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Disallow control characters in hostnames in http.client, addressing
2+
CVE-2019-18348. Such potentially malicious header injection URLs now cause a
3+
InvalidURL to be raised.

0 commit comments

Comments
 (0)
0