Merge branch 'fix-cmd-injection'

Author: Byron

Diff for: .github/workflows/pythonpackage.yml

@@ -18,7 +18,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python-version: [3.7, 3.7.5, 3.7.12, 3.8, 3.8.0, 3.8.11, 3.8, 3.9, 3.9.0, 3.9.7, "3.10"]
+        python-version: [3.7, 3.8, 3.9, "3.10", "3.11"]
 
     steps:
     - uses: actions/checkout@v3

Diff for: doc/source/changes.rst

@@ -2,6 +2,18 @@
 Changelog
 =========
 
+3.1.30
+======
+
+- Make injections of command-invocations harder or impossible for clone and others.
+  See https://github.com/gitpython-developers/GitPython/pull/1518 for details.
+  Note that this might constitute a breaking change for some users, and if so please
+  let us know and we add an opt-out to this.
+
+See the following for all changes.
+https://github.com/gitpython-developers/gitpython/milestone/60?closed=1
+
+
 3.1.29
 ======
 

Diff for: git/remote.py

@@ -964,7 +964,7 @@ def fetch(
             args = [refspec]
 
         proc = self.repo.git.fetch(
-            self, *args, as_process=True, with_stdout=False, universal_newlines=True, v=verbose, **kwargs
+            "--", self, *args, as_process=True, with_stdout=False, universal_newlines=True, v=verbose, **kwargs
         )
         res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout)
         if hasattr(self.repo.odb, "update_cache"):
@@ -991,7 +991,7 @@ def pull(
             self._assert_refspec()
         kwargs = add_progress(kwargs, self.repo.git, progress)
         proc = self.repo.git.pull(
-            self, refspec, with_stdout=False, as_process=True, universal_newlines=True, v=True, **kwargs
+            "--", self, refspec, with_stdout=False, as_process=True, universal_newlines=True, v=True, **kwargs
         )
         res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout)
         if hasattr(self.repo.odb, "update_cache"):
@@ -1034,6 +1034,7 @@ def push(
             be 0."""
         kwargs = add_progress(kwargs, self.repo.git, progress)
         proc = self.repo.git.push(
+            "--",
             self,
             refspec,
             porcelain=True,

Diff for: git/repo/base.py

@@ -1169,6 +1169,7 @@ def _clone(
             multi = shlex.split(" ".join(multi_options))
         proc = git.clone(
             multi,
+            "--",
             Git.polish_url(str(url)),
             clone_path,
             with_extended_output=True,
@@ -1305,7 +1306,7 @@ def archive(
         if not isinstance(path, (tuple, list)):
             path = [path]
         # end assure paths is list
-        self.git.archive(treeish, *path, **kwargs)
+        self.git.archive("--", treeish, *path, **kwargs)
         return self
 
     def has_separate_working_tree(self) -> bool:

Diff for: test/test_repo.py

@@ -1180,3 +1180,29 @@ def test_do_not_strip_newline_in_stdout(self, rw_dir):
         r.git.add(Git.polish_url(fp))
         r.git.commit(message="init")
         self.assertEqual(r.git.show("HEAD:hello.txt", strip_newline_in_stdout=False), "hello\n")
+
+    @with_rw_repo("HEAD")
+    def test_clone_command_injection(self, rw_repo):
+        tmp_dir = pathlib.Path(tempfile.mkdtemp())
+        unexpected_file = tmp_dir / "pwn"
+        assert not unexpected_file.exists()
+
+        payload = f"--upload-pack=touch {unexpected_file}"
+        rw_repo.clone(payload)
+
+        assert not unexpected_file.exists()
+        # A repo was cloned with the payload as name
+        assert pathlib.Path(payload).exists()
+
+    @with_rw_repo("HEAD")
+    def test_clone_from_command_injection(self, rw_repo):
+        tmp_dir = pathlib.Path(tempfile.mkdtemp())
+        temp_repo = Repo.init(tmp_dir / "repo")
+        unexpected_file = tmp_dir / "pwn"
+
+        assert not unexpected_file.exists()
+        payload = f"--upload-pack=touch {unexpected_file}"
+        with self.assertRaises(GitCommandError):
+            rw_repo.clone_from(payload, temp_repo.common_dir)
+
+        assert not unexpected_file.exists()