packages/python-aiohttp
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
python-aiohttp/CVE-2024-27306.patch

243 lines
9.0 KiB
Diff
Raw Normal View History

Fix CVE-2024-27306 (cherry picked from commit 5be406e20d7c33018a8b9a3ed31e95fc26d8fae4)
2024-04-22 11:38:08 +08:00
From 28335525d1eac015a7e7584137678cbb6ff19397 Mon Sep 17 00:00:00 2001
From: Sam Bull <git@sambull.org>
Date: Thu, 11 Apr 2024 15:54:45 +0100
Subject: [PATCH] Escape filenames and paths in HTML when generating index
pages (#8317) (#8319)
Co-authored-by: J. Nick Koston <nick@koston.org>
(cherry picked from commit ffbc43233209df302863712b511a11bdb6001b0f)
---
CHANGES/8317.bugfix.rst | 1 +
aiohttp/web_urldispatcher.py | 12 ++--
tests/test_web_urldispatcher.py | 124 ++++++++++++++++++++++++++++----
3 files changed, 118 insertions(+), 19 deletions(-)
create mode 100644 CHANGES/8317.bugfix.rst
diff --git a/CHANGES/8317.bugfix.rst b/CHANGES/8317.bugfix.rst
new file mode 100644
index 0000000000..b24ef2aeb8
--- /dev/null
+++ b/CHANGES/8317.bugfix.rst
@@ -0,0 +1 @@
+Escaped filenames in static view -- by :user:`bdraco`.
diff --git a/aiohttp/web_urldispatcher.py b/aiohttp/web_urldispatcher.py
index 9969653344..954291f644 100644
--- a/aiohttp/web_urldispatcher.py
+++ b/aiohttp/web_urldispatcher.py
@@ -1,7 +1,9 @@
import abc
import asyncio
import base64
+import functools
import hashlib
+import html
import inspect
import keyword
import os
@@ -90,6 +92,8 @@
_ExpectHandler = Callable[[Request], Awaitable[Optional[StreamResponse]]]
_Resolve = Tuple[Optional["UrlMappingMatchInfo"], Set[str]]
+html_escape = functools.partial(html.escape, quote=True)
+
class _InfoDict(TypedDict, total=False):
path: str
@@ -708,7 +712,7 @@ def _directory_as_html(self, filepath: Path) -> str:
assert filepath.is_dir()
relative_path_to_dir = filepath.relative_to(self._directory).as_posix()
- index_of = f"Index of /{relative_path_to_dir}"
+ index_of = f"Index of /{html_escape(relative_path_to_dir)}"
h1 = f"<h1>{index_of}</h1>"
index_list = []
@@ -716,7 +720,7 @@ def _directory_as_html(self, filepath: Path) -> str:
for _file in sorted(dir_index):
# show file url as relative to static path
rel_path = _file.relative_to(self._directory).as_posix()
- file_url = self._prefix + "/" + rel_path
+ quoted_file_url = _quote_path(f"{self._prefix}/{rel_path}")
# if file is a directory, add '/' to the end of the name
if _file.is_dir():
@@ -725,9 +729,7 @@ def _directory_as_html(self, filepath: Path) -> str:
file_name = _file.name
index_list.append(
- '<li><a href="{url}">{name}</a></li>'.format(
- url=file_url, name=file_name
- )
+ f'<li><a href="{quoted_file_url}">{html_escape(file_name)}</a></li>'
)
ul = "<ul>\n{}\n</ul>".format("\n".join(index_list))
body = f"<body>\n{h1}\n{ul}\n</body>"
diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py
index 76e533e473..0441890c10 100644
--- a/tests/test_web_urldispatcher.py
+++ b/tests/test_web_urldispatcher.py
@@ -1,6 +1,7 @@
import asyncio
import functools
import pathlib
+import sys
from typing import Optional
from unittest import mock
from unittest.mock import MagicMock
@@ -14,31 +15,38 @@
@pytest.mark.parametrize(
- "show_index,status,prefix,data",
+ "show_index,status,prefix,request_path,data",
[
- pytest.param(False, 403, "/", None, id="index_forbidden"),
+ pytest.param(False, 403, "/", "/", None, id="index_forbidden"),
pytest.param(
True,
200,
"/",
- b"<html>\n<head>\n<title>Index of /.</title>\n"
- b"</head>\n<body>\n<h1>Index of /.</h1>\n<ul>\n"
- b'<li><a href="/my_dir">my_dir/</a></li>\n'
- b'<li><a href="/my_file">my_file</a></li>\n'
- b"</ul>\n</body>\n</html>",
- id="index_root",
+ "/",
+ b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
+ b' /.</h1>\n<ul>\n<li><a href="/my_dir">my_dir/</a></li>\n<li><a href="/my_file">'
+ b"my_file</a></li>\n</ul>\n</body>\n</html>",
),
pytest.param(
True,
200,
"/static",
- b"<html>\n<head>\n<title>Index of /.</title>\n"
- b"</head>\n<body>\n<h1>Index of /.</h1>\n<ul>\n"
- b'<li><a href="/static/my_dir">my_dir/</a></li>\n'
- b'<li><a href="/static/my_file">my_file</a></li>\n'
- b"</ul>\n</body>\n</html>",
+ "/static",
+ b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
+ b' /.</h1>\n<ul>\n<li><a href="/static/my_dir">my_dir/</a></li>\n<li><a href="'
+ b'/static/my_file">my_file</a></li>\n</ul>\n</body>\n</html>',
id="index_static",
),
+ pytest.param(
+ True,
+ 200,
+ "/static",
+ "/static/my_dir",
+ b"<html>\n<head>\n<title>Index of /my_dir</title>\n</head>\n<body>\n<h1>"
+ b'Index of /my_dir</h1>\n<ul>\n<li><a href="/static/my_dir/my_file_in_dir">'
+ b"my_file_in_dir</a></li>\n</ul>\n</body>\n</html>",
+ id="index_subdir",
+ ),
],
)
async def test_access_root_of_static_handler(
@@ -47,6 +55,7 @@ async def test_access_root_of_static_handler(
show_index: bool,
status: int,
prefix: str,
+ request_path: str,
data: Optional[bytes],
) -> None:
# Tests the operation of static file server.
@@ -72,7 +81,94 @@ async def test_access_root_of_static_handler(
client = await aiohttp_client(app)
# Request the root of the static directory.
- async with await client.get(prefix) as r:
+ async with await client.get(request_path) as r:
+ assert r.status == status
+
+ if data:
+ assert r.headers["Content-Type"] == "text/html; charset=utf-8"
+ read_ = await r.read()
+ assert read_ == data
+
+
+@pytest.mark.internal # Dependent on filesystem
+@pytest.mark.skipif(
+ not sys.platform.startswith("linux"),
+ reason="Invalid filenames on some filesystems (like Windows)",
+)
+@pytest.mark.parametrize(
+ "show_index,status,prefix,request_path,data",
+ [
+ pytest.param(False, 403, "/", "/", None, id="index_forbidden"),
+ pytest.param(
+ True,
+ 200,
+ "/",
+ "/",
+ b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
+ b' /.</h1>\n<ul>\n<li><a href="/%3Cimg%20src=0%20onerror=alert(1)%3E.dir">&l'
+ b't;img src=0 onerror=alert(1)&gt;.dir/</a></li>\n<li><a href="/%3Cimg%20sr'
+ b'c=0%20onerror=alert(1)%3E.txt">&lt;img src=0 onerror=alert(1)&gt;.txt</a></l'
+ b"i>\n</ul>\n</body>\n</html>",
+ ),
+ pytest.param(
+ True,
+ 200,
+ "/static",
+ "/static",
+ b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
+ b' /.</h1>\n<ul>\n<li><a href="/static/%3Cimg%20src=0%20onerror=alert(1)%3E.'
+ b'dir">&lt;img src=0 onerror=alert(1)&gt;.dir/</a></li>\n<li><a href="/stat'
+ b'ic/%3Cimg%20src=0%20onerror=alert(1)%3E.txt">&lt;img src=0 onerror=alert(1)&'
+ b"gt;.txt</a></li>\n</ul>\n</body>\n</html>",
+ id="index_static",
+ ),
+ pytest.param(
+ True,
+ 200,
+ "/static",
+ "/static/<img src=0 onerror=alert(1)>.dir",
+ b"<html>\n<head>\n<title>Index of /&lt;img src=0 onerror=alert(1)&gt;.dir</t"
+ b"itle>\n</head>\n<body>\n<h1>Index of /&lt;img src=0 onerror=alert(1)&gt;.di"
+ b'r</h1>\n<ul>\n<li><a href="/static/%3Cimg%20src=0%20onerror=alert(1)%3E.di'
+ b'r/my_file_in_dir">my_file_in_dir</a></li>\n</ul>\n</body>\n</html>',
+ id="index_subdir",
+ ),
+ ],
+)
+async def test_access_root_of_static_handler_xss(
+ tmp_path: pathlib.Path,
+ aiohttp_client: AiohttpClient,
+ show_index: bool,
+ status: int,
+ prefix: str,
+ request_path: str,
+ data: Optional[bytes],
+) -> None:
+ # Tests the operation of static file server.
+ # Try to access the root of static file server, and make
+ # sure that correct HTTP statuses are returned depending if we directory
+ # index should be shown or not.
+ # Ensure that html in file names is escaped.
+ # Ensure that links are url quoted.
+ my_file = tmp_path / "<img src=0 onerror=alert(1)>.txt"
+ my_dir = tmp_path / "<img src=0 onerror=alert(1)>.dir"
+ my_dir.mkdir()
+ my_file_in_dir = my_dir / "my_file_in_dir"
+
+ with my_file.open("w") as fw:
+ fw.write("hello")
+
+ with my_file_in_dir.open("w") as fw:
+ fw.write("world")
+
+ app = web.Application()
+
+ # Register global static route:
+ app.router.add_static(prefix, str(tmp_path), show_index=show_index)
+ client = await aiohttp_client(app)
+
+ # Request the root of the static directory.
+ async with await client.get(request_path) as r:
assert r.status == status
if data:
Reference in New Issue Copy Permalink