Skip to content

Commit a51c520

Browse files
[3.12] gh-150743: Limit trailer lines and interim responses read by http.client (GH-150749) (#152527)
[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. (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 db4a157 commit a51c520

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
@@ -1319,6 +1319,35 @@ def test_overflowing_header_limit_after_100(self):
13191319
self.assertIn('got more than ', str(cm.exception))
13201320
self.assertIn('headers', str(cm.exception))
13211321

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

1422+
def test_chunked_too_many_trailers(self):
1423+
"""A response streaming endless trailer lines must raise, not hang"""
1424+
too_many_trailers = "".join(
1425+
f"X-Trailer{i}: {i}\r\n" for i in range(client._MAXHEADERS + 1)
1426+
)
1427+
# An unbounded read() reaches the trailers via the final 0 chunk.
1428+
sock = FakeSocket(
1429+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1430+
resp = client.HTTPResponse(sock, method="GET")
1431+
resp.begin()
1432+
with self.assertRaisesRegex(
1433+
client.HTTPException,
1434+
f"got more than {client._MAXHEADERS} trailers",
1435+
):
1436+
resp.read()
1437+
resp.close()
1438+
1439+
# A bounded read(amt) larger than the body hits the same limit.
1440+
sock = FakeSocket(
1441+
chunked_start + last_chunk + too_many_trailers + chunked_end)
1442+
resp = client.HTTPResponse(sock, method="GET")
1443+
resp.begin()
1444+
with self.assertRaisesRegex(
1445+
client.HTTPException,
1446+
f"got more than {client._MAXHEADERS} trailers",
1447+
):
1448+
resp.read(len(chunked_expected) + 1)
1449+
resp.close()
1450+
13931451
def test_chunked_sync(self):
13941452
"""Check that we don't read past the end of the chunked-encoding stream"""
13951453
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)