From 4c6c59e71d88634592a30c07b10707845c3cd812 Mon Sep 17 00:00:00 2001 From: Amin Alaee Date: Fri, 10 Jun 2022 15:21:44 +0200 Subject: [PATCH 1/6] Allow StaticFiles follow symlinks Add test to prevent path traversal Update requirements.txt Update tests --- requirements.txt | 2 +- starlette/staticfiles.py | 2 +- tests/test_staticfiles.py | 49 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index c834ac8b8..d016660c8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -18,7 +18,7 @@ trio==0.19.0 # Documentation mkdocs==1.3.0 -mkdocs-material==8.2.8 +mkdocs-material==8.3.3 mkautodoc==0.1.0 # Packaging diff --git a/starlette/staticfiles.py b/starlette/staticfiles.py index d09630f35..83c98466c 100644 --- a/starlette/staticfiles.py +++ b/starlette/staticfiles.py @@ -161,7 +161,7 @@ def lookup_path( self, path: str ) -> typing.Tuple[str, typing.Optional[os.stat_result]]: for directory in self.all_directories: - full_path = os.path.realpath(os.path.join(directory, path)) + full_path = os.path.abspath(os.path.join(directory, path)) directory = os.path.realpath(directory) if os.path.commonprefix([full_path, directory]) != directory: # Don't allow misbehaving clients to break out of the static files diff --git a/tests/test_staticfiles.py b/tests/test_staticfiles.py index 7d13a0522..382716d49 100644 --- a/tests/test_staticfiles.py +++ b/tests/test_staticfiles.py @@ -441,3 +441,52 @@ def mock_timeout(*args, **kwargs): response = client.get("/example.txt") assert response.status_code == 500 assert response.text == "Internal Server Error" + + +def test_staticfiles_follows_symlinks(tmpdir, test_client_factory): + statics_path = os.path.join(tmpdir, "statics") + os.mkdir(statics_path) + + symlink_path = os.path.join(tmpdir, "symlink") + os.mkdir(symlink_path) + + symlink_file_path = os.path.join(symlink_path, "index.html") + with open(symlink_file_path, "w") as file: + file.write("

Hello

") + + statics_file_path = os.path.join(statics_path, "index.html") + os.symlink(symlink_file_path, statics_file_path) + + app = StaticFiles(directory=statics_path) + client = test_client_factory(app) + + response = client.get("/index.html") + assert response.url == "http://testserver/index.html" + assert response.status_code == 200 + assert response.text == "

Hello

" + + +def test_staticfiles_disallows_path_traversal_with_symlinks(tmpdir): + statics_path = os.path.join(tmpdir, "statics") + os.mkdir(statics_path) + + symlink_path = os.path.join(tmpdir, "symlink") + os.mkdir(symlink_path) + + symlink_file_path = os.path.join(symlink_path, "index.html") + with open(symlink_file_path, "w") as file: + file.write("

Hello

") + + temp_path = os.path.join(tmpdir, "index.html") + os.symlink(symlink_file_path, temp_path) + + app = StaticFiles(directory=statics_path) + # We can't test this with 'requests', so we test the app directly here. + path = app.get_path({"path": "/../index.html"}) + scope = {"method": "GET"} + + with pytest.raises(HTTPException) as exc_info: + anyio.run(app.get_response, path, scope) + + assert exc_info.value.status_code == 404 + assert exc_info.value.detail == "Not Found" From a799da72730b1da3c1030367bb05be482f22fbbd Mon Sep 17 00:00:00 2001 From: Amin Alaee Date: Fri, 10 Jun 2022 15:21:44 +0200 Subject: [PATCH 2/6] Allow StaticFiles follow symlinks Add test to prevent path traversal Update requirements.txt Update tests --- requirements.txt | 2 +- starlette/staticfiles.py | 2 +- tests/test_staticfiles.py | 49 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index c834ac8b8..d016660c8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -18,7 +18,7 @@ trio==0.19.0 # Documentation mkdocs==1.3.0 -mkdocs-material==8.2.8 +mkdocs-material==8.3.3 mkautodoc==0.1.0 # Packaging diff --git a/starlette/staticfiles.py b/starlette/staticfiles.py index d09630f35..83c98466c 100644 --- a/starlette/staticfiles.py +++ b/starlette/staticfiles.py @@ -161,7 +161,7 @@ def lookup_path( self, path: str ) -> typing.Tuple[str, typing.Optional[os.stat_result]]: for directory in self.all_directories: - full_path = os.path.realpath(os.path.join(directory, path)) + full_path = os.path.abspath(os.path.join(directory, path)) directory = os.path.realpath(directory) if os.path.commonprefix([full_path, directory]) != directory: # Don't allow misbehaving clients to break out of the static files diff --git a/tests/test_staticfiles.py b/tests/test_staticfiles.py index 7d13a0522..382716d49 100644 --- a/tests/test_staticfiles.py +++ b/tests/test_staticfiles.py @@ -441,3 +441,52 @@ def mock_timeout(*args, **kwargs): response = client.get("/example.txt") assert response.status_code == 500 assert response.text == "Internal Server Error" + + +def test_staticfiles_follows_symlinks(tmpdir, test_client_factory): + statics_path = os.path.join(tmpdir, "statics") + os.mkdir(statics_path) + + symlink_path = os.path.join(tmpdir, "symlink") + os.mkdir(symlink_path) + + symlink_file_path = os.path.join(symlink_path, "index.html") + with open(symlink_file_path, "w") as file: + file.write("

Hello

") + + statics_file_path = os.path.join(statics_path, "index.html") + os.symlink(symlink_file_path, statics_file_path) + + app = StaticFiles(directory=statics_path) + client = test_client_factory(app) + + response = client.get("/index.html") + assert response.url == "http://testserver/index.html" + assert response.status_code == 200 + assert response.text == "

Hello

" + + +def test_staticfiles_disallows_path_traversal_with_symlinks(tmpdir): + statics_path = os.path.join(tmpdir, "statics") + os.mkdir(statics_path) + + symlink_path = os.path.join(tmpdir, "symlink") + os.mkdir(symlink_path) + + symlink_file_path = os.path.join(symlink_path, "index.html") + with open(symlink_file_path, "w") as file: + file.write("

Hello

") + + temp_path = os.path.join(tmpdir, "index.html") + os.symlink(symlink_file_path, temp_path) + + app = StaticFiles(directory=statics_path) + # We can't test this with 'requests', so we test the app directly here. + path = app.get_path({"path": "/../index.html"}) + scope = {"method": "GET"} + + with pytest.raises(HTTPException) as exc_info: + anyio.run(app.get_response, path, scope) + + assert exc_info.value.status_code == 404 + assert exc_info.value.detail == "Not Found" From 140edb60f47ae6beb1bb4a225825788f858e1122 Mon Sep 17 00:00:00 2001 From: Amin Alaee Date: Mon, 12 Dec 2022 10:14:53 +0100 Subject: [PATCH 3/6] fix naming in tests --- tests/test_staticfiles.py | 45 ++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/tests/test_staticfiles.py b/tests/test_staticfiles.py index cdc119fba..ae694bab0 100644 --- a/tests/test_staticfiles.py +++ b/tests/test_staticfiles.py @@ -1,6 +1,7 @@ import os import pathlib import stat +import tempfile import time import anyio @@ -454,15 +455,13 @@ def test_staticfiles_follows_symlinks(tmpdir, test_client_factory): statics_path = os.path.join(tmpdir, "statics") os.mkdir(statics_path) - symlink_path = os.path.join(tmpdir, "symlink") - os.mkdir(symlink_path) - - symlink_file_path = os.path.join(symlink_path, "index.html") - with open(symlink_file_path, "w") as file: + source_path = tempfile.mkdtemp() + source_file_path = os.path.join(source_path, "page.html") + with open(source_file_path, "w") as file: file.write("

Hello

") statics_file_path = os.path.join(statics_path, "index.html") - os.symlink(symlink_file_path, statics_file_path) + os.symlink(source_file_path, statics_file_path) app = StaticFiles(directory=statics_path) client = test_client_factory(app) @@ -473,19 +472,39 @@ def test_staticfiles_follows_symlinks(tmpdir, test_client_factory): assert response.text == "

Hello

" -def test_staticfiles_disallows_path_traversal_with_symlinks(tmpdir): +def test_staticfiles_follows_symlink_directories(tmpdir, test_client_factory): statics_path = os.path.join(tmpdir, "statics") + statics_html_path = os.path.join(statics_path, "html") os.mkdir(statics_path) - symlink_path = os.path.join(tmpdir, "symlink") - os.mkdir(symlink_path) + source_path = tempfile.mkdtemp() + source_file_path = os.path.join(source_path, "page.html") + with open(source_file_path, "w") as file: + file.write("

Hello

") + + os.symlink(source_path, statics_html_path) + + app = StaticFiles(directory=statics_path) + client = test_client_factory(app) + + response = client.get("/html/page.html") + assert response.url == "http://testserver/html/page.html" + assert response.status_code == 200 + assert response.text == "

Hello

" + + +def test_staticfiles_disallows_path_traversal_with_symlinks(tmpdir): + statics_path = os.path.join(tmpdir, "statics") + + root_source_path = tempfile.mkdtemp() + source_path = os.path.join(root_source_path, "statics") + os.mkdir(source_path) - symlink_file_path = os.path.join(symlink_path, "index.html") - with open(symlink_file_path, "w") as file: + source_file_path = os.path.join(root_source_path, "index.html") + with open(source_file_path, "w") as file: file.write("

Hello

") - temp_path = os.path.join(tmpdir, "index.html") - os.symlink(symlink_file_path, temp_path) + os.symlink(source_path, statics_path) app = StaticFiles(directory=statics_path) # We can't test this with 'requests', so we test the app directly here. From 34fe0839299a606173d0ca7404b404b84786debb Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Sat, 4 Feb 2023 11:19:13 +0100 Subject: [PATCH 4/6] Update tests/test_staticfiles.py --- tests/test_staticfiles.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_staticfiles.py b/tests/test_staticfiles.py index 99f9a3dde..ebe47f84c 100644 --- a/tests/test_staticfiles.py +++ b/tests/test_staticfiles.py @@ -507,7 +507,7 @@ def test_staticfiles_disallows_path_traversal_with_symlinks(tmpdir): os.symlink(source_path, statics_path) app = StaticFiles(directory=statics_path) - # We can't test this with 'requests', so we test the app directly here. + # We can't test this with 'httpx', so we test the app directly here. path = app.get_path({"path": "/../index.html"}) scope = {"method": "GET"} From 7a8d49c218a3d5800a186c9bd37571ee9f1f1059 Mon Sep 17 00:00:00 2001 From: Amin Alaee Date: Sat, 4 Feb 2023 17:29:47 +0100 Subject: [PATCH 5/6] add follow_symlink --- starlette/staticfiles.py | 8 +++++++- tests/test_staticfiles.py | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/starlette/staticfiles.py b/starlette/staticfiles.py index 83c98466c..4d075b3ed 100644 --- a/starlette/staticfiles.py +++ b/starlette/staticfiles.py @@ -45,12 +45,14 @@ def __init__( ] = None, html: bool = False, check_dir: bool = True, + follow_symlink: bool = False, ) -> None: self.directory = directory self.packages = packages self.all_directories = self.get_directories(directory, packages) self.html = html self.config_checked = False + self.follow_symlink = follow_symlink if check_dir and directory is not None and not os.path.isdir(directory): raise RuntimeError(f"Directory '{directory}' does not exist") @@ -161,7 +163,11 @@ def lookup_path( self, path: str ) -> typing.Tuple[str, typing.Optional[os.stat_result]]: for directory in self.all_directories: - full_path = os.path.abspath(os.path.join(directory, path)) + joined_path = os.path.join(directory, path) + if self.follow_symlink: + full_path = os.path.abspath(joined_path) + else: + full_path = os.path.realpath(joined_path) directory = os.path.realpath(directory) if os.path.commonprefix([full_path, directory]) != directory: # Don't allow misbehaving clients to break out of the static files diff --git a/tests/test_staticfiles.py b/tests/test_staticfiles.py index ebe47f84c..eb6c73f7f 100644 --- a/tests/test_staticfiles.py +++ b/tests/test_staticfiles.py @@ -463,7 +463,7 @@ def test_staticfiles_follows_symlinks(tmpdir, test_client_factory): statics_file_path = os.path.join(statics_path, "index.html") os.symlink(source_file_path, statics_file_path) - app = StaticFiles(directory=statics_path) + app = StaticFiles(directory=statics_path, follow_symlink=True) client = test_client_factory(app) response = client.get("/index.html") @@ -484,7 +484,7 @@ def test_staticfiles_follows_symlink_directories(tmpdir, test_client_factory): os.symlink(source_path, statics_html_path) - app = StaticFiles(directory=statics_path) + app = StaticFiles(directory=statics_path, follow_symlink=True) client = test_client_factory(app) response = client.get("/html/page.html") @@ -506,7 +506,7 @@ def test_staticfiles_disallows_path_traversal_with_symlinks(tmpdir): os.symlink(source_path, statics_path) - app = StaticFiles(directory=statics_path) + app = StaticFiles(directory=statics_path, follow_symlink=True) # We can't test this with 'httpx', so we test the app directly here. path = app.get_path({"path": "/../index.html"}) scope = {"method": "GET"} From e3e85db681780ebcad573969febe941e1dba434d Mon Sep 17 00:00:00 2001 From: Amin Alaee Date: Sat, 4 Feb 2023 17:42:51 +0100 Subject: [PATCH 6/6] update docs --- docs/staticfiles.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/staticfiles.md b/docs/staticfiles.md index 3591b4f9f..fa14e6c34 100644 --- a/docs/staticfiles.md +++ b/docs/staticfiles.md @@ -3,12 +3,13 @@ Starlette also includes a `StaticFiles` class for serving files in a given direc ### StaticFiles -Signature: `StaticFiles(directory=None, packages=None, check_dir=True)` +Signature: `StaticFiles(directory=None, packages=None, check_dir=True, follow_symlink=False)` * `directory` - A string or [os.Pathlike][pathlike] denoting a directory path. * `packages` - A list of strings or list of tuples of strings of python packages. * `html` - Run in HTML mode. Automatically loads `index.html` for directories if such file exist. * `check_dir` - Ensure that the directory exists upon instantiation. Defaults to `True`. +* `follow_symlink` - A boolean indicating if symbolic links for files and directories should be followed. Defaults to `False`. You can combine this ASGI application with Starlette's routing to provide comprehensive static file serving.