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

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

Merged
merged 10 commits into from
Apr 11, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9562328
Escape filenames and paths in HTML when generating index pages
bdraco Feb 4, 2024
888ee75
Merge branch 'master' into escape-filenames
Dreamsorcerer Apr 8, 2024
584e0f3
Update test_web_urldispatcher.py
Dreamsorcerer Apr 11, 2024
95bc95b
Update tests/test_web_urldispatcher.py
Dreamsorcerer Apr 11, 2024
538a7fe
Update test_web_urldispatcher.py
Dreamsorcerer Apr 11, 2024
eb7c7fc
Update test_web_urldispatcher.py
Dreamsorcerer Apr 11, 2024
96e43d2
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 11, 2024
c4892ef
Create 8317.bugfix.rst
Dreamsorcerer Apr 11, 2024
20be297
Update test_web_urldispatcher.py
Dreamsorcerer Apr 11, 2024
e3935e0
Update test_web_urldispatcher.py
Dreamsorcerer Apr 11, 2024
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 keyword
import os
import re
Expand Down Expand Up @@ -87,6 +89,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 @@ -686,15 +690,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 @@ -703,9 +707,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 @@ -71,7 +80,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