From b330573c65db35ee1228615ec257619b49b918c7 Mon Sep 17 00:00:00 2001 From: Gyorgy Sarvari Date: Sat, 3 Jan 2026 01:55:05 +0000 Subject: [PATCH] Reject static URLs that traverse outside static root (#11888) (#11906) From: Sam Bull (cherry picked from commit 63961fa77fa2443109f25c3d8ab94772d3878626) Co-authored-by: J. Nick Koston CVE: CVE-2025-69226 Upstream-Status: Backport [https://github.com/aio-libs/aiohttp/commit/f2a86fd5ac0383000d1715afddfa704413f0711e] Signed-off-by: Gyorgy Sarvari --- aiohttp/web_urldispatcher.py | 18 +++++++++--------- tests/test_urldispatch.py | 18 +++++++++++++++++- tests/test_web_sendfile_functional.py | 2 +- tests/test_web_urldispatcher.py | 4 ++-- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 28ae251..a121656 100644 --- a/aiohttp/web_urldispatcher.py +++ b/aiohttp/web_urldispatcher.py @@ -7,6 +7,7 @@ import html import inspect import keyword import os +import platform import re import sys import warnings @@ -94,6 +95,7 @@ ROUTE_RE: Final[Pattern[str]] = re.compile( ) PATH_SEP: Final[str] = re.escape("/") +IS_WINDOWS: Final[bool] = platform.system() == "Windows" _ExpectHandler = Callable[[Request], Awaitable[Optional[StreamResponse]]] _Resolve = Tuple[Optional["UrlMappingMatchInfo"], Set[str]] @@ -649,7 +651,12 @@ class StaticResource(PrefixResource): async def resolve(self, request: Request) -> _Resolve: path = request.rel_url.path_safe method = request.method - if not path.startswith(self._prefix2) and path != self._prefix: + # We normalise here to avoid matches that traverse below the static root. + # e.g. /static/../../../../home/user/webapp/static/ + norm_path = os.path.normpath(path) + if IS_WINDOWS: + norm_path = norm_path.replace("\\", "/") + if not norm_path.startswith(self._prefix2) and norm_path != self._prefix: return None, set() allowed_methods = self._allowed_methods @@ -666,14 +673,7 @@ class StaticResource(PrefixResource): return iter(self._routes.values()) async def _handle(self, request: Request) -> StreamResponse: - rel_url = request.match_info["filename"] - filename = Path(rel_url) - if filename.anchor: - # rel_url is an absolute name like - # /static/\\machine_name\c$ or /static/D:\path - # where the static dir is totally different - raise HTTPForbidden() - + filename = request.match_info["filename"] unresolved_path = self._directory.joinpath(filename) loop = asyncio.get_running_loop() return await loop.run_in_executor( diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index ba6bdff..e329ea2 100644 --- a/tests/test_urldispatch.py +++ b/tests/test_urldispatch.py @@ -1,4 +1,5 @@ import pathlib +import platform import re from collections.abc import Container, Iterable, Mapping, MutableMapping, Sized from typing import NoReturn @@ -1041,7 +1042,22 @@ async def test_405_for_resource_adapter(router) -> None: assert (None, {"HEAD", "GET"}) == ret -async def test_check_allowed_method_for_found_resource(router) -> None: +@pytest.mark.skipif(platform.system() == "Windows", reason="Different path formats") +async def test_static_resource_outside_traversal(router: web.UrlDispatcher) -> None: + """Test relative path traversing outside root does not resolve.""" + static_file = pathlib.Path(aiohttp.__file__) + request_path = "/st" + "/.." * (len(static_file.parts) - 2) + str(static_file) + assert pathlib.Path(request_path).resolve() == static_file + + resource = router.add_static("/st", static_file.parent) + ret = await resource.resolve(make_mocked_request("GET", request_path)) + # Should not resolve, otherwise filesystem information may be leaked. + assert (None, set()) == ret + + +async def test_check_allowed_method_for_found_resource( + router: web.UrlDispatcher, +) -> None: handler = make_handler() resource = router.add_resource("/") resource.add_route("GET", handler) diff --git a/tests/test_web_sendfile_functional.py b/tests/test_web_sendfile_functional.py index 0325a46..3207623 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -638,7 +638,7 @@ async def test_static_file_directory_traversal_attack(aiohttp_client) -> None: url_abspath = "/static/" + str(full_path.resolve()) resp = await client.get(url_abspath) - assert 403 == resp.status + assert resp.status == 404 await resp.release() await client.close() diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index ee60b69..7de3ea5 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -838,8 +838,8 @@ async def test_static_absolute_url( here = pathlib.Path(__file__).parent app.router.add_static("/static", here) client = await aiohttp_client(app) - resp = await client.get("/static/" + str(file_path.resolve())) - assert resp.status == 403 + async with client.get("/static/" + str(file_path.resolve())) as resp: + assert resp.status == 404 async def test_for_issue_5250(