From afb7ee6ff4809796a56877efd3ef1bcd50603542 Mon Sep 17 00:00:00 2001 From: amiremohamadi Date: Wed, 12 Feb 2020 17:22:08 +0330 Subject: [PATCH 1/4] reject control characters within http method name --- Lib/http/client.py | 14 ++++++++++++++ .../2020-02-12-14-17-39.bpo-39603.Gt3RSg.rst | 2 ++ 2 files changed, 16 insertions(+) create mode 100644 Misc/NEWS.d/next/Security/2020-02-12-14-17-39.bpo-39603.Gt3RSg.rst diff --git a/Lib/http/client.py b/Lib/http/client.py index 33a434733f8a46..76550d43208b41 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -147,6 +147,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'} @@ -1086,6 +1090,7 @@ def putrequest(self, method, url, skip_host=False, raise CannotSendRequest(self.__state) # Save the method for use later in the response phase + self._validate_method(method) self._method = method url = url or '/' @@ -1175,6 +1180,15 @@ 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( + f"method can't contain control characters. {method!r} " + f"(found at least {match.group()!r})") + def _validate_path(self, url): """Validate a url for putrequest.""" # Prevent CVE-2019-9740. 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..2fd85da3a42731 --- /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 encoding method parameter in +http.client.putrequest(...) to ASCII. From b14bd482de4ccabca8a37863014c38f7db4fd74a Mon Sep 17 00:00:00 2001 From: AMIR <31338382+amiremohamadi@users.noreply.github.com> Date: Wed, 12 Feb 2020 17:28:19 +0330 Subject: [PATCH 2/4] Update 2020-02-12-14-17-39.bpo-39603.Gt3RSg.rst --- .../next/Security/2020-02-12-14-17-39.bpo-39603.Gt3RSg.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 2fd85da3a42731..990affc3edd9d8 100644 --- 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 @@ -1,2 +1,2 @@ -Prevent http header injection by encoding method parameter in -http.client.putrequest(...) to ASCII. +Prevent http header injection by rejecting control characters in +http.client.putrequest(...). From 31ae5136bbe2ef614a6ee1c798edb4c2e93bdaa1 Mon Sep 17 00:00:00 2001 From: amiremohamadi Date: Wed, 12 Feb 2020 22:54:13 +0330 Subject: [PATCH 3/4] add test for rejecting bad HTTP method names --- Lib/test/test_httplib.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index e1aa41421febf4..249c5fafc77e98 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -364,6 +364,28 @@ def test_headers_debuglevel(self): self.assertEqual(lines[3], "header: Second: val2") +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 TransferEncodingTest(TestCase): expected_body = b"It's just a flesh wound" From 26ae13000de2d616f9e33c77c953db1a938fefc6 Mon Sep 17 00:00:00 2001 From: amiremohamadi Date: Sat, 18 Jul 2020 22:35:58 +0430 Subject: [PATCH 4/4] fix comment position --- Lib/http/client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/http/client.py b/Lib/http/client.py index 76550d43208b41..c708ec2218c426 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -1089,8 +1089,9 @@ def putrequest(self, method, url, skip_host=False, else: raise CannotSendRequest(self.__state) - # Save the method for use later in the response phase self._validate_method(method) + + # Save the method for use later in the response phase self._method = method url = url or '/'