Skip to content

Commit b88d913

Browse files
codexByron
authored andcommitted
Validate submodule names before filesystem operations
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. Add local-repository regression coverage for both new initialization and an existing separate Git directory outside the clone. This follows Git commit 0383bbb901 (submodule-config: verify submodule names as paths) while also accounting for os.path.join absolute-path semantics. Advisory: GHSA-hmq2-w58f-27jc Validation: - pytest -q test/test_submodule.py on Python 3.14.6 (39 passed, 1 skipped, 1 xfailed) - ruff check and format --check on changed files - mypy git/objects/submodule/base.py
1 parent 6e61b1d commit b88d913

2 files changed

Lines changed: 67 additions & 1 deletion

File tree

git/objects/submodule/base.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import gc
77
from io import BytesIO
88
import logging
9+
import ntpath
910
import os
1011
import os.path as osp
1112
import stat
@@ -302,10 +303,21 @@ def _config_parser_constrained(self, read_only: bool) -> SectionConstraint:
302303
parser.set_submodule(self)
303304
return SectionConstraint(parser, sm_section(self.name))
304305

306+
@classmethod
307+
def _validated_name(cls, name: str) -> str:
308+
if (
309+
not name
310+
or name.startswith(("/", "\\"))
311+
or ntpath.splitdrive(name)[0]
312+
or ".." in name.replace("\\", "/").split("/")
313+
):
314+
raise ValueError("Invalid submodule name %r" % name)
315+
return name
316+
305317
@classmethod
306318
def _module_abspath(cls, parent_repo: "Repo", path: PathLike, name: str) -> PathLike:
307319
if cls._need_gitfile_submodules(parent_repo.git):
308-
return osp.join(parent_repo.git_dir, "modules", name)
320+
return osp.join(parent_repo.git_dir, "modules", cls._validated_name(name))
309321
if parent_repo.working_tree_dir:
310322
return osp.join(parent_repo.working_tree_dir, path)
311323
raise NotADirectoryError()
@@ -523,6 +535,7 @@ def add(
523535
raise InvalidGitRepositoryError("Cannot add submodules to bare repositories")
524536
# END handle bare repos
525537

538+
cls._validated_name(name)
526539
path = cls._to_relative_path(repo, path)
527540

528541
# Ensure we never put backslashes into the URL, as might happen on Windows.
@@ -726,6 +739,7 @@ def update(
726739
return self
727740
# END pass in bare mode
728741

742+
self._validated_name(self.name)
729743
if progress is None:
730744
progress = UpdateProgress()
731745
# END handle progress
@@ -1020,6 +1034,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool =
10201034
raise ValueError("You must specify to move at least the module or the configuration of the submodule")
10211035
# END handle input
10221036

1037+
self._validated_name(self.name)
10231038
module_checkout_path = self._to_relative_path(self.repo, module_path)
10241039

10251040
# VERIFY DESTINATION
@@ -1160,6 +1175,7 @@ def remove(
11601175
raise ValueError("Need to specify to delete at least the module, or the configuration")
11611176
# END handle parameters
11621177

1178+
self._validated_name(self.name)
11631179
# Recursively remove children of this submodule.
11641180
nc = 0
11651181
for csm in self.children():
@@ -1416,6 +1432,9 @@ def rename(self, new_name: str) -> "Submodule":
14161432
if self.name == new_name:
14171433
return self
14181434

1435+
self._validated_name(self.name)
1436+
self._validated_name(new_name)
1437+
14191438
# .git/config
14201439
with self.repo.config_writer() as pw:
14211440
# As we ourselves didn't write anything about submodules into the parent
@@ -1463,6 +1482,7 @@ def module(self) -> "Repo":
14631482
If a repository was not available.
14641483
This could also mean that it was not yet initialized.
14651484
"""
1485+
self._validated_name(self.name)
14661486
module_checkout_abspath = self.abspath
14671487
try:
14681488
repo = git.Repo(module_checkout_abspath)

test/test_submodule.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -925,6 +925,52 @@ def test_update_submodule_with_relative_path(self, rwdir):
925925

926926
cloned_repo.submodule_update(init=True, recursive=True)
927927

928+
@with_rw_directory
929+
@_patch_git_config("protocol.file.allow", "always")
930+
def test_update_rejects_parent_component_in_name(self, rwdir):
931+
source = git.Repo.init(osp.join(rwdir, "source"))
932+
source.git.commit(m="initial commit", allow_empty=True)
933+
934+
parent = git.Repo.init(osp.join(rwdir, "parent"))
935+
parent.git.submodule("add", source.working_tree_dir, "module")
936+
parent.index.commit("add submodule")
937+
modules_file = Path(parent.working_tree_dir) / ".gitmodules"
938+
modules_file.write_text(
939+
modules_file.read_text().replace('submodule "module"', 'submodule "../../../escaped/module"')
940+
)
941+
parent.index.add([".gitmodules"])
942+
parent.index.commit("change submodule name")
943+
944+
clone = git.Repo.clone_from(parent.working_tree_dir, osp.join(rwdir, "clone"))
945+
with pytest.raises(ValueError, match="submodule name"):
946+
clone.submodules[0].update(init=True)
947+
assert not osp.exists(osp.join(rwdir, "escaped"))
948+
949+
Path(rwdir, "escaped").mkdir()
950+
git.Repo.clone_from(
951+
source.working_tree_dir,
952+
osp.join(clone.working_tree_dir, "module"),
953+
separate_git_dir=osp.join(rwdir, "escaped", "module"),
954+
)
955+
with pytest.raises(ValueError, match="submodule name"):
956+
clone.submodules[0].update(init=True)
957+
958+
invalid_names = (
959+
"",
960+
"..",
961+
"../module",
962+
R"..\module",
963+
"nested/../module",
964+
R"nested\..\module",
965+
"/module",
966+
R"\module",
967+
"C:module",
968+
R"C:\module",
969+
)
970+
for name in invalid_names:
971+
with pytest.raises(ValueError, match="submodule name"):
972+
Submodule._module_abspath(clone, "module", name)
973+
928974
@with_rw_directory
929975
@_patch_git_config("protocol.file.allow", "always")
930976
def test_list_only_valid_submodules(self, rwdir):

0 commit comments

Comments
 (0)