From 05002d7ee4f2254bcf054d7157cf73af01e54255 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 31 Jul 2022 14:09:01 +0300 Subject: [PATCH] Path protection with pathlib --- sanic/static.py | 45 ++++++++++++++++++++++++++------------------ tests/test_static.py | 16 +++++++++------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/sanic/static.py b/sanic/static.py index 875fa3df..19ab9115 100644 --- a/sanic/static.py +++ b/sanic/static.py @@ -1,6 +1,7 @@ from functools import partial, wraps from mimetypes import guess_type from os import path, sep +from pathlib import Path from time import gmtime, strftime from urllib.parse import unquote @@ -26,31 +27,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 ".." in file_path_raw.parts: + 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 InvalidUsage("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) + 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 try: headers = {} diff --git a/tests/test_static.py b/tests/test_static.py index e5ecd673..45281707 100644 --- a/tests/test_static.py +++ b/tests/test_static.py @@ -404,11 +404,10 @@ def test_dotted_dir_ok( app: Sanic, static_file_directory: str, double_dotted_directory_file: Path ): app.static("/foo", static_file_directory) - url = ( - "/foo" - + str(double_dotted_directory_file)[len(static_file_directory) :] + dot_relative_path = str( + double_dotted_directory_file.relative_to(static_file_directory) ) - _, response = app.test_client.get(url) + _, response = app.test_client.get("/foo/" + dot_relative_path) assert response.status == 200 assert response.body == b"DOT\n" @@ -416,8 +415,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( @@ -429,6 +431,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