prevent out-of-repo access when manipulating references.

codex · Byron · Byron · commit 25ba54dd3fb3 · 2026-04-28T09:24:58.000+08:00
This previously made it possible to create, modify and delete files outside outside
of the repository, which is a problem if inputs aren't trusted.

Co-authored-by: Sebastian Thiel &lt;sebastian.thiel@icloud.com&gt;
diff --git a/git/refs/log.py b/git/refs/log.py
@@ -213,7 +213,7 @@ def path(cls, ref: "SymbolicReference") -> str:
         :param ref:
             :class:`~git.refs.symbolic.SymbolicReference` instance
         """
-        return osp.join(ref.repo.git_dir, "logs", to_native_path(ref.path))
+        return to_native_path(ref._get_validated_reflog_path(ref.repo, ref.path))
 
     @classmethod
     def iter_entries(cls, stream: Union[str, "BytesIO", mmap]) -> Iterator[RefLogEntry]:
diff --git a/git/refs/remote.py b/git/refs/remote.py
@@ -63,12 +63,13 @@ def delete(cls, repo: "Repo", *refs: "RemoteReference", **kwargs: Any) -> None:
         # generally ignored in the refs/ folder. We don't though and delete remainders
         # manually.
         for ref in refs:
+            cls._check_ref_name_valid(ref.path)
             try:
-                os.remove(os.path.join(repo.common_dir, ref.path))
+                os.remove(cls._get_validated_path(repo.common_dir, ref.path))
             except OSError:
                 pass
             try:
-                os.remove(os.path.join(repo.git_dir, ref.path))
+                os.remove(cls._get_validated_path(repo.git_dir, ref.path))
             except OSError:
                 pass
         # END for each ref
diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py
@@ -110,6 +110,32 @@ def name(self) -> str:
     def abspath(self) -> PathLike:
         return join_path_native(_git_dir(self.repo, self.path), self.path)
 
+    @staticmethod
+    def _get_validated_path(base: PathLike, path: PathLike) -> str:
+        path = os.fspath(path)
+        base_path = os.path.realpath(os.fspath(base))
+        abs_path = os.path.realpath(os.path.join(base_path, path))
+        try:
+            common_path = os.path.commonpath([base_path, abs_path])
+        except ValueError as e:
+            raise ValueError("Reference path %r escapes the repository" % path) from e
+        if os.path.normcase(common_path) != os.path.normcase(base_path):
+            raise ValueError("Reference path %r escapes the repository" % path)
+        return abs_path
+
+    @classmethod
+    def _get_validated_ref_path(cls, repo: "Repo", path: PathLike) -> str:
+        """Return the absolute filesystem path for a ref after validating it."""
+        cls._check_ref_name_valid(path)
+        ref_path = os.fspath(path)
+        return cls._get_validated_path(_git_dir(repo, ref_path), ref_path)
+
+    @classmethod
+    def _get_validated_reflog_path(cls, repo: "Repo", path: PathLike) -> str:
+        """Return the absolute filesystem path for a reflog after validating it."""
+        cls._check_ref_name_valid(path)
+        return cls._get_validated_path(os.path.join(repo.git_dir, "logs"), path)
+
     @classmethod
     def _get_packed_refs_path(cls, repo: "Repo") -> str:
         return os.path.join(repo.common_dir, "packed-refs")
@@ -485,7 +511,7 @@ def set_reference(
             # END handle non-existing
         # END retrieve old hexsha
 
-        fpath = self.abspath
+        fpath = self._get_validated_ref_path(self.repo, self.path)
         assure_directory_exists(fpath, is_file=True)
 
         lfd = LockedFD(fpath)
@@ -632,7 +658,7 @@ def delete(cls, repo: "Repo", path: PathLike) -> None:
             Alternatively the symbolic reference to be deleted.
         """
         full_ref_path = cls.to_full_path(path)
-        abs_path = os.path.join(repo.common_dir, full_ref_path)
+        abs_path = cls._get_validated_ref_path(repo, full_ref_path)
         if os.path.exists(abs_path):
             os.remove(abs_path)
         else:
@@ -695,9 +721,8 @@ def _create(
         symbolic reference. Otherwise it will be resolved to the corresponding object
         and a detached symbolic reference will be created instead.
         """
-        git_dir = _git_dir(repo, path)
         full_ref_path = cls.to_full_path(path)
-        abs_ref_path = os.path.join(git_dir, full_ref_path)
+        abs_ref_path = cls._get_validated_ref_path(repo, full_ref_path)
 
         # Figure out target data.
         target = reference
@@ -789,8 +814,8 @@ def rename(self, new_path: PathLike, force: bool = False) -> "SymbolicReference"
         if self.path == new_path:
             return self
 
-        new_abs_path = os.path.join(_git_dir(self.repo, new_path), new_path)
-        cur_abs_path = os.path.join(_git_dir(self.repo, self.path), self.path)
+        new_abs_path = self._get_validated_ref_path(self.repo, new_path)
+        cur_abs_path = self._get_validated_ref_path(self.repo, self.path)
         if os.path.isfile(new_abs_path):
             if not force:
                 # If they point to the same file, it's not an error.
diff --git a/test/test_refs.py b/test/test_refs.py
@@ -18,6 +18,7 @@
     RefLog,
     Reference,
     RemoteReference,
+    Repo,
     SymbolicReference,
     TagReference,
 )
@@ -29,6 +30,14 @@
 
 
 class TestRefs(TestBase):
+    def _repo_with_initial_commit(self, base_dir):
+        repo_dir = base_dir / "repo"
+        repo = Repo.init(repo_dir)
+        (repo_dir / "file.txt").write_text("initial\n", encoding="utf-8")
+        repo.index.add(["file.txt"])
+        repo.index.commit("initial")
+        return repo
+
     def test_from_path(self):
         # Should be able to create any reference directly.
         for ref_type in (Reference, Head, TagReference, RemoteReference):
@@ -648,6 +657,88 @@ def test_refs_outside_repo(self):
             ref_file_name = Path(ref_file.name).name
             self.assertRaises(BadName, self.rorepo.commit, f"../../{ref_file_name}")
 
+    def test_reference_create_rejects_path_traversal(self):
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_dir = Path(tmp_dir)
+            repo = self._repo_with_initial_commit(base_dir)
+            outside_path = base_dir / "outside_write.txt"
+
+            self.assertRaises(ValueError, Reference.create, repo, "../../../outside_write.txt", "HEAD")
+            assert not outside_path.exists()
+
+    def test_symbolic_reference_create_rejects_path_traversal(self):
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_dir = Path(tmp_dir)
+            repo = self._repo_with_initial_commit(base_dir)
+            outside_path = base_dir / "outside_write.txt"
+
+            self.assertRaises(ValueError, SymbolicReference.create, repo, "../../outside_write.txt", "HEAD")
+            assert not outside_path.exists()
+
+    def test_symbolic_reference_set_reference_rejects_path_traversal(self):
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_dir = Path(tmp_dir)
+            repo = self._repo_with_initial_commit(base_dir)
+            outside_path = base_dir / "outside_write.txt"
+
+            self.assertRaises(ValueError, SymbolicReference(repo, "../../outside_write.txt").set_reference, "HEAD")
+            assert not outside_path.exists()
+
+    def test_symbolic_reference_rename_rejects_path_traversal(self):
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_dir = Path(tmp_dir)
+            repo = self._repo_with_initial_commit(base_dir)
+            outside_path = base_dir / "outside_move.txt"
+            ref = SymbolicReference.create(repo, "SAFE_RENAME_SOURCE", "HEAD")
+
+            self.assertRaises(ValueError, ref.rename, "../../outside_move.txt")
+            assert not outside_path.exists()
+            assert Path(ref.abspath).is_file()
+
+    def test_symbolic_reference_delete_rejects_path_traversal(self):
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_dir = Path(tmp_dir)
+            repo = self._repo_with_initial_commit(base_dir)
+            outside_path = base_dir / "outside_delete.txt"
+            outside_path.write_text("do not delete\n", encoding="utf-8")
+
+            self.assertRaises(ValueError, SymbolicReference.delete, repo, "../../outside_delete.txt")
+            assert outside_path.read_text(encoding="utf-8") == "do not delete\n"
+
+    def test_symbolic_reference_log_append_rejects_path_traversal(self):
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_dir = Path(tmp_dir)
+            repo = self._repo_with_initial_commit(base_dir)
+            outside_path = base_dir / "outside_reflog.txt"
+
+            ref = SymbolicReference(repo, "../../../outside_reflog.txt")
+            self.assertRaises(ValueError, ref.log_append, Commit.NULL_BIN_SHA, "do not write", repo.head.commit.binsha)
+            assert not outside_path.exists()
+
+    def test_remote_reference_delete_cleanup_rejects_path_traversal(self):
+        with tempfile.TemporaryDirectory() as tmp_dir:
+            base_dir = Path(tmp_dir)
+            git_dir = base_dir / "repo" / ".git"
+            git_dir.mkdir(parents=True)
+            outside_path = base_dir / "outside_remote_delete.txt"
+            outside_path.write_text("do not delete\n", encoding="utf-8")
+
+            class GitStub:
+                def branch(self, *args):
+                    pass
+
+            class RepoStub:
+                pass
+
+            repo = RepoStub()
+            repo.git = GitStub()
+            repo.common_dir = str(git_dir)
+            repo.git_dir = str(git_dir)
+            ref = RemoteReference(repo, "../../outside_remote_delete.txt", check_path=False)
+
+            self.assertRaises(ValueError, RemoteReference.delete, repo, ref)
+            assert outside_path.read_text(encoding="utf-8") == "do not delete\n"
+
     def test_validity_ref_names(self):
         """Ensure ref names are checked for validity.
 
