starlette: StaticFiles middleware doesn't follow symlinks

Report about the incident: https://github.com/encode/starlette/pull/1681#issuecomment-1152178256

Yeah, I’m going to clarify things in a few hours. 👍

I would be interested to know why the change had to be reverted.

A fix is already merged ,and it will be available on the next release (we are at 0.20.1 by the time I wrote this). 🙌

Just an additional piece of info here, this causes issues when using bazel, which relies heavily on symlinks.

In particular a dep of ours uses starlette underneath and because the underlying static file is a symlink (managed by bazel), starlette throws on lookup, making the package not usable.

Wow this is a crazy coincidence! Using Bazel trying to setup Dagster which relies on Starlette and was only getting empty pages with assets not being served. I can now stop pulling my hair out. +1 to having this fixed!

Same re dagster 😃

Currently using this workaround until a new version of Starlette is released. It’s really hacky but it gets the job done 🤣

dagit_wrapper.py

from dagit.cli import main
import os

from dagit.webserver import DagitWebserver
from starlette.routing import Mount
from starlette.staticfiles import StaticFiles

# Necessary because of https://github.com/encode/starlette/issues/1083
def custom_build_static_routes(self):
    manifest_path = self.relative_path("webapp/build/manifest.json")
    real_manifest_path = os.path.realpath(manifest_path)
    real_build_dir = os.path.dirname(real_manifest_path)

    return [
        Mount(
            "/static",
            StaticFiles(
                directory=os.path.join(real_build_dir, "static"),
                check_dir=False,
            ),
            name="static",
        ),
        Mount(
            "/vendor",
            StaticFiles(
                directory=os.path.join(real_build_dir, "vendor"),
                check_dir=False,
            ),
            name="vendor",
        ),
        *self.root_static_file_routes(),
    ]


DagitWebserver.build_static_routes = custom_build_static_routes

if __name__ == "__main__":
    working_directory = os.environ["BUILD_WORKING_DIRECTORY"]
    os.chdir(working_directory)
    main()

flask

In flask, static files are served with:

send_static_file -> send_from_directory -> werkzeug.safe_join

safe_join checks that the provided path isn’t above the current directory, but it will happily follow a symlink that points above the process’s working directory (I tested this).

django

In django, the staticfiles finders call FileSystemStorage.path which uses a similar safe_join

from testing, it seems like this will also follow a symlink above the working directory

edit note django by default only serves static files when DEBUG=1, see here for example

symlink upload

Re someone’s point about what if users can write symlinks to the StaticFiles dir – would be interested to know if this is possible via normal HTTP file uploads.

In ext4, for example, I think symlink status is stored in the inode table along with the uga/rwx mode bits. From skimming the nginx upload module, it doesn’t look like those get transferred. It sets permission here from store_access, which is a config, not from the upload. (I could be wrong about this).

Also the RFC for HTTP uploads doesn’t talk about symlinks or mode bits https://www.rfc-editor.org/rfc/rfc1867.

I’m assuming starlette also doesn’t receive or set mode bits for file uploads?

opt-in

I like the idea of an opt-in follow_symlink setting – in my case I would enable it in DEBUG and disable in prod.

sorry for not thinking through security issues when posting the feature request

great catch, thanks to kind stranger on gitter, and thanks for being quick + transparent about the fix

here’s a subclass that solves the problem

you install it the same way you install the stock StaticFiles. I used it with starlette==0.13.8, not sure if it works on newer versions.

class StaticFilesSym(StaticFiles):
  "subclass StaticFiles middleware to allow symlinks"
  async def lookup_path(self, path):
    for directory in self.all_directories:
      full_path = os.path.realpath(os.path.join(directory, path))
      try:
        stat_result = await aio_stat(full_path)
        return (full_path, stat_result)
      except FileNotFoundError:
        pass
    return ("", None)

update jan 2022: this is broken with starlette 0.17. You can patch by:

• async def -> def
• aio_stat -> os.stat

update jun 2022: this patch seems to not be vulnerable to the same security issue as #1681 – a request for /static/../../../etc/passwd seems to be getting rewritten to / (tested with starlette 0.17.1, fastapi 0.73.0). I’m guessing the .. are being processed before the request hits the middleware.

This is just luck, I wasn’t thinking defensively either. I would recommend disabling my patch in production. (in my case I only need it in local dev mode for hitting incremental webpack builds).

post the PR!!!