Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Escape filenames and paths in HTML when generating index pages (#8317) #8319

Merged
merged 1 commit into from
Apr 11, 2024
Merged

Escape filenames and paths in HTML when generating index pages (#8317) #8319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/8317.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Escaped filenames in static view -- by :user:`bdraco`.
12 changes: 7 additions & 5 deletions aiohttp/web_urldispatcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import abc
import asyncio
import base64
import functools
import hashlib
import html
import inspect
import keyword
import os
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -708,15 +712,15 @@ 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 = []
dir_index = filepath.iterdir()
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():
Expand All @@ -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>"
Expand Down
124 changes: 110 additions & 14 deletions tests/test_web_urldispatcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(
Expand All @@ -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.
Expand All @@ -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:
Expand Down
Loading