sanic-org / sanic

Accelerate your web app development | Build fast. Run fast.
https://sanic.dev
MIT License
18.01k stars 1.54k forks source link

Static routes containing ".." don't work #2726

Closed chuckds closed 1 year ago

chuckds commented 1 year ago

Is there an existing issue for this?

Describe the bug

Sanic does serve static files if the server file path contains "..", instead it returns file not found errors.

Code snippet

The following doesn't work:

from sanic import Sanic

app = Sanic("Example")
app.static("/", "./static/../static/")
if __name__ == "__main__":
    app.run(host="0.0.0.0", port=8000)

Attempt to access any files in the static directory will get the error: [2023-03-27 04:05:31 -0700] [60813] [ERROR] File not found: path=static/../static, relative_url=test.txt Removing the redundant ../static/ will result in the files being correctly served.

Expected Behavior

The above example should serve all files in the static directory and not return file not found errors.

How do you run Sanic?

Sanic CLI

Operating System

linux

Sanic Version

22.9.1

Additional context

This issue appears to have been introduced by #2506 or #2508. The fix might look something like the following:

diff --git a/sanic/mixins/static.py b/sanic/mixins/static.py
index bcffbc8..28effc4 100644
--- a/sanic/mixins/static.py
+++ b/sanic/mixins/static.py
@@ -323,7 +323,7 @@ class StaticHandleMixin(metaclass=SanicMeta):
             # 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_raw = Path(root_path, unquoted_file_uri)
             file_path = file_path_raw.resolve()
             if (
                 file_path < root_path and not file_path_raw.is_symlink()

It looks like the ".." was originally being done on the URI part only but with the #2506 change this is now done on the combination of the root path and relative path. The above change instead uses the resolved form of the root path (which won't contain "..").

However, there are likely other options and having more context about what the checks in this area are for would be crucial.

Tronic commented 1 year ago

Looks like Sanic should resolve that static dir path when the route is defined, making it an absolute path without any .. elements. If that is the only thing missing, it should be easy to fix this. PRs welcome, if you are up to that :)

ahopkins commented 1 year ago

https://github.com/sanic-org/sanic/blob/main/sanic/mixins/static.py#L98

All we need is a .resolve() on this line and a test.