Skip to content

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

Merged
simonw merged 5 commits intosimonw:masterfrom
abdusco:static_mount_fix
Jul 11, 2019
Merged

Fix static mounts using relative paths and prevent traversal exploits#554
simonw merged 5 commits intosimonw:masterfrom
abdusco:static_mount_fix

Conversation

@abdusco
Copy link
Contributor

@abdusco abdusco commented Jul 9, 2019 •
edited
Loading

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.

datasette/datasette/utils/asgi.py

Lines 303 to 306 in 81fa8b6

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.

@abdusco abdusco mentioned this pull request Jul 9, 2019
Closed
@abdusco
Fix static mounts with absolute paths not working on Windows
d9812bd 
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
Copy link
Contributor Author

abdusco commented Jul 9, 2019

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

@abdusco
Copy link
Contributor Author

abdusco 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
Copy link
Owner

simonw 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 simonw merged commit 74ecf8a into simonw:master Jul 11, 2019
@simonw
Copy link
Owner

simonw commented Jul 11, 2019

Released as 0.29.1: https://datasette.readthedocs.io/en/latest/changelog.html#v0-29-1

@simonw simonw mentioned this pull request Jul 11, 2019
Open
simonw pushed a commit that referenced this pull request Nov 11, 2019
@abdusco @simonw
Fix static mounts using relative paths and prevent traversal exploits (…
8288950 
…#554)

Thanks, @abdusco! Closes #555
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@abdusco @simonw