Skip to content

Commit ee6d8cf

Browse files
prryplatypusahopkinsChihweiLHBird
committed
Prevent directory traversion with static files (#2495)
Co-authored-by: Adam Hopkins <adam@amhopkins.com> Co-authored-by: Zhiwei Liang <zhi.wei.liang@outlook.com>
1 parent c4da66b commit ee6d8cf

2 files changed

Lines changed: 73 additions & 13 deletions

File tree

sanic/mixins/routes.py

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@
33
from functools import partial, wraps
44
from inspect import getsource, signature
55
from mimetypes import guess_type
6-
from os import path
6+
from os import path, sep
77
from pathlib import PurePath
8-
from re import sub
98
from textwrap import dedent
109
from time import gmtime, strftime
1110
from typing import Any, Callable, Iterable, List, Optional, Set, Tuple, Union
@@ -775,23 +774,30 @@ async def _static_request_handler(
775774
content_type=None,
776775
__file_uri__=None,
777776
):
777+
<<<<<<< HEAD
778778
# Using this to determine if the URL is trying to break out of the path
779779
# served. os.path.realpath seems to be very slow
780780
if __file_uri__ and "../" in __file_uri__:
781781
raise InvalidUsage("Invalid URL")
782+
=======
783+
>>>>>>> 9d415e4 (Prevent directory traversion with static files (#2495))
782784
# Merge served directory and requested file if provided
783-
# Strip all / that in the beginning of the URL to help prevent python
784-
# from herping a derp and treating the uri as an absolute path
785-
root_path = file_path = file_or_directory
785+
root_path = file_path = path.abspath(unquote(file_or_directory))
786+
786787
if __file_uri__:
787-
file_path = path.join(
788-
file_or_directory, sub("^[/]*", "", __file_uri__)
789-
)
788+
# Strip all / that in the beginning of the URL to help prevent
789+
# python from herping a derp and treating the uri as an
790+
# absolute path
791+
unquoted_file_uri = unquote(__file_uri__).lstrip("/")
792+
793+
segments = unquoted_file_uri.split("/")
794+
if ".." in segments or any(sep in segment for segment in segments):
795+
raise BadRequest("Invalid URL")
796+
797+
file_path = path.join(file_or_directory, unquoted_file_uri)
798+
file_path = path.abspath(file_path)
790799

791-
# URL decode the path sent by the browser otherwise we won't be able to
792-
# match filenames which got encoded (filenames with spaces etc)
793-
file_path = path.abspath(unquote(file_path))
794-
if not file_path.startswith(path.abspath(unquote(root_path))):
800+
if not file_path.startswith(root_path):
795801
error_logger.exception(
796802
f"File not found: path={file_or_directory}, "
797803
f"relative_url={__file_uri__}"

tests/test_static.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import inspect
22
import logging
33
import os
4+
import sys
45

56
from collections import Counter
67
from pathlib import Path
78
from time import gmtime, strftime
89

910
import pytest
1011

11-
from sanic import text
12+
from sanic import Sanic, text
1213
from sanic.exceptions import FileNotFound
1314

1415

@@ -21,6 +22,22 @@ def static_file_directory():
2122
return static_directory
2223

2324

25+
@pytest.fixture(scope="module")
26+
def double_dotted_directory_file(static_file_directory: str):
27+
"""Generate double dotted directory and its files"""
28+
if sys.platform == "win32":
29+
raise Exception("Windows doesn't support double dotted directories")
30+
31+
file_path = Path(static_file_directory) / "dotted.." / "dot.txt"
32+
double_dotted_dir = file_path.parent
33+
Path.mkdir(double_dotted_dir, exist_ok=True)
34+
with open(file_path, "w") as f:
35+
f.write("DOT\n")
36+
yield file_path
37+
Path.unlink(file_path)
38+
Path.rmdir(double_dotted_dir)
39+
40+
2441
def get_file_path(static_file_directory, file_name):
2542
return os.path.join(static_file_directory, file_name)
2643

@@ -578,3 +595,40 @@ def test_resource_type_dir(app, static_file_directory):
578595
def test_resource_type_unknown(app, static_file_directory, caplog):
579596
with pytest.raises(ValueError):
580597
app.static("/static", static_file_directory, resource_type="unknown")
598+
599+
600+
@pytest.mark.skipif(
601+
sys.platform == "win32",
602+
reason="Windows does not support double dotted directories",
603+
)
604+
def test_dotted_dir_ok(
605+
app: Sanic, static_file_directory: str, double_dotted_directory_file: Path
606+
):
607+
app.static("/foo", static_file_directory)
608+
double_dotted_directory_file = str(double_dotted_directory_file).lstrip(
609+
static_file_directory
610+
)
611+
_, response = app.test_client.get("/foo/" + double_dotted_directory_file)
612+
assert response.status == 200
613+
assert response.body == b"DOT\n"
614+
615+
616+
def test_breakout(app: Sanic, static_file_directory: str):
617+
app.static("/foo", static_file_directory)
618+
619+
_, response = app.test_client.get("/foo/..%2Fstatic/test.file")
620+
assert response.status == 400
621+
622+
623+
@pytest.mark.skipif(
624+
sys.platform != "win32", reason="Block backslash on Windows only"
625+
)
626+
def test_double_backslash_prohibited_on_win32(
627+
app: Sanic, static_file_directory: str
628+
):
629+
app.static("/foo", static_file_directory)
630+
631+
_, response = app.test_client.get("/foo/static/..\\static/test.file")
632+
assert response.status == 400
633+
_, response = app.test_client.get("/foo/static\\../static/test.file")
634+
assert response.status == 400

0 commit comments

Comments
 (0)