diff --git a/fsspec/implementations/dirfs.py b/fsspec/implementations/dirfs.py index 65b9b5da1..0fe1ababf 100644 --- a/fsspec/implementations/dirfs.py +++ b/fsspec/implementations/dirfs.py @@ -1,6 +1,20 @@ from .. import filesystem from ..asyn import AsyncFileSystem from .chained import ChainedFileSystem +from .local import LocalFileSystem + + +def _escapes_root(path): + """Whether a relative path would resolve above its root via ".." segments.""" + depth = 0 + for part in path.split("/"): + if part == "..": + depth -= 1 + if depth < 0: + return True + elif part and part != ".": + depth += 1 + return False class DirFileSystem(AsyncFileSystem, ChainedFileSystem): @@ -54,7 +68,15 @@ def _join(self, path): return path if not path: return self.path - return self.fs.sep.join((self.path, self._strip_protocol(path))) + path = self._strip_protocol(path) + # ".." only navigates above the root on filesystems that resolve it + # against a real directory tree; on object stores it is a literal + # path part, so only guard the local case here. + if isinstance(self.fs, LocalFileSystem) and _escapes_root(path): + raise ValueError( + f"path {path!r} escapes the {self.path!r} root of the filesystem" + ) + return self.fs.sep.join((self.path, path)) if isinstance(path, dict): return {self._join(_path): value for _path, value in path.items()} return [self._join(_path) for _path in path] diff --git a/fsspec/implementations/tests/test_dirfs.py b/fsspec/implementations/tests/test_dirfs.py index 7f963c0ee..fd53134b6 100644 --- a/fsspec/implementations/tests/test_dirfs.py +++ b/fsspec/implementations/tests/test_dirfs.py @@ -2,6 +2,7 @@ from fsspec.asyn import AsyncFileSystem from fsspec.implementations.dirfs import DirFileSystem +from fsspec.implementations.local import LocalFileSystem from fsspec.spec import AbstractFileSystem PATH = "path/to/dir" @@ -104,6 +105,30 @@ def test_path_no_leading_slash(fs, root, rel, full): assert dirfs._relpath(full) == rel +@pytest.mark.parametrize( + "path", + ["..", "../", "../secret", "foo/../..", "foo/../../secret", "/../secret"], +) +def test_join_rejects_escape_local(tmp_path, path): + dirfs = DirFileSystem(str(tmp_path), LocalFileSystem()) + with pytest.raises(ValueError): + dirfs._join(path) + + +@pytest.mark.parametrize("path", ["foo", "foo/bar", "foo/../bar"]) +def test_join_allows_internal_local(tmp_path, path): + dirfs = DirFileSystem(str(tmp_path), LocalFileSystem()) + assert dirfs._join(path) == f"{dirfs.path}/{path}" + + +@pytest.mark.parametrize("path", ["../secret", "foo/../../secret", ".."]) +def test_join_keeps_dotdot_for_non_local(fs, path): + # ".." is only special on local-style filesystems; elsewhere it is a + # legitimate path part and must not be rejected. + dirfs = DirFileSystem("root", fs) + assert dirfs._join(path) == f"root/{path}" + + def test_sep(mocker, dirfs): sep = mocker.Mock() dirfs.fs.sep = sep