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

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

Closed
1 task done
chuckds opened this issue Mar 27, 2023 · 2 comments · Fixed by #2728
Closed
1 task done

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

chuckds opened this issue Mar 27, 2023 · 2 comments · Fixed by #2728
Labels

Comments

@chuckds
Copy link

chuckds commented Mar 27, 2023

Is there an existing issue for this?

  • I have searched the existing issues

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.

@chuckds chuckds added the bug label Mar 27, 2023
@Tronic
Copy link
Member

Tronic commented Mar 27, 2023

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants