From ca0218ea87242c6031887d138183a9b05c256514 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 Nov 2024 08:50:36 -0600 Subject: [PATCH] [PR #9851/541d86d backport][3.10] Fix incorrect parsing of chunk extensions with the pure Python parser (#9853) CVE: CVE-2024-52304 Upstream-Status: Backport [https://github.com/aio-libs/aiohttp/commit/259edc369075de63e6f3a4eaade058c62af0df71] Signed-off-by: Ankur Tyagi --- aiohttp/http_parser.py | 7 ++++ tests/test_http_parser.py | 74 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/aiohttp/http_parser.py b/aiohttp/http_parser.py index 013511917..7a552458e 100644 --- a/aiohttp/http_parser.py +++ b/aiohttp/http_parser.py @@ -848,6 +848,13 @@ class HttpPayloadParser: i = chunk.find(CHUNK_EXT, 0, pos) if i >= 0: size_b = chunk[:i] # strip chunk-extensions + # Verify no LF in the chunk-extension + if b"\n" in (ext := chunk[i:pos]): + exc = BadHttpMessage( + f"Unexpected LF in chunk-extension: {ext!r}" + ) + set_exception(self.payload, exc) + raise exc else: size_b = chunk[:pos] diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index ee7dc4aab..2f34f0bc0 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -13,6 +13,7 @@ from yarl import URL import aiohttp from aiohttp import http_exceptions, streams +from aiohttp.base_protocol import BaseProtocol from aiohttp.http_parser import ( NO_EXTENSIONS, DeflateBuffer, @@ -1369,7 +1370,78 @@ def test_parse_chunked_payload_empty_body_than_another_chunked( assert b"second" == b"".join(d for d in payload._buffer) -def test_partial_url(parser: Any) -> None: +async def test_parse_chunked_payload_split_chunks(response: Any) -> None: + network_chunks = ( + b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n", + b"5\r\nfi", + b"rst", + # This simulates a bug in lax mode caused when the \r\n separator, before the + # next HTTP chunk, appears at the start of the next network chunk. + b"\r\n", + b"6", + b"\r", + b"\n", + b"second\r", + b"\n0\r\n\r\n", + ) + reader = response.feed_data(network_chunks[0])[0][0][1] + for c in network_chunks[1:]: + response.feed_data(c) + + assert response.feed_eof() is None + assert reader.is_eof() + assert await reader.read() == b"firstsecond" + + +@pytest.mark.skipif(NO_EXTENSIONS, reason="Only tests C parser.") +async def test_parse_chunked_payload_with_lf_in_extensions_c_parser( + loop: asyncio.AbstractEventLoop, protocol: BaseProtocol +) -> None: + """Test the C-parser with a chunked payload that has a LF in the chunk extensions.""" + # The C parser will raise a BadHttpMessage from feed_data + parser = HttpRequestParserC( + protocol, + loop, + 2**16, + max_line_size=8190, + max_field_size=8190, + ) + payload = ( + b"GET / HTTP/1.1\r\nHost: localhost:5001\r\n" + b"Transfer-Encoding: chunked\r\n\r\n2;\nxx\r\n4c\r\n0\r\n\r\n" + b"GET /admin HTTP/1.1\r\nHost: localhost:5001\r\n" + b"Transfer-Encoding: chunked\r\n\r\n0\r\n\r\n" + ) + with pytest.raises(http_exceptions.BadHttpMessage, match="\\\\nxx"): + parser.feed_data(payload) + + +async def test_parse_chunked_payload_with_lf_in_extensions_py_parser( + loop: asyncio.AbstractEventLoop, protocol: BaseProtocol +) -> None: + """Test the py-parser with a chunked payload that has a LF in the chunk extensions.""" + # The py parser will not raise the BadHttpMessage directly, but instead + # it will set the exception on the StreamReader. + parser = HttpRequestParserPy( + protocol, + loop, + 2**16, + max_line_size=8190, + max_field_size=8190, + ) + payload = ( + b"GET / HTTP/1.1\r\nHost: localhost:5001\r\n" + b"Transfer-Encoding: chunked\r\n\r\n2;\nxx\r\n4c\r\n0\r\n\r\n" + b"GET /admin HTTP/1.1\r\nHost: localhost:5001\r\n" + b"Transfer-Encoding: chunked\r\n\r\n0\r\n\r\n" + ) + messages, _, _ = parser.feed_data(payload) + reader = messages[0][1] + assert isinstance(reader.exception(), http_exceptions.BadHttpMessage) + assert "\\nxx" in str(reader.exception()) + + +def test_partial_url(parser: HttpRequestParser) -> None: messages, upgrade, tail = parser.feed_data(b"GET /te") assert len(messages) == 0 messages, upgrade, tail = parser.feed_data(b"st HTTP/1.1\r\n\r\n")