dirfs: reject paths that escape the root via ".."

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]

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