diff --git a/docs/content/2026-news.md b/docs/content/2026-news.md index dec7246d3..2d892c289 100644 --- a/docs/content/2026-news.md +++ b/docs/content/2026-news.md @@ -5,6 +5,13 @@ ### Bug Fixes +- **Chunked framing lines were read without a bound**: a chunk-size line or + trailer section that never terminates was reread in full on every socket read + with no size limit, so one request could keep a worker busy without ever + reaching the application. These lines are now bounded by the existing request + limits and scanned incrementally, and a malformed chunked body answers + `400 Bad Request` with one log line instead of a traceback. + - **HTTP/2 follow-up to the review**: a bad request now resets its own stream instead of the connection; the request line and field limits and the method token rule apply to HTTP/2 requests; forbidden trailers are diff --git a/gunicorn/http/body.py b/gunicorn/http/body.py index 99d75ec2d..aca64d0ad 100644 --- a/gunicorn/http/body.py +++ b/gunicorn/http/body.py @@ -6,7 +6,12 @@ import sys from gunicorn.http.errors import (NoMoreData, ChunkMissingTerminator, - InvalidChunkSize, InvalidChunkExtension) + InvalidChunkSize, InvalidChunkExtension, + LimitRequestHeaders) + +#: Fallbacks when the request carries no header limits of its own. +DEFAULT_MAX_CHUNK_SIZE_LINE = 8190 +DEFAULT_MAX_TRAILER_SECTION = 8190 * 32 class ChunkedReader: @@ -14,6 +19,15 @@ def __init__(self, req, unreader): self.req = req self.parser = self.parse_chunked(unreader) self.buf = io.BytesIO() + # A chunk-size line is a few hex digits plus optional extensions, and + # a trailer section is a header block. Both are read a socket buffer + # at a time while looking for their terminator, so both need a bound: + # without one a peer can make a worker read and scan for as long as it + # cares to send, and the request never reaches the application. + self.limit_chunk_size_line = getattr( + req, "limit_request_field_size", 0) or DEFAULT_MAX_CHUNK_SIZE_LINE + self.limit_trailer_section = getattr( + req, "max_buffer_headers", 0) or DEFAULT_MAX_TRAILER_SECTION def read(self, size): if not isinstance(size, int): @@ -38,26 +52,31 @@ def read(self, size): return ret def parse_trailers(self, unreader, data): - buf = io.BytesIO() - buf.write(data) + buf = bytearray(data) - idx = buf.getvalue().find(b"\r\n\r\n") - done = buf.getvalue()[:2] == b"\r\n" + idx = buf.find(b"\r\n\r\n") + done = buf[:2] == b"\r\n" while idx < 0 and not done: + if len(buf) > self.limit_trailer_section: + raise LimitRequestHeaders("limit request trailer section size") + # Only the last three bytes can start a terminator that the next + # read completes, so the scan resumes there instead of running + # over the whole buffer again. + searched = max(0, len(buf) - 3) try: - self.get_data(unreader, buf) + self.read_into(unreader, buf) except NoMoreData: # RFC 9112 7.1.2: the last chunk (0 CRLF) must be followed by # a CRLF-terminated trailer section. Hitting EOF before that # means the chunked body was truncated, not cleanly ended. raise ChunkMissingTerminator(b"") from None - idx = buf.getvalue().find(b"\r\n\r\n") - done = buf.getvalue()[:2] == b"\r\n" + idx = buf.find(b"\r\n\r\n", searched) + done = buf[:2] == b"\r\n" if done: - unreader.unread(buf.getvalue()[2:]) + unreader.unread(bytes(buf[2:])) return b"" - self.req.trailers = self.req.parse_headers(buf.getvalue()[:idx], from_trailer=True) - unreader.unread(buf.getvalue()[idx + 4:]) + self.req.trailers = self.req.parse_headers(bytes(buf[:idx]), from_trailer=True) + unreader.unread(bytes(buf[idx + 4:])) def parse_chunked(self, unreader): (size, rest) = self.parse_chunk_size(unreader) @@ -81,17 +100,21 @@ def parse_chunked(self, unreader): (size, rest) = self.parse_chunk_size(unreader, data=rest[2:]) def parse_chunk_size(self, unreader, data=None): - buf = io.BytesIO() - if data is not None: - buf.write(data) + buf = bytearray(data) if data else bytearray() - idx = buf.getvalue().find(b"\r\n") + idx = buf.find(b"\r\n") while idx < 0: - self.get_data(unreader, buf) - idx = buf.getvalue().find(b"\r\n") - - data = buf.getvalue() - line, rest_chunk = data[:idx], data[idx + 2:] + if len(buf) > self.limit_chunk_size_line: + raise InvalidChunkSize( + b"line over %d bytes with no terminator" % self.limit_chunk_size_line) + # Only the last byte can start a terminator that the next read + # completes, so the scan resumes there instead of running over + # the whole buffer again. + searched = max(0, len(buf) - 1) + self.read_into(unreader, buf) + idx = buf.find(b"\r\n", searched) + + line, rest_chunk = bytes(buf[:idx]), bytes(buf[idx + 2:]) # RFC9112 7.1.1: BWS before chunk-ext - but ONLY then chunk_size, *chunk_ext = line.split(b";", 1) @@ -117,6 +140,13 @@ def get_data(self, unreader, buf): raise NoMoreData() buf.write(data) + def read_into(self, unreader, buf): + """Append one socket read to a bytearray, without copying it.""" + data = unreader.read() + if not data: + raise NoMoreData() + buf += data + class LengthReader: def __init__(self, unreader, length): diff --git a/gunicorn/http/errors.py b/gunicorn/http/errors.py index a3294128f..403a76b6f 100644 --- a/gunicorn/http/errors.py +++ b/gunicorn/http/errors.py @@ -97,27 +97,30 @@ def __str__(self): return "Unsupported transfer coding: %r" % self.hdr -class InvalidChunkSize(IOError): +class InvalidChunkSize(ParseException): def __init__(self, data): self.data = data + self.code = 400 def __str__(self): return "Invalid chunk size: %r" % self.data -class ChunkMissingTerminator(IOError): +class ChunkMissingTerminator(ParseException): def __init__(self, term): self.term = term + self.code = 400 def __str__(self): return "Invalid chunk terminator is not '\\r\\n': %r" % self.term -class InvalidChunkExtension(IOError): +class InvalidChunkExtension(ParseException): """Invalid chunk extension per RFC 9112.""" def __init__(self, reason): self.reason = reason + self.code = 400 def __str__(self): return "Invalid chunk extension: %s" % self.reason diff --git a/gunicorn/workers/base.py b/gunicorn/workers/base.py index bb74f3212..79191239d 100644 --- a/gunicorn/workers/base.py +++ b/gunicorn/workers/base.py @@ -14,7 +14,8 @@ from gunicorn import util from gunicorn.http.errors import ( - ForbiddenProxyRequest, InvalidHeader, + ChunkMissingTerminator, ForbiddenProxyRequest, + InvalidChunkExtension, InvalidChunkSize, InvalidHeader, InvalidHeaderName, InvalidHTTPVersion, InvalidProxyLine, InvalidRequestLine, InvalidRequestMethod, InvalidSchemeHeaders, @@ -209,6 +210,7 @@ def handle_error(self, req, client, addr, exc): InvalidSchemeHeaders, UnsupportedTransferCoding, ConfigurationProblem, ObsoleteFolding, ExpectationFailed, InvalidH2CPreface, SSLError, + InvalidChunkSize, ChunkMissingTerminator, InvalidChunkExtension, )): status_int = 400 @@ -250,6 +252,9 @@ def handle_error(self, req, client, addr, exc): status_int = 403 elif isinstance(exc, InvalidSchemeHeaders): mesg = "%s" % str(exc) + elif isinstance(exc, (InvalidChunkSize, ChunkMissingTerminator, + InvalidChunkExtension)): + mesg = "%s" % str(exc) elif isinstance(exc, InvalidH2CPreface): mesg = "%s" % str(exc) elif isinstance(exc, SSLError): diff --git a/tests/test_http.py b/tests/test_http.py index f31e7941a..aafa400a7 100644 --- a/tests/test_http.py +++ b/tests/test_http.py @@ -8,10 +8,11 @@ from unittest import mock from gunicorn import util -from gunicorn.http.body import Body, LengthReader, EOFReader +from gunicorn.http.body import Body, ChunkedReader, LengthReader, EOFReader from gunicorn.http.wsgi import FileWrapper, Response from gunicorn.http.unreader import Unreader, IterUnreader, SocketUnreader -from gunicorn.http.errors import InvalidHeader, InvalidHeaderName, InvalidHTTPVersion +from gunicorn.http.errors import (InvalidChunkSize, InvalidHeader, InvalidHeaderName, + InvalidHTTPVersion, LimitRequestHeaders) from gunicorn.http.message import TOKEN_RE @@ -480,3 +481,61 @@ def test_normal_response_unaffected(): assert resp.response_length == 5 resp.write(b"hello") assert resp.sent == 5 + + +class _CountingUnreader(IterUnreader): + """An unreader that hands out fixed-size reads and counts them.""" + + def __init__(self, blocks): + super().__init__(iter(blocks)) + self.reads = 0 + + def read(self, size=None): + self.reads += 1 + return super().read(size) + + +def _chunked_reader(unreader, field_size=8190, headers=8190 * 32): + req = mock.Mock() + req.limit_request_field_size = field_size + req.max_buffer_headers = headers + req.parse_headers.return_value = [("X-Sum", "1")] + return req, ChunkedReader(req, unreader) + + +def test_chunk_size_line_is_bounded(): + """An endless chunk-size line is refused instead of read for ever.""" + unreader = _CountingUnreader([b"a" * 8192 for _ in range(1024)]) # 8MB, no CRLF + _, reader = _chunked_reader(unreader) + with pytest.raises(InvalidChunkSize): + reader.parse_chunk_size(unreader) + assert unreader.reads <= 3, "kept reading well past the limit" + + +def test_trailer_section_is_bounded(): + """An endless trailer section is refused instead of read for ever.""" + unreader = _CountingUnreader([b"x" * 8192 for _ in range(1024)]) + _, reader = _chunked_reader(unreader, headers=8190) + with pytest.raises(LimitRequestHeaders): + reader.parse_trailers(unreader, b"") + assert unreader.reads <= 3, "kept reading well past the limit" + + +def test_chunked_message_split_across_reads(): + """Terminators straddling two reads are found, and long lines still parse. + + Delivered a byte at a time, so every chunk-size line, chunk terminator + and the trailer section are split at every possible offset. + """ + message = (b"1;padding=" + b"x" * 200 + b"\r\n" + b"a\r\n" + b"2\r\nbc\r\n" + b"0\r\n" + b"X-Sum: 1\r\n\r\n" + b"leftover") + unreader = IterUnreader(iter([message[i:i + 1] for i in range(len(message))])) + req, reader = _chunked_reader(unreader) + assert Body(reader).read(100) == b"abc" + assert req.parse_headers.call_args[0][0] == b"X-Sum: 1" + # Whatever followed the message is left for the next request + assert b"".join(iter(unreader.read, b"")) == b"leftover"