From 8d718d1fb8ee7a923c0e42cf100908ecead4f564 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 | 17 +++++++++-------- tests/test_urldispatch.py | 18 +++++++++++++++++- tests/test_web_sendfile_functional.py | 2 +- tests/test_web_urldispatcher.py | 4 ++-- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py index 954291f..864f8dd 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 warnings from contextlib import contextmanager @@ -88,6 +89,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]] @@ -647,7 +649,12 @@ class StaticResource(PrefixResource): path = request.rel_url.raw_path method = request.method allowed_methods = set(self._routes) - 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() if method not in allowed_methods: @@ -663,14 +670,8 @@ class StaticResource(PrefixResource): return iter(self._routes.values()) async def _handle(self, request: Request) -> StreamResponse: - rel_url = request.match_info["filename"] try: - 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) if self._follow_symlinks: normalized_path = Path(os.path.normpath(unresolved_path)) diff --git a/tests/test_urldispatch.py b/tests/test_urldispatch.py index 4f3abb8..cec4cd0 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 urllib.parse import unquote @@ -967,7 +968,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 57ac084..aa53726 100644 --- a/tests/test_web_sendfile_functional.py +++ b/tests/test_web_sendfile_functional.py @@ -565,7 +565,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 0441890..4164677 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -701,8 +701,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(