Merge pull request #2225 from gitpython-developers/submodule-path-hardening

Reject unsafe submodule checkout paths

Author: Byron

doc/source/changes.rst

@@ -2,6 +2,19 @@
 Changelog
 =========
 
+3.1.62
+======
+
+Security fixes for
+
+* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-59cr-6r3x-644w
+
+If you can, also try and provide feedback on the upcoming v4 branch
+https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome.
+
+See the following for all changes.
+https://github.com/gitpython-developers/GitPython/releases/tag/3.1.62
+
 3.1.61
 ======
 

git/objects/submodule/base.py

@@ -414,6 +414,18 @@ def _to_relative_path(cls, parent_repo: "Repo", path: PathLike) -> PathLike:
 
         return path
 
+    @property
+    def abspath(self) -> PathLike:
+        root = self.repo.working_tree_dir
+        if root is None:
+            return super().abspath
+        path = root
+        for component in os.fspath(self._to_relative_path(self.repo, self.path)).split("/"):
+            path = join_path_native(path, component)
+            if osp.islink(path):
+                raise ValueError("Submodule checkout path %r contains a symbolic link" % self.path)
+        return path
+
     @classmethod
     def _write_git_file_and_module_config(cls, working_tree_dir: PathLike, module_abspath: PathLike) -> None:
         """Write a ``.git`` file containing a (preferably) relative path to the actual

test/test_submodule.py

@@ -1369,6 +1369,46 @@ class Repo:
             osp.join(Repo.working_tree_dir + "-other", "module"),
         )
 
+    @with_rw_directory
+    def test_update_rejects_checkout_path_outside_parent(self, rwdir):
+        parent = git.Repo.init(osp.join(rwdir, "parent"))
+        submodule = Submodule(
+            parent,
+            Submodule.NULL_BIN_SHA,
+            name="module",
+            path=osp.join("..", "outside"),
+            url="unused",
+        )
+
+        with mock.patch.object(Submodule, "_clone_repo", side_effect=AssertionError("clone attempted")):
+            with pytest.raises(ValueError, match="is not in repository"):
+                submodule.update(init=True)
+
+    @with_rw_directory
+    def test_update_rejects_checkout_path_through_symlink(self, rwdir):
+        parent = git.Repo.init(osp.join(rwdir, "parent"))
+        os.mkdir(osp.join(parent.working_tree_dir, "target"))
+        os.symlink("target", osp.join(parent.working_tree_dir, "link"))
+        submodule = Submodule(
+            parent,
+            Submodule.NULL_BIN_SHA,
+            name="module",
+            path=osp.join("link", "module"),
+            url="unused",
+        )
+
+        with mock.patch.object(Submodule, "_clone_repo", side_effect=AssertionError("clone attempted")):
+            with pytest.raises(ValueError, match="contains a symbolic link"):
+                submodule.update(init=True)
+
+    @with_rw_directory
+    def test_update_rejects_checkout_path_at_parent_root(self, rwdir):
+        parent = git.Repo.init(osp.join(rwdir, "parent"))
+        submodule = Submodule(parent, Submodule.NULL_BIN_SHA, name="module", path=".", url="unused")
+
+        with pytest.raises(ValueError, match="must not be the repository root"):
+            submodule.update(init=True)
+
     @skipUnless(sys.platform == "win32", "Specifically for Windows.")
     @with_rw_directory
     def test_to_relative_path_windows_path_kinds(self, rwdir):