Skip to content

Commit 2833552

Browse files
Dreamsorcererbdraco
Dreamsorcerer
and
bdraco
authored
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 ffbc432)
1 parent ed43040 commit 2833552

File tree

3 files changed

+118
-19
lines changed

3 files changed

+118
-19
lines changed

‎CHANGES/8317.bugfix.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Escaped filenames in static view -- by :user:`bdraco`.

‎aiohttp/web_urldispatcher.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import abc
22
import asyncio
33
import base64
4+
import functools
45
import hashlib
6+
import html
57
import inspect
68
import keyword
79
import os
@@ -90,6 +92,8 @@
9092
_ExpectHandler = Callable[[Request], Awaitable[Optional[StreamResponse]]]
9193
_Resolve = Tuple[Optional["UrlMappingMatchInfo"], Set[str]]
9294

95+
html_escape = functools.partial(html.escape, quote=True)
96+
9397

9498
class _InfoDict(TypedDict, total=False):
9599
path: str
@@ -708,15 +712,15 @@ def _directory_as_html(self, filepath: Path) -> str:
708712
assert filepath.is_dir()
709713

710714
relative_path_to_dir = filepath.relative_to(self._directory).as_posix()
711-
index_of = f"Index of /{relative_path_to_dir}"
715+
index_of = f"Index of /{html_escape(relative_path_to_dir)}"
712716
h1 = f"<h1>{index_of}</h1>"
713717

714718
index_list = []
715719
dir_index = filepath.iterdir()
716720
for _file in sorted(dir_index):
717721
# show file url as relative to static path
718722
rel_path = _file.relative_to(self._directory).as_posix()
719-
file_url = self._prefix + "/" + rel_path
723+
quoted_file_url = _quote_path(f"{self._prefix}/{rel_path}")
720724

721725
# if file is a directory, add '/' to the end of the name
722726
if _file.is_dir():
@@ -725,9 +729,7 @@ def _directory_as_html(self, filepath: Path) -> str:
725729
file_name = _file.name
726730

727731
index_list.append(
728-
'<li><a href="{url}">{name}</a></li>'.format(
729-
url=file_url, name=file_name
730-
)
732+
f'<li><a href="{quoted_file_url}">{html_escape(file_name)}</a></li>'
731733
)
732734
ul = "<ul>\n{}\n</ul>".format("\n".join(index_list))
733735
body = f"<body>\n{h1}\n{ul}\n</body>"

‎tests/test_web_urldispatcher.py‎

Lines changed: 110 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import asyncio
22
import functools
33
import pathlib
4+
import sys
45
from typing import Optional
56
from unittest import mock
67
from unittest.mock import MagicMock
@@ -14,31 +15,38 @@
1415

1516

1617
@pytest.mark.parametrize(
17-
"show_index,status,prefix,data",
18+
"show_index,status,prefix,request_path,data",
1819
[
19-
pytest.param(False, 403, "/", None, id="index_forbidden"),
20+
pytest.param(False, 403, "/", "/", None, id="index_forbidden"),
2021
pytest.param(
2122
True,
2223
200,
2324
"/",
24-
b"<html>\n<head>\n<title>Index of /.</title>\n"
25-
b"</head>\n<body>\n<h1>Index of /.</h1>\n<ul>\n"
26-
b'<li><a href="/my_dir">my_dir/</a></li>\n'
27-
b'<li><a href="/my_file">my_file</a></li>\n'
28-
b"</ul>\n</body>\n</html>",
29-
id="index_root",
25+
"/",
26+
b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
27+
b' /.</h1>\n<ul>\n<li><a href="/my_dir">my_dir/</a></li>\n<li><a href="/my_file">'
28+
b"my_file</a></li>\n</ul>\n</body>\n</html>",
3029
),
3130
pytest.param(
3231
True,
3332
200,
3433
"/static",
35-
b"<html>\n<head>\n<title>Index of /.</title>\n"
36-
b"</head>\n<body>\n<h1>Index of /.</h1>\n<ul>\n"
37-
b'<li><a href="/static/my_dir">my_dir/</a></li>\n'
38-
b'<li><a href="/static/my_file">my_file</a></li>\n'
39-
b"</ul>\n</body>\n</html>",
34+
"/static",
35+
b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
36+
b' /.</h1>\n<ul>\n<li><a href="/static/my_dir">my_dir/</a></li>\n<li><a href="'
37+
b'/static/my_file">my_file</a></li>\n</ul>\n</body>\n</html>',
4038
id="index_static",
4139
),
40+
pytest.param(
41+
True,
42+
200,
43+
"/static",
44+
"/static/my_dir",
45+
b"<html>\n<head>\n<title>Index of /my_dir</title>\n</head>\n<body>\n<h1>"
46+
b'Index of /my_dir</h1>\n<ul>\n<li><a href="/static/my_dir/my_file_in_dir">'
47+
b"my_file_in_dir</a></li>\n</ul>\n</body>\n</html>",
48+
id="index_subdir",
49+
),
4250
],
4351
)
4452
async def test_access_root_of_static_handler(
@@ -47,6 +55,7 @@ async def test_access_root_of_static_handler(
4755
show_index: bool,
4856
status: int,
4957
prefix: str,
58+
request_path: str,
5059
data: Optional[bytes],
5160
) -> None:
5261
# Tests the operation of static file server.
@@ -72,7 +81,94 @@ async def test_access_root_of_static_handler(
7281
client = await aiohttp_client(app)
7382

7483
# Request the root of the static directory.
75-
async with await client.get(prefix) as r:
84+
async with await client.get(request_path) as r:
85+
assert r.status == status
86+
87+
if data:
88+
assert r.headers["Content-Type"] == "text/html; charset=utf-8"
89+
read_ = await r.read()
90+
assert read_ == data
91+
92+
93+
@pytest.mark.internal # Dependent on filesystem
94+
@pytest.mark.skipif(
95+
not sys.platform.startswith("linux"),
96+
reason="Invalid filenames on some filesystems (like Windows)",
97+
)
98+
@pytest.mark.parametrize(
99+
"show_index,status,prefix,request_path,data",
100+
[
101+
pytest.param(False, 403, "/", "/", None, id="index_forbidden"),
102+
pytest.param(
103+
True,
104+
200,
105+
"/",
106+
"/",
107+
b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
108+
b' /.</h1>\n<ul>\n<li><a href="/%3Cimg%20src=0%20onerror=alert(1)%3E.dir">&l'
109+
b't;img src=0 onerror=alert(1)&gt;.dir/</a></li>\n<li><a href="/%3Cimg%20sr'
110+
b'c=0%20onerror=alert(1)%3E.txt">&lt;img src=0 onerror=alert(1)&gt;.txt</a></l'
111+
b"i>\n</ul>\n</body>\n</html>",
112+
),
113+
pytest.param(
114+
True,
115+
200,
116+
"/static",
117+
"/static",
118+
b"<html>\n<head>\n<title>Index of /.</title>\n</head>\n<body>\n<h1>Index of"
119+
b' /.</h1>\n<ul>\n<li><a href="/static/%3Cimg%20src=0%20onerror=alert(1)%3E.'
120+
b'dir">&lt;img src=0 onerror=alert(1)&gt;.dir/</a></li>\n<li><a href="/stat'
121+
b'ic/%3Cimg%20src=0%20onerror=alert(1)%3E.txt">&lt;img src=0 onerror=alert(1)&'
122+
b"gt;.txt</a></li>\n</ul>\n</body>\n</html>",
123+
id="index_static",
124+
),
125+
pytest.param(
126+
True,
127+
200,
128+
"/static",
129+
"/static/<img src=0 onerror=alert(1)>.dir",
130+
b"<html>\n<head>\n<title>Index of /&lt;img src=0 onerror=alert(1)&gt;.dir</t"
131+
b"itle>\n</head>\n<body>\n<h1>Index of /&lt;img src=0 onerror=alert(1)&gt;.di"
132+
b'r</h1>\n<ul>\n<li><a href="/static/%3Cimg%20src=0%20onerror=alert(1)%3E.di'
133+
b'r/my_file_in_dir">my_file_in_dir</a></li>\n</ul>\n</body>\n</html>',
134+
id="index_subdir",
135+
),
136+
],
137+
)
138+
async def test_access_root_of_static_handler_xss(
139+
tmp_path: pathlib.Path,
140+
aiohttp_client: AiohttpClient,
141+
show_index: bool,
142+
status: int,
143+
prefix: str,
144+
request_path: str,
145+
data: Optional[bytes],
146+
) -> None:
147+
# Tests the operation of static file server.
148+
# Try to access the root of static file server, and make
149+
# sure that correct HTTP statuses are returned depending if we directory
150+
# index should be shown or not.
151+
# Ensure that html in file names is escaped.
152+
# Ensure that links are url quoted.
153+
my_file = tmp_path / "<img src=0 onerror=alert(1)>.txt"
154+
my_dir = tmp_path / "<img src=0 onerror=alert(1)>.dir"
155+
my_dir.mkdir()
156+
my_file_in_dir = my_dir / "my_file_in_dir"
157+
158+
with my_file.open("w") as fw:
159+
fw.write("hello")
160+
161+
with my_file_in_dir.open("w") as fw:
162+
fw.write("world")
163+
164+
app = web.Application()
165+
166+
# Register global static route:
167+
app.router.add_static(prefix, str(tmp_path), show_index=show_index)
168+
client = await aiohttp_client(app)
169+
170+
# Request the root of the static directory.
171+
async with await client.get(request_path) as r:
76172
assert r.status == status
77173

78174
if data:

0 commit comments

Comments
 (0)