[3.15] gh-150743: Limit trailer lines and interim responses read by http.client (GH-150741) (#152523)

gh-150743: Limit trailer lines and interim responses read by http.client (GH-150741)

http.client read chunked-response trailer lines and skipped interim (1xx)
responses in unbounded loops, so a server streaming either forever would
hang the client even with a socket timeout set (data keeps arriving, so
the timeout never fires).

Trailer lines are now limited to max_response_headers (100 by default)
and interim responses to 100; HTTPException is raised past either limit.

Follow-up to gh-88188 for CVE-2021-3737, which bounded header lines
within an interim response but not these two sibling loops.
(cherry picked from commit 41cc78a)

Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>

---

This issue was reported to us via [GHSA-w4q2-g22w-6fr4](https://github.com/python/cpython/security/advisories/GHSA-w4q2-g22w-6fr4) and was determined not to be high enough severity to handle privately.

Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>

Author: miss-islington

Doc/library/http.client.rst

@@ -433,6 +433,7 @@ HTTPConnection Objects
 
    The maximum number of allowed response headers to help prevent denial-of-service
    attacks. By default, the maximum number of allowed headers is set to 100.
+   The same limit applies to the trailer section of a chunked response.
 
    .. versionadded:: 3.15
 

Lib/http/client.py

@@ -111,6 +111,13 @@
 _MAXLINE = 65536
 _MAXHEADERS = 100
 
+# maximal number of interim (1xx) responses tolerated before the final
+# response.  Real servers send at most a few; without a bound, a server
+# streaming "100 Continue" responses would hang getresponse() forever.
+# A socket timeout cannot detect that as data keeps arriving within every
+# timeout window.
+_MAXINTERIMRESPONSES = 100
+
 # Data larger than this will be read in chunks, to prevent extreme
 # overallocation.
 _MIN_READ_BUF_SIZE = 1 << 20
@@ -293,6 +300,7 @@ def __init__(self, sock, debuglevel=0, method=None, url=None):
         self.chunk_left = _UNKNOWN      # bytes left to read in current chunk
         self.length = _UNKNOWN          # number of bytes left in response
         self.will_close = _UNKNOWN      # conn will close at end of response
+        self._max_headers = None        # configured header count limit
 
     def _read_status(self):
         line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
@@ -332,8 +340,13 @@ def begin(self, *, _max_headers=None):
             # we've already started reading the response
             return
 
+        # Trailers of a chunked response are read by read() long after
+        # begin() returns, so remember the configured header count limit
+        # for _read_and_discard_trailer() to enforce.
+        self._max_headers = _max_headers
+
         # read until we get a non-100 response
-        while True:
+        for _ in range(_MAXINTERIMRESPONSES):
             version, status, reason = self._read_status()
             if status != CONTINUE:
                 break
@@ -342,6 +355,9 @@ def begin(self, *, _max_headers=None):
             if self.debuglevel > 0:
                 print("headers:", skipped_headers)
             del skipped_headers
+        else:
+            raise HTTPException(
+                f"got more than {_MAXINTERIMRESPONSES} interim responses")
 
         self.code = self.status = status
         self.reason = reason.strip()
@@ -561,6 +577,10 @@ def _read_next_chunk_size(self):
     def _read_and_discard_trailer(self):
         # read and discard trailer up to the CRLF terminator
         ### note: we shouldn't have any trailers!
+        max_trailers = self._max_headers
+        if max_trailers is None:
+            max_trailers = _MAXHEADERS
+        trailers_read = 0
         while True:
             line = self.fp.readline(_MAXLINE + 1)
             if len(line) > _MAXLINE:
@@ -571,6 +591,13 @@ def _read_and_discard_trailer(self):
                 break
             if line in (b'\r\n', b'\n', b''):
                 break
+            # Bound the trailer count just as response headers are bounded.
+            # A server streaming trailer lines forever would otherwise hang
+            # the client; a socket timeout cannot detect that as data keeps
+            # arriving within every timeout window.
+            trailers_read += 1
+            if trailers_read > max_trailers:
+                raise HTTPException(f"got more than {max_trailers} trailers")
 
     def _get_chunk_left(self):
         # return self.chunk_left, reading a new chunk if necessary.

Lib/test/test_httplib.py

@@ -478,6 +478,35 @@ def test_max_connection_headers(self):
             response = conn.getresponse()
             response.read()
 
+    def test_max_connection_trailers(self):
+        # max_response_headers also limits trailer lines of a chunked
+        # response, which are read and discarded by read().
+        max_trailers = client._MAXHEADERS + 20
+        trailer_lines = "".join(
+            f"X-Trailer{i}: {i}\r\n" for i in range(max_trailers - 1)
+        )
+        body = chunked_start + last_chunk + trailer_lines + chunked_end
+
+        with self.subTest(max_response_headers=None):
+            conn = client.HTTPConnection("example.com")
+            conn.sock = FakeSocket(body)
+            conn.request("GET", "/")
+            response = conn.getresponse()
+            with self.assertRaisesRegex(
+                client.HTTPException,
+                f"got more than {client._MAXHEADERS} trailers",
+            ):
+                response.read()
+
+        with self.subTest(max_response_headers=max_trailers):
+            conn = client.HTTPConnection(
+                "example.com", max_response_headers=max_trailers
+            )
+            conn.sock = FakeSocket(body)
+            conn.request("GET", "/")
+            response = conn.getresponse()
+            self.assertEqual(response.read(), chunked_expected)
+
 class HttpMethodTests(TestCase):
     def test_invalid_method_names(self):
         methods = (
@@ -1378,6 +1407,35 @@ def test_overflowing_header_limit_after_100(self):
         self.assertIn('got more than ', str(cm.exception))
         self.assertIn('headers', str(cm.exception))
 
+    def test_too_many_interim_responses(self):
+        # A server streaming "100 Continue" responses forever must not
+        # hang getresponse().
+        body = (
+            'HTTP/1.1 100 Continue\r\n\r\n'
+            * (client._MAXINTERIMRESPONSES + 1)
+        )
+        resp = client.HTTPResponse(FakeSocket(body))
+        with self.assertRaises(client.HTTPException) as cm:
+            resp.begin()
+        self.assertIn('got more than ', str(cm.exception))
+        self.assertIn('interim responses', str(cm.exception))
+
+    def test_multiple_interim_responses(self):
+        # A reasonable number of interim responses before the final
+        # response is skipped as before.
+        body = (
+            'HTTP/1.1 100 Continue\r\n\r\n' * 3 +
+            'HTTP/1.1 200 OK\r\n'
+            'Content-Length: 5\r\n'
+            '\r\n'
+            'hello'
+        )
+        resp = client.HTTPResponse(FakeSocket(body), method="GET")
+        resp.begin()
+        self.assertEqual(resp.status, 200)
+        self.assertEqual(resp.read(), b'hello')
+        resp.close()
+
     def test_overflowing_chunked_line(self):
         body = (
             'HTTP/1.1 200 OK\r\n'
@@ -1449,6 +1507,35 @@ def test_chunked_trailers(self):
         self.assertEqual(sock.file.read(), b"") #we read to the end
         resp.close()
 
+    def test_chunked_too_many_trailers(self):
+        """A response streaming endless trailer lines must raise, not hang"""
+        too_many_trailers = "".join(
+            f"X-Trailer{i}: {i}\r\n" for i in range(client._MAXHEADERS + 1)
+        )
+        # An unbounded read() reaches the trailers via the final 0 chunk.
+        sock = FakeSocket(
+            chunked_start + last_chunk + too_many_trailers + chunked_end)
+        resp = client.HTTPResponse(sock, method="GET")
+        resp.begin()
+        with self.assertRaisesRegex(
+            client.HTTPException,
+            f"got more than {client._MAXHEADERS} trailers",
+        ):
+            resp.read()
+        resp.close()
+
+        # A bounded read(amt) larger than the body hits the same limit.
+        sock = FakeSocket(
+            chunked_start + last_chunk + too_many_trailers + chunked_end)
+        resp = client.HTTPResponse(sock, method="GET")
+        resp.begin()
+        with self.assertRaisesRegex(
+            client.HTTPException,
+            f"got more than {client._MAXHEADERS} trailers",
+        ):
+            resp.read(len(chunked_expected) + 1)
+        resp.close()
+
     def test_chunked_sync(self):
         """Check that we don't read past the end of the chunked-encoding stream"""
         expected = chunked_expected

Misc/NEWS.d/next/Security/2026-05-30-00-00-00.gh-issue-150743.httpdos.rst

@@ -0,0 +1,7 @@
+:mod:`http.client` now limits the number of chunked-response trailer lines
+it will read to :attr:`~http.client.HTTPConnection.max_response_headers`
+(100 by default), and the number of interim (1xx) responses it will skip
+to 100.  A malicious or broken server could previously stream trailer
+lines or ``100 Continue`` responses forever, hanging the client even when
+a socket timeout was in use.
+Reported by ``@YLChen-007`` via GHSA-w4q2-g22w-6fr4.