diff --git a/CHANGES/13175.bugfix.rst b/CHANGES/13175.bugfix.rst new file mode 100644 index 00000000000..ce39fa8f3cd --- /dev/null +++ b/CHANGES/13175.bugfix.rst @@ -0,0 +1,4 @@ +Fixed :attr:`~aiohttp.web.BaseRequest.raw_path` including the scheme and host +for absolute-form request targets, and made the pure-Python parser reject +authority-form targets (``host:port``) for methods other than ``CONNECT`` +-- by :user:`Dreamsorcerer`. diff --git a/aiohttp/http_parser.py b/aiohttp/http_parser.py index 9d0669cf8b9..dba6219bad2 100644 --- a/aiohttp/http_parser.py +++ b/aiohttp/http_parser.py @@ -696,8 +696,9 @@ def parse_message(self, lines: list[bytes]) -> RawRequestMessage: # absolute-form for proxy maybe, # https://datatracker.ietf.org/doc/html/rfc7230#section-5.3.2 url = URL(path, encoded=True) - if url.scheme == "": - # not absolute-form + if not url.absolute: + # authority-form is only allowed with CONNECT + # https://www.rfc-editor.org/info/rfc9112/#section-3.2.3-1 raise InvalidURLError( path.encode(errors="surrogateescape").decode("latin1") ) diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index 5d152dedcd2..bafbb5fdb86 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -465,7 +465,26 @@ def raw_path(self) -> str: E.g., ``/my%2Fpath%7Cwith%21some%25strange%24characters`` """ - return self._message.path + path = self._message.path + + # An absolute-form target carries a "scheme://authority" that must not + # leak into the path. Strip it, keeping the remainder byte-for-byte, + # exactly as an origin-form target. Authority-form is used only by + # CONNECT and is left unchanged. + # https://www.rfc-editor.org/info/rfc9112/#section-3.2.2-9 + # https://www.rfc-editor.org/info/rfc9112/#name-authority-form + if self._message.url.absolute and self._method != "CONNECT": + # absolute-form always contains "://" (guaranteed by the parser). + scheme_sep = path.find("://") + assert scheme_sep != -1 + cursor = scheme_sep + 3 + rel = len(path) + for delimiter in "/?#": + found = path.find(delimiter, cursor) + if found != -1: + rel = min(rel, found) + return path[rel:] + return path @reify def query(self) -> MultiDictProxy[str]: diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index 6d700d7dae5..15d3338bbc6 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -1133,6 +1133,12 @@ def test_url_absolute(parser: HttpRequestParser) -> None: assert msg.url == URL("https://www.google.com/path/to.html") +def test_url_authority_form_only_connect(parser: HttpRequestParser) -> None: + # https://www.rfc-editor.org/info/rfc9112/#section-3.2.3-1 + with pytest.raises(http_exceptions.InvalidURLError): + parser.feed_data(b"GET www.google.com:443 HTTP/1.1\r\nHost: a\r\n\r\n") + + def test_headers_old_websocket_key1(parser: HttpRequestParser) -> None: text = b"GET /test HTTP/1.1\r\nHost: a\r\nSEC-WEBSOCKET-KEY1: line\r\n\r\n" diff --git a/tests/test_web_middleware.py b/tests/test_web_middleware.py index 6e881d2488a..cc3a526a9ca 100644 --- a/tests/test_web_middleware.py +++ b/tests/test_web_middleware.py @@ -1,5 +1,6 @@ import asyncio from collections.abc import Awaitable, Callable, Iterable +from contextlib import suppress from typing import NoReturn import pytest @@ -415,6 +416,32 @@ async def handle(request: web.Request) -> web.StreamResponse: assert resp.headers["Location"] == "/google.com" assert resp.url.query == URL("//google.com").query + async def test_open_redirect_absolute_form_target( + self, aiohttp_server: AiohttpServer + ) -> None: + async def handle(request: web.Request) -> web.Response: + assert False + + app = web.Application(middlewares=[web.normalize_path_middleware()]) + app.add_routes([web.get("/google.com/", handle)]) + server = await aiohttp_server(app) + + reader, writer = await asyncio.open_connection(server.host, server.port) + try: + writer.write( + b"GET http://google.com/google.com HTTP/1.1\r\n" + b"Host: localhost\r\nConnection: close\r\n\r\n" + ) + await writer.drain() + head = (await reader.readuntil(b"\r\n\r\n")).decode("ascii") + finally: + writer.close() + with suppress(ConnectionResetError, BrokenPipeError): + await writer.wait_closed() + + assert head.startswith("HTTP/1.1 308 ") + assert "\r\nLocation: /google.com/\r\n" in head + async def test_normalize_path_skips_parser_error( aiohttp_server: AiohttpServer, diff --git a/tests/test_web_request.py b/tests/test_web_request.py index b518d2a513f..d420495b39c 100644 --- a/tests/test_web_request.py +++ b/tests/test_web_request.py @@ -220,6 +220,41 @@ def test_absolute_url() -> None: assert req.rel_url == URL.build(path="/path/to", query={"a": "1"}) +def test_absolute_form_raw_path() -> None: + # An absolute-form target (RFC 9112 3.2.2) must not leak the scheme/host + # into raw_path. The path, query and fragment are kept byte-for-byte, the + # same raw form an origin-form target yields. + req = make_mocked_request("GET", "https://example.com/path/to?a=1#frag") + assert req.raw_path == "/path/to?a=1#frag" + assert req.raw_path == make_mocked_request("GET", "/path/to?a=1#frag").raw_path + + +def test_connect_authority_form_raw_path() -> None: + # Authority-form is only used by CONNECT (RFC 9112 3.2.3); its target is a + # bare host:port with no scheme prefix, so raw_path returns it unchanged. + message = RawRequestMessage( + "CONNECT", + "example.com:443", + HttpVersion(1, 1), + HeadersDictProxy(CIMultiDict()), + (), + False, + None, + False, + False, + URL.build(authority="example.com:443", encoded=True), + ) + protocol = mock.Mock() + protocol.ssl_context = None + protocol.peername = None + protocol.sockname = ("127.0.0.1", 80) + req = web.BaseRequest( + message, mock.Mock(), protocol, mock.Mock(), mock.Mock(), mock.Mock() + ) + assert req._message.url.absolute + assert req.raw_path == "example.com:443" + + def test_clone_absolute_scheme() -> None: req = make_mocked_request("GET", "https://example.com/path/to?a=1") assert req.scheme == "https"