Skip to content

Commit

Permalink
Forbid unsafe protocol URLs in Repo.clone_from()
Browse files Browse the repository at this point in the history
Since the URL is passed directly to git clone, and the remote-ext helper
will happily execute shell commands, so by default disallow URLs that
contain a "::" unless a new unsafe_protocols kwarg is passed.
(CVE-2022-24439)

Fixes gitpython-developers#1515
  • Loading branch information
s-t-e-v-e-n-k committed Dec 15, 2022
1 parent 17ff263 commit ea52206
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
4 changes: 4 additions & 0 deletions git/repo/base.py
Expand Up @@ -1253,6 +1253,7 @@ def clone_from(
progress: Optional[Callable] = None,
env: Optional[Mapping[str, str]] = None,
multi_options: Optional[List[str]] = None,
unsafe_protocols: bool = False,
**kwargs: Any,
) -> "Repo":
"""Create a clone from the given URL
Expand All @@ -1268,10 +1269,13 @@ def clone_from(
as its value.
:param multi_options: See ``clone`` method
:param kwargs: see the ``clone`` method
:param unsafe_protocols: Allow unsafe protocols to be used, like ext::
:return: Repo instance pointing to the cloned directory"""
git = cls.GitCommandWrapperType(os.getcwd())
if env is not None:
git.update_environment(**env)
if not unsafe_protocols and "::" in url:
raise ValueError(f"{url} requires unsafe_protocols flag")
return cls._clone(git, url, to_path, GitCmdObjectDB, progress, multi_options, **kwargs)

def archive(
Expand Down
20 changes: 20 additions & 0 deletions test/test_repo.py
Expand Up @@ -13,6 +13,7 @@
import pickle
import sys
import tempfile
import uuid
from unittest import mock, skipIf, SkipTest

import pytest
Expand Down Expand Up @@ -263,6 +264,25 @@ def test_leaking_password_in_clone_logs(self, rw_dir):
to_path=rw_dir,
)

def test_clone_from_forbids_helper_urls_by_default(self):
with self.assertRaises(ValueError):
Repo.clone_from("ext::sh -c touch% /tmp/foo", "tmp")

@with_rw_repo("HEAD")
def test_clone_from_allow_unsafe(self, repo):
bad_filename = pathlib.Path(f'{tempfile.gettempdir()}/{uuid.uuid4()}')
bad_url = f'ext::sh -c touch% {bad_filename}'
try:
repo.clone_from(
bad_url, 'tmp',
multi_options=["-c protocol.ext.allow=always"],
unsafe_protocols=True
)
except GitCommandError:
pass
self.assertTrue(bad_filename.is_file())
bad_filename.unlink()

@with_rw_repo("HEAD")
def test_max_chunk_size(self, repo):
class TestOutputStream(TestBase):
Expand Down

0 comments on commit ea52206

Please sign in to comment.