Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
55 changes: 52 additions & 3 deletions sanic/handlers/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@
from sanic.response import file, html, redirect


def _is_path_within_root(path: Path, root: Path) -> bool:
"""Check if a path (after resolution) is within the root directory.

Returns False for:
- Broken symlinks (cannot be resolved)
- Paths that resolve outside the root directory
- Any errors during resolution
"""
try:
resolved = path.resolve()
resolved.relative_to(root.resolve())
except (ValueError, OSError, RuntimeError):
return False
else:
return True


class DirectoryHandler:
"""Serve files from a directory.

Expand All @@ -22,6 +39,13 @@ class DirectoryHandler:
directory_view (bool): Whether to show a directory listing or not.
index (Optional[Union[str, Sequence[str]]]): The index file(s) to
serve if the directory is requested. Defaults to None.
root_path (Optional[Path]): The root path for security checks.
Symlinks resolving outside this path will be hidden from
directory listings. Defaults to directory if not specified.
follow_external_symlink_files (bool): Whether to show file symlinks
pointing outside root in directory listings. Defaults to False.
follow_external_symlink_dirs (bool): Whether to show directory symlinks
pointing outside root in directory listings. Defaults to False.
"""

def __init__(
Expand All @@ -30,6 +54,9 @@ def __init__(
directory: Path,
directory_view: bool = False,
index: Optional[Union[str, Sequence[str]]] = None,
root_path: Optional[Path] = None,
follow_external_symlink_files: bool = False,
follow_external_symlink_dirs: bool = False,
) -> None:
if isinstance(index, str):
index = [index]
Expand All @@ -39,6 +66,9 @@ def __init__(
self.directory = directory
self.directory_view = directory_view
self.index = tuple(index)
self.root_path = root_path if root_path is not None else directory
self.follow_external_symlink_files = follow_external_symlink_files
self.follow_external_symlink_dirs = follow_external_symlink_dirs

async def handle(self, request: Request, path: str):
"""Handle the request.
Expand Down Expand Up @@ -81,8 +111,13 @@ def _index(self, location: Path, path: str, debug: bool):
page = DirectoryPage(self._iter_files(location), path, debug)
return html(page.render())

def _prepare_file(self, path: Path) -> dict[str, Union[int, str]]:
stat = path.stat()
def _prepare_file(
self, path: Path
) -> Optional[dict[str, Union[int, str]]]:
try:
stat = path.stat()
except OSError:
return None
modified = (
datetime.fromtimestamp(stat.st_mtime)
.isoformat()[:19]
Expand All @@ -102,7 +137,21 @@ def _prepare_file(self, path: Path) -> dict[str, Union[int, str]]:
}

def _iter_files(self, location: Path) -> Iterable[FileInfo]:
prepared = [self._prepare_file(f) for f in location.iterdir()]
prepared = []
for f in location.iterdir():
if f.is_symlink() and not _is_path_within_root(f, self.root_path):
# External symlink - check if allowed based on type
try:
is_dir = f.resolve().is_dir()
except OSError:
continue # Broken symlink
if is_dir and not self.follow_external_symlink_dirs:
continue
if not is_dir and not self.follow_external_symlink_files:
continue
file_info = self._prepare_file(f)
if file_info is not None:
prepared.append(file_info)
for item in sorted(prepared, key=itemgetter("priority", "file_name")):
del item["priority"]
yield cast(FileInfo, item)
89 changes: 65 additions & 24 deletions sanic/mixins/static.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ def static(
index: Optional[Union[str, Sequence[str]]] = None,
directory_view: bool = False,
directory_handler: Optional[DirectoryHandler] = None,
follow_external_symlink_files: bool = False,
follow_external_symlink_dirs: bool = False,
):
"""Register a root to serve files from. The input can either be a file or a directory.

Expand Down Expand Up @@ -91,6 +93,12 @@ def static(
instance of DirectoryHandler that can be used for explicitly
controlling and subclassing the behavior of the default
directory handler.
follow_external_symlink_files (bool, optional): Whether to serve
files that are symlinks pointing outside the static root.
Defaults to `False` for security.
follow_external_symlink_dirs (bool, optional): Whether to serve
files from directories that are symlinks pointing outside
the static root. Defaults to `False` for security.

Returns:
List[sanic.router.Route]: Routes registered on the router.
Expand Down Expand Up @@ -142,6 +150,9 @@ def static(
directory=file_or_directory,
directory_view=directory_view,
index=index,
root_path=file_or_directory,
follow_external_symlink_files=follow_external_symlink_files,
follow_external_symlink_dirs=follow_external_symlink_dirs,
)

static = FutureStatic(
Expand All @@ -157,6 +168,8 @@ def static(
content_type,
resource_type,
directory_handler,
follow_external_symlink_files,
follow_external_symlink_dirs,
)
self._future_statics.add(static)

Expand Down Expand Up @@ -231,6 +244,8 @@ def _register_static(
stream_large_files=static.stream_large_files,
content_type=static.content_type,
directory_handler=static.directory_handler,
follow_external_symlink_files=static.follow_external_symlink_files,
follow_external_symlink_dirs=static.follow_external_symlink_dirs,
)
)

Expand All @@ -254,6 +269,8 @@ async def _static_request_handler(
use_content_range: bool,
stream_large_files: Union[bool, int],
directory_handler: DirectoryHandler,
follow_external_symlink_files: bool,
follow_external_symlink_dirs: bool,
content_type: Optional[str] = None,
__file_uri__: Optional[str] = None,
):
Expand All @@ -265,7 +282,11 @@ async def _static_request_handler(

# Merge served directory and requested file if provided
file_path = await self._get_file_path(
file_or_directory, __file_uri__, not_found
file_or_directory,
__file_uri__,
not_found,
follow_external_symlink_files,
follow_external_symlink_dirs,
)

try:
Expand Down Expand Up @@ -339,33 +360,53 @@ async def _static_request_handler(
)
raise

async def _get_file_path(self, file_or_directory, __file_uri__, not_found):
file_path_raw = Path(unquote(file_or_directory))
root_path = file_path = file_path_raw.resolve()
async def _get_file_path(
self,
file_or_directory,
__file_uri__,
not_found,
follow_external_symlink_files: bool,
follow_external_symlink_dirs: bool,
):
"""
Resolve a filesystem path safely.

Security goals:
- Prevent path traversal via `..`
- Prevent escaping the root via symlinks unless explicitly allowed
- Treat file URIs as relative paths even if they look absolute
"""

def reject():
error_logger.exception(
f"File not found: path={file_or_directory}, "
f"relative_url={__file_uri__}"
)
raise not_found

root_raw = Path(unquote(file_or_directory))
root_path = root_raw.resolve()
file_path_raw = root_raw

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 ".." in file_path_raw.parts:
error_logger.exception(
f"File not found: path={file_or_directory}, "
f"relative_url={__file_uri__}"
)
raise not_found
# URLs may start with `/`, Path() interprets as absolute
rel_uri = unquote(__file_uri__).lstrip("/")
file_path_raw = Path(root_raw, rel_uri)

if ".." in file_path_raw.parts:
reject()

file_path = file_path_raw.resolve()

try:
file_path.relative_to(root_path)
except ValueError:
if not file_path_raw.is_symlink():
error_logger.exception(
f"File not found: path={file_or_directory}, "
f"relative_url={__file_uri__}"
)
raise not_found
is_file_symlink = file_path_raw.is_symlink()
if is_file_symlink:
allowed = follow_external_symlink_files
else:
allowed = follow_external_symlink_dirs
Comment thread
ahopkins marked this conversation as resolved.
Outdated
if not allowed:
reject()

return file_path
2 changes: 2 additions & 0 deletions sanic/models/futures.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ class FutureStatic(NamedTuple):
content_type: Optional[str]
resource_type: Optional[str]
directory_handler: DirectoryHandler
follow_external_symlink_files: bool
follow_external_symlink_dirs: bool


class FutureSignal(NamedTuple):
Expand Down
8 changes: 6 additions & 2 deletions tests/test_static.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def test_static_file_pathlib_relative_path_traversal(
pytest.skip("Current working directory does not end with 'sanic'")

file_path = "./tests/static/../static/"
app.static("/", file_path)
app.static("/", file_path, follow_external_symlink_files=True)
_, response = app.test_client.get(f"/{file_name}")
assert response.status == 200
assert response.body == get_file_content(static_file_directory, file_name)
Expand Down Expand Up @@ -224,7 +224,11 @@ def test_static_file_content_type_forced(app, static_file_directory):
)
@pytest.mark.parametrize("base_uri", ["/static", "", "/dir"])
def test_static_directory(app, file_name, base_uri, static_file_directory):
app.static(base_uri, static_file_directory)
app.static(
base_uri,
static_file_directory,
follow_external_symlink_files=(file_name == "symlink"),
)

request, response = app.test_client.get(uri=f"{base_uri}/{file_name}")
assert response.status == 200
Expand Down
Loading
Loading