Skip to content

Commit d804d28

Browse files
[3.13] gh-150743: Limit trailer lines and interim responses read by http.client (GH-150749) (#152526)
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 84badb7) 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) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 7c6d529 commit d804d28

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

Lib/http/client.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,13 @@
111111
_MAXLINE = 65536
112112
_MAXHEADERS = 100
113113

114+
# maximal number of interim (1xx) responses tolerated before the final
115+
# response. Real servers send at most a few; without a bound, a server
116+
# streaming "100 Continue" responses would hang getresponse() forever.
117+
# A socket timeout cannot detect that as data keeps arriving within every
118+
# timeout window.
119+
_MAXINTERIMRESPONSES = 100
120+
114121
# Data larger than this will be read in chunks, to prevent extreme
115122
# overallocation.
116123
_MIN_READ_BUF_SIZE = 1 << 20
@@ -332,7 +339,7 @@ def begin(self):
332339
return
333340

334341
# read until we get a non-100 response
335-
while True:
342+
for _ in range(_MAXINTERIMRESPONSES):
336343
version, status, reason = self._read_status()
337344
if status != CONTINUE:
338345
break
@@ -341,6 +348,9 @@ def begin(self):
341348
if self.debuglevel > 0:
342349
print("headers:", skipped_headers)
343350
del skipped_headers
351+
else:
352+
raise HTTPException(
353+
f"got more than {_MAXINTERIMRESPONSES} interim responses")
344354

345355
self.code = self.status = status
346356
self.reason = reason.strip()
@@ -558,6 +568,7 @@ def _read_next_chunk_size(self):
558568
def _read_and_discard_trailer(self):
559569
# read and discard trailer up to the CRLF terminator
560570
### note: we shouldn't have any trailers!
571+
trailers_read = 0
561572
while True:
562573
line = self.fp.readline(_MAXLINE + 1)
563574
if len(line) > _MAXLINE:
@@ -568,6 +579,14 @@ def _read_and_discard_trailer(self):
568579
break
569580
if line in (b'\r\n', b'\n', b''):
570581
break
582+
# Bound the trailer count just as response headers are bounded.
583+
# A server streaming trailer lines forever would otherwise hang
584+
# the client; a socket timeout cannot detect that as data keeps
585+
# arriving within every timeout window.
586+
trailers_read += 1
587+
if trailers_read > _MAXHEADERS:
588+
raise HTTPException(
589+
f"got more than {_MAXHEADERS} trailers")
571590

572591
def _get_chunk_left(self):
573592
# return self.chunk_left, reading a new chunk if necessary.

Lib/test/test_httplib.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1322,6 +1322,35 @@ def test_overflowing_header_limit_after_100(self):
13221322
self.assertIn('got more than ', str(cm.exception))
13231323
self.assertIn('headers', str(cm.exception))
13241324

1325+
def test_too_many_interim_responses(self):
1326+
# A server streaming "100 Continue" responses forever must not
1327+
# hang getresponse().
1328+
body = (
1329+
'HTTP/1.1 100 Continue\r\n\r\n'
1330+
* (client._MAXINTERIMRESPONSES + 1)
1331+
)
1332+
resp = client.HTTPResponse(FakeSocket(body))
1333+
with self.assertRaises(client.HTTPException) as cm:
1334+
resp.begin()
1335+
self.assertIn('got more than ', str(cm.exception))
1336+
self.assertIn('interim responses', str(cm.exception))
1337+
1338+
def test_multiple_interim_responses(self):
1339+
# A reasonable number of interim responses before the final
1340+
# response is skipped as before.
1341+
body = (
1342+
'HTTP/1.1 100 Continue\r\n\r\n' * 3 +
1343+
'HTTP/1.1 200 OK\r\n'
1344+
'Content-Length: 5\r\n'
1345+
'\r\n'
1346+
'hello'
1347+
)
1348+
resp = client.HTTPResponse(FakeSocket(body), method="GET")
1349+
resp.begin()
1350+
self.assertEqual(resp.status, 200)
1351+
self.assertEqual(resp.read(), b'hello')
1352+
resp.close()
1353+
13251354
def test_overflowing_chunked_line(self):
13261355
body = (
13271356
'HTTP/1.1 200 OK\r\n'
@@ -1393,6 +1422,35 @@ def test_chunked_trailers(self):
13931422
self.assertEqual(sock.file.read(), b"") #we read to the end
13941423
resp.close()
13951424

1425+
def test_chunked_too_many_trailers(self):
1426+
"""A response streaming endless trailer lines must raise, not hang"""
1427+
too_many_trailers = "".join(
1428+
f"X-Trailer{i}: {i}\r\n" for i in range(client._MAXHEADERS + 1)
1429+
)
1430+
# An unbounded read() reaches the trailers via the final 0 chunk.
1431+
sock = FakeSocket(
1432+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1433+
resp = client.HTTPResponse(sock, method="GET")
1434+
resp.begin()
1435+
with self.assertRaisesRegex(
1436+
client.HTTPException,
1437+
f"got more than {client._MAXHEADERS} trailers",
1438+
):
1439+
resp.read()
1440+
resp.close()
1441+
1442+
# A bounded read(amt) larger than the body hits the same limit.
1443+
sock = FakeSocket(
1444+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1445+
resp = client.HTTPResponse(sock, method="GET")
1446+
resp.begin()
1447+
with self.assertRaisesRegex(
1448+
client.HTTPException,
1449+
f"got more than {client._MAXHEADERS} trailers",
1450+
):
1451+
resp.read(len(chunked_expected) + 1)
1452+
resp.close()
1453+
13961454
def test_chunked_sync(self):
13971455
"""Check that we don't read past the end of the chunked-encoding stream"""
13981456
expected = chunked_expected
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
:mod:`http.client` now limits the number of chunked-response trailer lines
2+
it will read to 100, and the number of interim (1xx) responses it will
3+
skip to 100. A malicious or broken server could previously stream trailer
4+
lines or ``100 Continue`` responses forever, hanging the client even when
5+
a socket timeout was in use.
6+
Reported by ``@YLChen-007`` via GHSA-w4q2-g22w-6fr4.

0 commit comments

Comments
 (0)