Skip to content

Commit

Permalink
Fix command injection
Browse files Browse the repository at this point in the history
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` (gitpython-developers#1516).

Ref gitpython-developers#1517
  • Loading branch information
stsewd committed Dec 21, 2022
1 parent 17ff263 commit 3488761
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
5 changes: 3 additions & 2 deletions git/remote.py
Expand Up @@ -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"):
Expand All @@ -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"):
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion git/repo/base.py
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 3488761

Please sign in to comment.