[Python-checkins] bpo-44022: Fix http client infinite line reading (DoS) after a HTTP 100 Continue (GH-25916)

miss-islington webhook-mailer at python.org
Wed May 5 19:06:01 EDT 2021


https://github.com/python/cpython/commit/ea9327036680acc92d9f89eaf6f6a54d2f8d78d9
commit: ea9327036680acc92d9f89eaf6f6a54d2f8d78d9
branch: 3.9
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: miss-islington <31488909+miss-islington at users.noreply.github.com>
date: 2021-05-05T16:05:52-07:00
summary:

bpo-44022: Fix http client infinite line reading (DoS) after a HTTP 100 Continue (GH-25916)


Fixes http.client potential denial of service where it could get stuck reading lines from a malicious server after a 100 Continue response.

Co-authored-by: Gregory P. Smith <greg at krypto.org>
(cherry picked from commit 47895e31b6f626bc6ce47d175fe9d43c1098909d)

Co-authored-by: Gen Xu <xgbarry at gmail.com>

files:
A Misc/NEWS.d/next/Security/2021-05-05-17-37-04.bpo-44022.bS3XJ9.rst
M Lib/http/client.py
M Lib/test/test_httplib.py

diff --git a/Lib/http/client.py b/Lib/http/client.py
index df417cf65e2043..975292505836e1 100644
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@ -201,15 +201,11 @@ def getallmatchingheaders(self, name):
                 lst.append(line)
         return lst
 
-def parse_headers(fp, _class=HTTPMessage):
-    """Parses only RFC2822 headers from a file pointer.
-
-    email Parser wants to see strings rather than bytes.
-    But a TextIOWrapper around self.rfile would buffer too many bytes
-    from the stream, bytes which we later need to read as bytes.
-    So we read the correct bytes here, as bytes, for email Parser
-    to parse.
+def _read_headers(fp):
+    """Reads potential header lines into a list from a file pointer.
 
+    Length of line is limited by _MAXLINE, and number of
+    headers is limited by _MAXHEADERS.
     """
     headers = []
     while True:
@@ -221,6 +217,19 @@ def parse_headers(fp, _class=HTTPMessage):
             raise HTTPException("got more than %d headers" % _MAXHEADERS)
         if line in (b'\r\n', b'\n', b''):
             break
+    return headers
+
+def parse_headers(fp, _class=HTTPMessage):
+    """Parses only RFC2822 headers from a file pointer.
+
+    email Parser wants to see strings rather than bytes.
+    But a TextIOWrapper around self.rfile would buffer too many bytes
+    from the stream, bytes which we later need to read as bytes.
+    So we read the correct bytes here, as bytes, for email Parser
+    to parse.
+
+    """
+    headers = _read_headers(fp)
     hstring = b''.join(headers).decode('iso-8859-1')
     return email.parser.Parser(_class=_class).parsestr(hstring)
 
@@ -308,15 +317,10 @@ def begin(self):
             if status != CONTINUE:
                 break
             # skip the header from the 100 response
-            while True:
-                skip = self.fp.readline(_MAXLINE + 1)
-                if len(skip) > _MAXLINE:
-                    raise LineTooLong("header line")
-                skip = skip.strip()
-                if not skip:
-                    break
-                if self.debuglevel > 0:
-                    print("header:", skip)
+            skipped_headers = _read_headers(self.fp)
+            if self.debuglevel > 0:
+                print("headers:", skipped_headers)
+            del skipped_headers
 
         self.code = self.status = status
         self.reason = reason.strip()
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index 0fe8ead3cef1e0..753590afffe624 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -1005,6 +1005,14 @@ def test_overflowing_header_line(self):
         resp = client.HTTPResponse(FakeSocket(body))
         self.assertRaises(client.LineTooLong, resp.begin)
 
+    def test_overflowing_header_limit_after_100(self):
+        body = (
+            'HTTP/1.1 100 OK\r\n'
+            'r\n' * 32768
+        )
+        resp = client.HTTPResponse(FakeSocket(body))
+        self.assertRaises(client.HTTPException, resp.begin)
+
     def test_overflowing_chunked_line(self):
         body = (
             'HTTP/1.1 200 OK\r\n'
@@ -1406,7 +1414,7 @@ def readline(self, limit):
 class OfflineTest(TestCase):
     def test_all(self):
         # Documented objects defined in the module should be in __all__
-        expected = {"responses"}  # White-list documented dict() object
+        expected = {"responses"}  # Allowlist documented dict() object
         # HTTPMessage, parse_headers(), and the HTTP status code constants are
         # intentionally omitted for simplicity
         blacklist = {"HTTPMessage", "parse_headers"}
diff --git a/Misc/NEWS.d/next/Security/2021-05-05-17-37-04.bpo-44022.bS3XJ9.rst b/Misc/NEWS.d/next/Security/2021-05-05-17-37-04.bpo-44022.bS3XJ9.rst
new file mode 100644
index 00000000000000..cf6b63e3961558
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2021-05-05-17-37-04.bpo-44022.bS3XJ9.rst
@@ -0,0 +1,2 @@
+mod:`http.client` now avoids infinitely reading potential HTTP headers after a
+``100 Continue`` status response from the server.



More information about the Python-checkins mailing list