From 2334ee57741648024b5688e8bd7b77ad4cb2306e Mon Sep 17 00:00:00 2001
From: Steve Kowalik <steven@wedontsleep.org>
Date: Thu, 15 Dec 2022 13:23:09 +1100
Subject: [PATCH] Forbid unsafe protocol URLs in Repo.clone_from()

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 #1515
---
 git/repo/base.py  |  4 ++++
 test/test_repo.py | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/git/repo/base.py b/git/repo/base.py
index c49c61184..42ea2b637 100644
--- a/git/repo/base.py
+++ b/git/repo/base.py
@@ -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
@@ -1267,11 +1268,14 @@ def clone_from(
             If you want to unset some variable, consider providing empty string
             as its value.
         :param multi_options: See ``clone`` method
+        :param unsafe_protocols: Allow unsafe protocols to be used, like ext
         :param kwargs: see the ``clone`` method
         :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(
diff --git a/test/test_repo.py b/test/test_repo.py
index 703dbb438..1f5435478 100644
--- a/test/test_repo.py
+++ b/test/test_repo.py
@@ -13,6 +13,7 @@
 import pickle
 import sys
 import tempfile
+import uuid
 from unittest import mock, skipIf, SkipTest
 
 import pytest
@@ -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):