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

Fix static mounts using relative paths and prevent traversal exploits #554

Merged
merged 5 commits into from Jul 11, 2019

Conversation

Projects
None yet
2 participants
@abdusco
Copy link
Contributor

commented Jul 9, 2019

While debugging why my static mounts using a relative path (--static mystatic:rel/path/to/dir) not working, I noticed that the requests fail no matter what, returning 404 errors.

The reason is that datasette tries to prevent traversal exploits by checking if the path is relative to its registered directory. This check fails when the mount is a relative directory, because /abs/dir/file obviously not under dir/file.

full_path = (Path(root_path) / path).absolute()
# Ensure full_path is within root_path to avoid weird "../" tricks
try:
full_path.relative_to(root_path)

This also has the consequence of returning any requested file, because when /abs/dir/../../evil.file resolves aiofiles happily returns it to the client after it resolves the path itself. The solution is to make sure we're checking relativity of paths after they're fully resolved.

I've implemented the mentioned changes and also updated the tests.

Fix static mounts with absolute paths not working on Windows
Since paths include a drive prefix like `C:` on Windows, splitting mount directive gives three segments, and python fails to unpack it into two variables.
@abdusco

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

I've also added another fix for using static mounts with absolute paths on Windows.

@abdusco

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

I wanted to add a test for it too, but I've realized it's impossible to test a server process as we cannot get its exit code.

# tests/test_cli.py
def test_static_mounts_on_windows():
    if sys.platform != "win32":
        return
    runner = CliRunner()
    result = runner.invoke(
        cli, ["serve", "--static", r"s:C:\\"]
    )
    assert result.exit_code == 0
@simonw

This comment has been minimized.

Copy link
Owner

commented Jul 11, 2019

Thanks for this!

The tests are failing for Python 3.5 right now which is strange. https://travis-ci.org/simonw/datasette/jobs/556272017

One failure looks like this:

app_client = <tests.fixtures.TestClient object at 0x7fb7803285f8>
    def test_static(app_client):
        response = app_client.get("/-/static/app2.css")
>       assert response.status == 404
E       assert 500 == 404
E        +  where 500 = <tests.fixtures.TestResponse object at 0x7fb780328a58>.status
/home/travis/build/simonw/datasette/tests/test_html.py:56: AssertionError
----------------------------- Captured stderr call -----------------------------
Traceback (most recent call last):
  File "/home/travis/build/simonw/datasette/datasette/utils/asgi.py", line 100, in __call__
    return await view(new_scope, receive, send)
  File "/home/travis/build/simonw/datasette/datasette/utils/asgi.py", line 303, in inner_static
    full_path = (Path(root_path) / path).resolve().absolute()
  File "/opt/python/3.5.6/lib/python3.5/pathlib.py", line 1109, in resolve
    s = self._flavour.resolve(self)
  File "/opt/python/3.5.6/lib/python3.5/pathlib.py", line 330, in resolve
    return _resolve(base, str(path)) or sep
  File "/opt/python/3.5.6/lib/python3.5/pathlib.py", line 315, in _resolve
    target = accessor.readlink(newpath)
  File "/opt/python/3.5.6/lib/python3.5/pathlib.py", line 422, in readlink
    return os.readlink(path)
FileNotFoundError: [Errno 2] No such file or directory: '/home/travis/build/simonw/datasette/datasette/static/app2.css'

Maybe an exception was renamed between 3.5 and 3.6?

simonw added some commits Jul 11, 2019

@simonw simonw merged commit 74ecf8a into simonw:master Jul 11, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@simonw

This comment has been minimized.

Copy link
Owner

commented Jul 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.