From 348876166192a404205e9b02448c0ed15ad156b0 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. 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..fd30fce45 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..c6a19b04c 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: