mirror of
https://github.com/python/cpython.git
synced 2026-06-29 04:10:54 +00:00
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. --- 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.
This commit is contained in:
parent
54524ab669
commit
41cc78a7a4
4 changed files with 123 additions and 1 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue