Skip to content

Commit 84badb7

Browse files
authored
[3.14] gh-150743: Limit trailer lines and interim responses read by http.client (GH-150749)
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. --- This issue was reported to us via [GHSA-w4q2-g22w-6fr4](https://github.com/python/cpython/security/advisories/GHSA-w4q2-g22w-6fr4)
1 parent 7d6fc2b commit 84badb7

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
@@ -1332,6 +1332,35 @@ def test_overflowing_header_limit_after_100(self):
13321332
self.assertIn('got more than ', str(cm.exception))
13331333
self.assertIn('headers', str(cm.exception))
13341334

1335+
def test_too_many_interim_responses(self):
1336+
# A server streaming "100 Continue" responses forever must not
1337+
# hang getresponse().
1338+
body = (
1339+
'HTTP/1.1 100 Continue\r\n\r\n'
1340+
* (client._MAXINTERIMRESPONSES + 1)
1341+
)
1342+
resp = client.HTTPResponse(FakeSocket(body))
1343+
with self.assertRaises(client.HTTPException) as cm:
1344+
resp.begin()
1345+
self.assertIn('got more than ', str(cm.exception))
1346+
self.assertIn('interim responses', str(cm.exception))
1347+
1348+
def test_multiple_interim_responses(self):
1349+
# A reasonable number of interim responses before the final
1350+
# response is skipped as before.
1351+
body = (
1352+
'HTTP/1.1 100 Continue\r\n\r\n' * 3 +
1353+
'HTTP/1.1 200 OK\r\n'
1354+
'Content-Length: 5\r\n'
1355+
'\r\n'
1356+
'hello'
1357+
)
1358+
resp = client.HTTPResponse(FakeSocket(body), method="GET")
1359+
resp.begin()
1360+
self.assertEqual(resp.status, 200)
1361+
self.assertEqual(resp.read(), b'hello')
1362+
resp.close()
1363+
13351364
def test_overflowing_chunked_line(self):
13361365
body = (
13371366
'HTTP/1.1 200 OK\r\n'
@@ -1403,6 +1432,35 @@ def test_chunked_trailers(self):
14031432
self.assertEqual(sock.file.read(), b"") #we read to the end
14041433
resp.close()
14051434

1435+
def test_chunked_too_many_trailers(self):
1436+
"""A response streaming endless trailer lines must raise, not hang"""
1437+
too_many_trailers = "".join(
1438+
f"X-Trailer{i}: {i}\r\n" for i in range(client._MAXHEADERS + 1)
1439+
)
1440+
# An unbounded read() reaches the trailers via the final 0 chunk.
1441+
sock = FakeSocket(
1442+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1443+
resp = client.HTTPResponse(sock, method="GET")
1444+
resp.begin()
1445+
with self.assertRaisesRegex(
1446+
client.HTTPException,
1447+
f"got more than {client._MAXHEADERS} trailers",
1448+
):
1449+
resp.read()
1450+
resp.close()
1451+
1452+
# A bounded read(amt) larger than the body hits the same limit.
1453+
sock = FakeSocket(
1454+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1455+
resp = client.HTTPResponse(sock, method="GET")
1456+
resp.begin()
1457+
with self.assertRaisesRegex(
1458+
client.HTTPException,
1459+
f"got more than {client._MAXHEADERS} trailers",
1460+
):
1461+
resp.read(len(chunked_expected) + 1)
1462+
resp.close()
1463+
14061464
def test_chunked_sync(self):
14071465
"""Check that we don't read past the end of the chunked-encoding stream"""
14081466
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)