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.

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` (gitpython-developers#1516).

Ref gitpython-developers#1517
  • Loading branch information
stsewd committed Dec 21, 2022
1 parent 17ff263 commit fbf9c7e
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 fbf9c7e

Please sign in to comment.