diff --git a/Lib/http/client.py b/Lib/http/client.py index 5d5d7a7f707b40..f7efe151f5b048 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -151,6 +151,10 @@ # _is_allowed_url_pchars_re = re.compile(r"^[/!$&'()*+,;=:@%a-zA-Z0-9._~-]+$") # We are more lenient for assumed real world compatibility purposes. +# These characters are not allowed within HTTP method names +# to prevent http header injection. +_contains_disallowed_method_pchar_re = re.compile('[\x00-\x1f]') + # We always set the Content-Length header for these methods because some # servers will otherwise respond with a 411 _METHODS_EXPECTING_BODY = {'PATCH', 'POST', 'PUT'} @@ -985,6 +989,8 @@ def putrequest(self, method, url, skip_host=False, else: raise CannotSendRequest(self.__state) + self._validate_method(method) + # Save the method for use later in the response phase self._method = method @@ -1075,6 +1081,16 @@ def _encode_request(self, request): # ASCII also helps prevent CVE-2019-9740. return request.encode('ascii') + def _validate_method(self, method): + """Validate a method name for putrequest.""" + # prevent http header injection + match = _contains_disallowed_method_pchar_re.search(method) + if match: + raise ValueError( + "method can't contain control characters. %r " + "(found at least %r)" + % (method, match.group())) + def _validate_path(self, url): """Validate a url for putrequest.""" # Prevent CVE-2019-9740. diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index a8e7a30af1a9f0..b1cd08b23a4b12 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -344,6 +344,28 @@ def test_invalid_headers(self): conn.putheader(name, value) +class HttpMethodTests(TestCase): + def test_invalid_method_names(self): + methods = ( + 'GET\r', + 'POST\n', + 'PUT\n\r', + 'POST\nValue', + 'POST\nHOST:abc', + 'GET\nrHost:abc\n', + 'POST\rRemainder:\r', + 'GET\rHOST:\n', + '\nPUT' + ) + + for method in methods: + with self.assertRaisesRegex( + ValueError, "method can't contain control characters"): + conn = client.HTTPConnection('example.com') + conn.sock = FakeSocket(None) + conn.request(method=method, url="/") + + class BasicTest(TestCase): def test_status_lines(self): # Test HTTP status lines @@ -1783,8 +1805,8 @@ def test_tunnel_debuglog(self): @support.reap_threads def test_main(verbose=None): - support.run_unittest(HeaderTests, OfflineTest, BasicTest, TimeoutTest, - PersistenceTest, + support.run_unittest(HeaderTests, OfflineTest, HttpMethodTests, + BasicTest, TimeoutTest, PersistenceTest, HTTPSTest, RequestBodyTest, SourceAddressTest, HTTPResponseTest, ExtendedReadTest, ExtendedReadTestChunked, TunnelTests) diff --git a/Misc/NEWS.d/next/Security/2020-02-12-14-17-39.bpo-39603.Gt3RSg.rst b/Misc/NEWS.d/next/Security/2020-02-12-14-17-39.bpo-39603.Gt3RSg.rst new file mode 100644 index 00000000000000..990affc3edd9d8 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2020-02-12-14-17-39.bpo-39603.Gt3RSg.rst @@ -0,0 +1,2 @@ +Prevent http header injection by rejecting control characters in +http.client.putrequest(...).