From fbf9c7e72218e44bc29eb4907d5c00118370376b Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 20 Dec 2022 20:26:37 -0500 Subject: [PATCH] Fix command injection Add `--` in some commands that receive user input and if interpreted as options could lead to remote code execution (RCE). There may be more commands that could benefit from `--` so the input is never interpreted as an option, but most of those aren't dangerous. Fixed commands: - push - pull - fetch - clone/clone_from and friends - archive (not sure if this one can be exploited, but it doesn't hurt adding `--` :)) For anyone using GitPython and exposing any of the GitPython methods to users, make sure to always validate the input (like if starts with `--`). And for anyone allowing users to pass arbitrary options, be aware that some options may lead fo RCE, like `--exc`, `--upload-pack`, `--receive-pack`, `--config` (https://github.com/gitpython-developers/GitPython/pull/1516). Ref https://github.com/gitpython-developers/GitPython/issues/1517 --- git/remote.py | 5 +++-- git/repo/base.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/git/remote.py b/git/remote.py index 7b44020c5..483d536ae 100644 --- a/git/remote.py +++ b/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 --git a/git/repo/base.py b/git/repo/base.py index c49c61184..49a3d5a16 100644 --- a/git/repo/base.py +++ b/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: