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.

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

Use pathlib for path resolution #2506

Merged
merged 4 commits into from
Jul 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ coverage:
default:
target: auto
threshold: 0.75
informational: true
project:
default:
target: auto
Expand Down
61 changes: 30 additions & 31 deletions sanic/mixins/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
from functools import partial, wraps
from inspect import getsource, signature
from mimetypes import guess_type
from os import path, sep
from pathlib import PurePath
from os import path
from pathlib import Path, PurePath
from textwrap import dedent
from time import gmtime, strftime
from typing import (
Expand All @@ -26,12 +26,7 @@
from sanic.compat import stat_async
from sanic.constants import DEFAULT_HTTP_CONTENT_TYPE, HTTP_METHODS
from sanic.errorpages import RESPONSE_MAPPING
from sanic.exceptions import (
BadRequest,
FileNotFound,
HeaderNotFound,
RangeNotSatisfiable,
)
from sanic.exceptions import FileNotFound, HeaderNotFound, RangeNotSatisfiable
from sanic.handlers import ContentRangeHandler
from sanic.log import error_logger
from sanic.models.futures import FutureRoute, FutureStatic
Expand Down Expand Up @@ -806,31 +801,39 @@ async def _static_request_handler(
__file_uri__=None,
):
# Merge served directory and requested file if provided
root_path = file_path = path.abspath(unquote(file_or_directory))
file_path_raw = Path(unquote(file_or_directory))
root_path = file_path = file_path_raw.resolve()
not_found = FileNotFound(
"File not found",
path=file_or_directory,
relative_url=__file_uri__,
)

if __file_uri__:
# Strip all / that in the beginning of the URL to help prevent
# python from herping a derp and treating the uri as an
# absolute path
unquoted_file_uri = unquote(__file_uri__).lstrip("/")
file_path_raw = Path(file_or_directory, unquoted_file_uri)
file_path = file_path_raw.resolve()
if (
file_path < root_path and not file_path_raw.is_symlink()
) or file_path_raw.match("../**/*"):
error_logger.exception(
f"File not found: path={file_or_directory}, "
f"relative_url={__file_uri__}"
)
raise not_found

segments = unquoted_file_uri.split("/")
if ".." in segments or any(sep in segment for segment in segments):
raise BadRequest("Invalid URL")

file_path = path.join(file_or_directory, unquoted_file_uri)
file_path = path.abspath(file_path)

if not file_path.startswith(root_path):
error_logger.exception(
f"File not found: path={file_or_directory}, "
f"relative_url={__file_uri__}"
)
raise FileNotFound(
"File not found",
path=file_or_directory,
relative_url=__file_uri__,
)
try:
file_path.relative_to(root_path)
ChihweiLHBird marked this conversation as resolved.
Show resolved Hide resolved
except ValueError:
if not file_path_raw.is_symlink():
ChihweiLHBird marked this conversation as resolved.
Show resolved Hide resolved
error_logger.exception(
f"File not found: path={file_or_directory}, "
f"relative_url={__file_uri__}"
)
raise not_found
try:
headers = {}
# Check if the client has been sent this file before
Expand Down Expand Up @@ -898,11 +901,7 @@ async def _static_request_handler(
except RangeNotSatisfiable:
raise
except FileNotFoundError:
raise FileNotFound(
"File not found",
path=file_or_directory,
relative_url=__file_uri__,
)
raise not_found
except Exception:
error_logger.exception(
f"Exception in static request handler: "
Expand Down
9 changes: 6 additions & 3 deletions tests/test_static.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,8 +616,11 @@ def test_dotted_dir_ok(
def test_breakout(app: Sanic, static_file_directory: str):
app.static("/foo", static_file_directory)

_, response = app.test_client.get("/foo/..%2Ffake/server.py")
assert response.status == 404

_, response = app.test_client.get("/foo/..%2Fstatic/test.file")
assert response.status == 400
assert response.status == 404


@pytest.mark.skipif(
Expand All @@ -629,6 +632,6 @@ def test_double_backslash_prohibited_on_win32(
app.static("/foo", static_file_directory)

_, response = app.test_client.get("/foo/static/..\\static/test.file")
assert response.status == 400
assert response.status == 404
_, response = app.test_client.get("/foo/static\\../static/test.file")
assert response.status == 400
assert response.status == 404