[CVE-2019-18348]Disallow control characters in hostnames in http.client

An issue was discovered in urllib2 in Python 2.x through 2.7.17
and urllib in Python 3.x through 3.8.0. CRLF injection is possible
if the attacker controls a url parameter, as demonstrated
by the first argument to urllib.request.urlopen with \r\n
(specifically in the host component of a URL) followed
by an HTTP header. This is similar to the CVE-2019-9740
query string issue and the CVE-2019-9947 path string issue.
(This is not exploitable when glibc has CVE-2016-10739 fixed.).
This is fixed in: v2.7.18, v2.7.18rc1; v3.5.10, v3.5.10rc1; v3.6.11,
v3.6.11rc1, v3.6.12; v3.7.8, v3.7.8rc1, v3.7.9; v3.8.3,
v3.8.3rc1, v3.8.4, v3.8.4rc1, v3.8.5, v3.8.6, v3.8.6rc1.

Change-Id: I9f29bdc023cd909dd7cc5250f1faba0940d591db
Signed-off-by: JinWang An <jinwang.an@samsung.com>

diff --git a/Lib/http/client.py b/Lib/http/client.py
index 09ffa7510f71f086f6a61269591e87edbc8b67e5..9ba549429e68c0468b992b1d8e3173cf8deb125d 100644 (file)
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@ -844,6 +844,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
@@ -1189,6 +1191,15 @@ class HTTPConnection:
             # For HTTP/1.0, the server will assume "not chunked"
             pass
 
+    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.
 

diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index c731ddb71f26d5d15bbe0bb6bf4b8a04ae38adc4..cff0ae0f388fda7b90f79162c2644b19e4e84232 100644 (file)
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -1176,6 +1176,17 @@ class BasicTest(TestCase):
         thread.join()
         self.assertEqual(result, b"proxied data\n")
 
+    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)
+
 class ExtendedReadTest(TestCase):
     """
     Test peek(), read1(), readline()

diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py
index 2ac73b58d832064f8328d6d06eb90135056c7c59..6d5b022c0fac8f99a0b7635ba8b6f711eae8797b 100644 (file)
--- a/Lib/test/test_urllib.py
+++ b/Lib/test/test_urllib.py
@@ -465,6 +465,38 @@ Connection: close
         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_URLopener_deprecation(self):
         with support.check_warnings(('',DeprecationWarning)):
             urllib.request.URLopener()