Skip to content

Commit

Permalink
Escape filenames and paths in HTML when generating index pages (#8317) (
Browse files Browse the repository at this point in the history 
#8319)

Co-authored-by: J. Nick Koston <nick@koston.org>
(cherry picked from commit ffbc432)
  • Loading branch information
@Dreamsorcerer
Dreamsorcerer authored Apr 11, 2024
1 parent ed43040 commit 2833552
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGES/8317.bugfix.rst
View file
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
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
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

0 comments on commit 2833552

Please # to comment.