Validate submodule names before filesystem operations

<!-- agent -->
Submodule names read from .gitmodules can influence the separate Git directory
path. Reject empty names, absolute or drive-qualified names, and parent
components with either path separator.

Validate before constructing module paths and before opening or mutating
existing checkouts, so a repository initialized by an older vulnerable version
cannot bypass the guard. Validate programmatic add and rename inputs before
making changes as well.

Advisory: GHSA-hmq2-w58f-27jc

Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <codex@openai.com>

Author: Byron

git/objects/submodule/base.py

@@ -6,6 +6,7 @@
 import gc
 from io import BytesIO
 import logging
+import ntpath
 import os
 import os.path as osp
 import stat
@@ -302,10 +303,21 @@ def _config_parser_constrained(self, read_only: bool) -> SectionConstraint:
         parser.set_submodule(self)
         return SectionConstraint(parser, sm_section(self.name))
 
+    @classmethod
+    def _validated_name(cls, name: str) -> str:
+        if (
+            not name
+            or name.startswith(("/", "\\"))
+            or ntpath.splitdrive(name)[0]
+            or ".." in name.replace("\\", "/").split("/")
+        ):
+            raise ValueError("Invalid submodule name %r" % name)
+        return name
+
     @classmethod
     def _module_abspath(cls, parent_repo: "Repo", path: PathLike, name: str) -> PathLike:
         if cls._need_gitfile_submodules(parent_repo.git):
-            return osp.join(parent_repo.git_dir, "modules", name)
+            return osp.join(parent_repo.git_dir, "modules", cls._validated_name(name))
         if parent_repo.working_tree_dir:
             return osp.join(parent_repo.working_tree_dir, path)
         raise NotADirectoryError()
@@ -523,6 +535,7 @@ def add(
             raise InvalidGitRepositoryError("Cannot add submodules to bare repositories")
         # END handle bare repos
 
+        cls._validated_name(name)
         path = cls._to_relative_path(repo, path)
 
         # Ensure we never put backslashes into the URL, as might happen on Windows.
@@ -771,6 +784,8 @@ def fetch_remotes(module_repo: "Repo") -> None:
             # END fetch new data
 
         try:
+            self._validated_name(self.name)
+
             # ENSURE REPO IS PRESENT AND UP-TO-DATE
             #######################################
             try:
@@ -1020,6 +1035,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool =
             raise ValueError("You must specify to move at least the module or the configuration of the submodule")
         # END handle input
 
+        self._validated_name(self.name)
         module_checkout_path = self._to_relative_path(self.repo, module_path)
 
         # VERIFY DESTINATION
@@ -1160,6 +1176,7 @@ def remove(
             raise ValueError("Need to specify to delete at least the module, or the configuration")
         # END handle parameters
 
+        self._validated_name(self.name)
         # Recursively remove children of this submodule.
         nc = 0
         for csm in self.children():
@@ -1416,6 +1433,9 @@ def rename(self, new_name: str) -> "Submodule":
         if self.name == new_name:
             return self
 
+        self._validated_name(self.name)
+        self._validated_name(new_name)
+
         # .git/config
         with self.repo.config_writer() as pw:
             # As we ourselves didn't write anything about submodules into the parent
@@ -1463,6 +1483,7 @@ def module(self) -> "Repo":
             If a repository was not available.
             This could also mean that it was not yet initialized.
         """
+        self._validated_name(self.name)
         module_checkout_abspath = self.abspath
         try:
             repo = git.Repo(module_checkout_abspath)

test/test_submodule.py

@@ -925,6 +925,75 @@ def test_update_submodule_with_relative_path(self, rwdir):
 
         cloned_repo.submodule_update(init=True, recursive=True)
 
+    @with_rw_directory
+    @_patch_git_config("protocol.file.allow", "always")
+    def test_update_rejects_parent_component_in_name(self, rwdir):
+        source = git.Repo.init(osp.join(rwdir, "source"))
+        source.git.commit(m="initial commit", allow_empty=True)
+
+        parent = git.Repo.init(osp.join(rwdir, "parent"))
+        parent.git.submodule("add", source.working_tree_dir, "module")
+        parent.index.commit("add submodule")
+        modules_file = Path(parent.working_tree_dir) / ".gitmodules"
+        modules_file.write_text(
+            modules_file.read_text().replace('submodule "module"', 'submodule "../../../escaped/module"')
+        )
+        parent.index.add([".gitmodules"])
+        parent.index.commit("change submodule name")
+
+        clone = git.Repo.clone_from(parent.working_tree_dir, osp.join(rwdir, "clone"))
+        with pytest.raises(ValueError, match="submodule name"):
+            clone.submodules[0].update(init=True)
+        assert not osp.exists(osp.join(rwdir, "escaped"))
+
+        Path(rwdir, "escaped").mkdir()
+        git.Repo.clone_from(
+            source.working_tree_dir,
+            osp.join(clone.working_tree_dir, "module"),
+            separate_git_dir=osp.join(rwdir, "escaped", "module"),
+        )
+        with pytest.raises(ValueError, match="submodule name"):
+            clone.submodules[0].update(init=True)
+
+        invalid_names = (
+            "",
+            "..",
+            "../module",
+            R"..\module",
+            "nested/../module",
+            R"nested\..\module",
+            "/module",
+            R"\module",
+            "C:module",
+            R"C:\module",
+        )
+        for name in invalid_names:
+            with pytest.raises(ValueError, match="submodule name"):
+                Submodule._module_abspath(clone, "module", name)
+
+    @with_rw_directory
+    @_patch_git_config("protocol.file.allow", "always")
+    def test_root_update_keeps_going_after_invalid_submodule_name(self, rwdir):
+        source = git.Repo.init(osp.join(rwdir, "source"))
+        source.git.commit(m="initial commit", allow_empty=True)
+
+        parent = git.Repo.init(osp.join(rwdir, "parent"))
+        parent.git.submodule("add", source.working_tree_dir, "invalid")
+        parent.git.submodule("add", source.working_tree_dir, "valid")
+        modules_file = Path(parent.working_tree_dir) / ".gitmodules"
+        modules_file.write_text(modules_file.read_text().replace('submodule "invalid"', 'submodule "../invalid"'))
+        parent.index.add([".gitmodules"])
+        parent.index.commit("add submodules")
+
+        clone = git.Repo.clone_from(parent.working_tree_dir, osp.join(rwdir, "clone"))
+        assert [sm.name for sm in clone.submodules] == ["../invalid", "valid"]
+
+        clone.submodule_update(keep_going=True)
+
+        assert os.listdir(osp.join(clone.working_tree_dir, "invalid")) == []
+        assert not clone.submodule("../invalid").module_exists()
+        assert clone.submodule("valid").module_exists()
+
     @with_rw_directory
     @_patch_git_config("protocol.file.allow", "always")
     def test_list_only_valid_submodules(self, rwdir):