1From fe9b71628767610a238e47cd46b82d411a7e871a Mon Sep 17 00:00:00 2001
2From: Narpat Mali <narpat.mali@windriver.com>
3Date: Sat, 7 Jan 2023 17:16:57 +0000
4Subject: [PATCH] python3-git: CVE-2022-24439 fix from PR 1521
5
6Forbid unsafe protocol URLs in Repo.clone{,_from}()
7Since the URL is passed directly to git clone, and the remote-ext helper
8will happily execute shell commands, so by default disallow URLs that
9contain a "::" unless a new unsafe_protocols kwarg is passed.
10(CVE-2022-24439)
11
12Fixes #1515
13
14CVE: CVE-2022-24439
15
16Upstream-Status: Backport [https://github.com/gitpython-developers/GitPython/pull/1521]
17
18Signed-off-by: Narpat Mali <narpat.mali@windriver.com>
19---
20 git/cmd.py                    | 51 ++++++++++++++++++++++++--
21 git/exc.py                    |  8 ++++
22 git/objects/submodule/base.py | 19 ++++++----
23 git/remote.py                 | 69 +++++++++++++++++++++++++++++++----
24 git/repo/base.py              | 44 ++++++++++++++++++----
25 5 files changed, 166 insertions(+), 25 deletions(-)
26
27diff --git a/git/cmd.py b/git/cmd.py
28index 4f05698..77026d6 100644
29--- a/git/cmd.py
30+++ b/git/cmd.py
31@@ -4,6 +4,7 @@
32 # This module is part of GitPython and is released under
33 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
34 from __future__ import annotations
35+import re
36 from contextlib import contextmanager
37 import io
38 import logging
39@@ -31,7 +32,9 @@ from git.util import is_cygwin_git, cygpath, expand_path, remove_password_if_pre
40
41 from .exc import (
42     GitCommandError,
43-    GitCommandNotFound
44+    GitCommandNotFound,
45+    UnsafeOptionError,
46+    UnsafeProtocolError
47 )
48 from .util import (
49     LazyMixin,
50@@ -225,6 +228,8 @@ class Git(LazyMixin):
51
52     _excluded_ = ('cat_file_all', 'cat_file_header', '_version_info')
53
54+    re_unsafe_protocol = re.compile("(.+)::.+")
55+
56     def __getstate__(self) -> Dict[str, Any]:
57         return slots_to_dict(self, exclude=self._excluded_)
58
59@@ -400,6 +405,44 @@ class Git(LazyMixin):
60             url = url.replace("\\\\", "\\").replace("\\", "/")
61         return url
62
63+    @classmethod
64+    def check_unsafe_protocols(cls, url: str) -> None:
65+        """
66+        Check for unsafe protocols.
67+        Apart from the usual protocols (http, git, ssh),
68+        Git allows "remote helpers" that have the form `<transport>::<address>`,
69+        one of these helpers (`ext::`) can be used to invoke any arbitrary command.
70+        See:
71+        - https://git-scm.com/docs/gitremote-helpers
72+        - https://git-scm.com/docs/git-remote-ext
73+        """
74+        match = cls.re_unsafe_protocol.match(url)
75+        if match:
76+            protocol = match.group(1)
77+            raise UnsafeProtocolError(
78+                f"The `{protocol}::` protocol looks suspicious, use `allow_unsafe_protocols=True` to allow it."
79+            )
80+
81+    @classmethod
82+    def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> None:
83+        """
84+        Check for unsafe options.
85+        Some options that are passed to `git <command>` can be used to execute
86+        arbitrary commands, this are blocked by default.
87+        """
88+        # Options can be of the form `foo` or `--foo bar` `--foo=bar`,
89+        # so we need to check if they start with "--foo" or if they are equal to "foo".
90+        bare_unsafe_options = [
91+            option.lstrip("-")
92+            for option in unsafe_options
93+        ]
94+        for option in options:
95+            for unsafe_option, bare_option in zip(unsafe_options, bare_unsafe_options):
96+                if option.startswith(unsafe_option) or option == bare_option:
97+                    raise UnsafeOptionError(
98+                        f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it."
99+                    )
100+
101     class AutoInterrupt(object):
102         """Kill/Interrupt the stored process instance once this instance goes out of scope. It is
103         used to prevent processes piling up in case iterators stop reading.
104@@ -1068,12 +1111,12 @@ class Git(LazyMixin):
105         return args
106
107     @classmethod
108-    def __unpack_args(cls, arg_list: Sequence[str]) -> List[str]:
109+    def _unpack_args(cls, arg_list: Sequence[str]) -> List[str]:
110
111         outlist = []
112         if isinstance(arg_list, (list, tuple)):
113             for arg in arg_list:
114-                outlist.extend(cls.__unpack_args(arg))
115+                outlist.extend(cls._unpack_args(arg))
116         else:
117             outlist.append(str(arg_list))
118
119@@ -1154,7 +1197,7 @@ class Git(LazyMixin):
120         # Prepare the argument list
121
122         opt_args = self.transform_kwargs(**opts_kwargs)
123-        ext_args = self.__unpack_args([a for a in args if a is not None])
124+        ext_args = self._unpack_args([a for a in args if a is not None])
125
126         if insert_after_this_arg is None:
127             args_list = opt_args + ext_args
128diff --git a/git/exc.py b/git/exc.py
129index e8ff784..5c96db2 100644
130--- a/git/exc.py
131+++ b/git/exc.py
132@@ -36,6 +36,14 @@ class NoSuchPathError(GitError, OSError):
133     """ Thrown if a path could not be access by the system. """
134
135
136+class UnsafeProtocolError(GitError):
137+    """Thrown if unsafe protocols are passed without being explicitly allowed."""
138+
139+
140+class UnsafeOptionError(GitError):
141+    """Thrown if unsafe options are passed without being explicitly allowed."""
142+
143+
144 class CommandError(GitError):
145     """Base class for exceptions thrown at every stage of `Popen()` execution.
146
147diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py
148index f782045..deb224e 100644
149--- a/git/objects/submodule/base.py
150+++ b/git/objects/submodule/base.py
151@@ -264,7 +264,8 @@ class Submodule(IndexObject, TraversableIterableObj):
152         # end
153
154     @classmethod
155-    def _clone_repo(cls, repo: 'Repo', url: str, path: PathLike, name: str, **kwargs: Any) -> 'Repo':
156+    def _clone_repo(cls, repo: 'Repo', url: str, path: PathLike, name: str,
157+            allow_unsafe_options: bool = False, allow_unsafe_protocols: bool = False,**kwargs: Any) -> 'Repo':
158         """:return: Repo instance of newly cloned repository
159         :param repo: our parent repository
160         :param url: url to clone from
161@@ -281,7 +282,8 @@ class Submodule(IndexObject, TraversableIterableObj):
162             module_checkout_path = osp.join(str(repo.working_tree_dir), path)
163         # end
164
165-        clone = git.Repo.clone_from(url, module_checkout_path, **kwargs)
166+        clone = git.Repo.clone_from(url, module_checkout_path, allow_unsafe_options=allow_unsafe_options,
167+                allow_unsafe_protocols=allow_unsafe_protocols, **kwargs)
168         if cls._need_gitfile_submodules(repo.git):
169             cls._write_git_file_and_module_config(module_checkout_path, module_abspath)
170         # end
171@@ -338,8 +340,8 @@ class Submodule(IndexObject, TraversableIterableObj):
172     @classmethod
173     def add(cls, repo: 'Repo', name: str, path: PathLike, url: Union[str, None] = None,
174             branch: Union[str, None] = None, no_checkout: bool = False, depth: Union[int, None] = None,
175-            env: Union[Mapping[str, str], None] = None, clone_multi_options: Union[Sequence[TBD], None] = None
176-            ) -> 'Submodule':
177+            env: Union[Mapping[str, str], None] = None, clone_multi_options: Union[Sequence[TBD], None] = None,
178+            allow_unsafe_options: bool = False, allow_unsafe_protocols: bool = False,) -> 'Submodule':
179         """Add a new submodule to the given repository. This will alter the index
180         as well as the .gitmodules file, but will not create a new commit.
181         If the submodule already exists, no matter if the configuration differs
182@@ -447,7 +449,8 @@ class Submodule(IndexObject, TraversableIterableObj):
183                 kwargs['multi_options'] = clone_multi_options
184
185             # _clone_repo(cls, repo, url, path, name, **kwargs):
186-            mrepo = cls._clone_repo(repo, url, path, name, env=env, **kwargs)
187+            mrepo = cls._clone_repo(repo, url, path, name, env=env, allow_unsafe_options=allow_unsafe_options,
188+                allow_unsafe_protocols=allow_unsafe_protocols, **kwargs)
189         # END verify url
190
191         ## See #525 for ensuring git urls in config-files valid under Windows.
192@@ -484,7 +487,8 @@ class Submodule(IndexObject, TraversableIterableObj):
193     def update(self, recursive: bool = False, init: bool = True, to_latest_revision: bool = False,
194                progress: Union['UpdateProgress', None] = None, dry_run: bool = False,
195                force: bool = False, keep_going: bool = False, env: Union[Mapping[str, str], None] = None,
196-               clone_multi_options: Union[Sequence[TBD], None] = None) -> 'Submodule':
197+               clone_multi_options: Union[Sequence[TBD], None] = None, allow_unsafe_options: bool = False,
198+               allow_unsafe_protocols: bool = False) -> 'Submodule':
199         """Update the repository of this submodule to point to the checkout
200         we point at with the binsha of this instance.
201
202@@ -585,7 +589,8 @@ class Submodule(IndexObject, TraversableIterableObj):
203                                 (self.url, checkout_module_abspath, self.name))
204                 if not dry_run:
205                     mrepo = self._clone_repo(self.repo, self.url, self.path, self.name, n=True, env=env,
206-                                             multi_options=clone_multi_options)
207+                            multi_options=clone_multi_options, allow_unsafe_options=allow_unsafe_options,
208+                            allow_unsafe_protocols=allow_unsafe_protocols)
209                 # END handle dry-run
210                 progress.update(END | CLONE, 0, 1, prefix + "Done cloning to %s" % checkout_module_abspath)
211
212diff --git a/git/remote.py b/git/remote.py
213index 59681bc..cea6b99 100644
214--- a/git/remote.py
215+++ b/git/remote.py
216@@ -473,6 +473,23 @@ class Remote(LazyMixin, IterableObj):
217     __slots__ = ("repo", "name", "_config_reader")
218     _id_attribute_ = "name"
219
220+    unsafe_git_fetch_options = [
221+        # This option allows users to execute arbitrary commands.
222+        # https://git-scm.com/docs/git-fetch#Documentation/git-fetch.txt---upload-packltupload-packgt
223+        "--upload-pack",
224+    ]
225+    unsafe_git_pull_options = [
226+        # This option allows users to execute arbitrary commands.
227+        # https://git-scm.com/docs/git-pull#Documentation/git-pull.txt---upload-packltupload-packgt
228+        "--upload-pack"
229+    ]
230+    unsafe_git_push_options = [
231+        # This option allows users to execute arbitrary commands.
232+        # https://git-scm.com/docs/git-push#Documentation/git-push.txt---execltgit-receive-packgt
233+        "--receive-pack",
234+        "--exec",
235+    ]
236+
237     def __init__(self, repo: 'Repo', name: str) -> None:
238         """Initialize a remote instance
239
240@@ -549,7 +566,8 @@ class Remote(LazyMixin, IterableObj):
241             yield Remote(repo, section[lbound + 1:rbound])
242         # END for each configuration section
243
244-    def set_url(self, new_url: str, old_url: Optional[str] = None, **kwargs: Any) -> 'Remote':
245+    def set_url(self, new_url: str, old_url: Optional[str] = None,
246+            allow_unsafe_protocols: bool = False, **kwargs: Any) -> 'Remote':
247         """Configure URLs on current remote (cf command git remote set_url)
248
249         This command manages URLs on the remote.
250@@ -558,15 +576,17 @@ class Remote(LazyMixin, IterableObj):
251         :param old_url: when set, replaces this URL with new_url for the remote
252         :return: self
253         """
254+        if not allow_unsafe_protocols:
255+            Git.check_unsafe_protocols(new_url)
256         scmd = 'set-url'
257         kwargs['insert_kwargs_after'] = scmd
258         if old_url:
259-            self.repo.git.remote(scmd, self.name, new_url, old_url, **kwargs)
260+            self.repo.git.remote(scmd, "--", self.name, new_url, old_url, **kwargs)
261         else:
262-            self.repo.git.remote(scmd, self.name, new_url, **kwargs)
263+            self.repo.git.remote(scmd, "--", self.name, new_url, **kwargs)
264         return self
265
266-    def add_url(self, url: str, **kwargs: Any) -> 'Remote':
267+    def add_url(self, url: str, allow_unsafe_protocols: bool = False, **kwargs: Any) -> 'Remote':
268         """Adds a new url on current remote (special case of git remote set_url)
269
270         This command adds new URLs to a given remote, making it possible to have
271@@ -575,7 +595,7 @@ class Remote(LazyMixin, IterableObj):
272         :param url: string being the URL to add as an extra remote URL
273         :return: self
274         """
275-        return self.set_url(url, add=True)
276+        return self.set_url(url, add=True, allow_unsafe_protocols=allow_unsafe_protocols)
277
278     def delete_url(self, url: str, **kwargs: Any) -> 'Remote':
279         """Deletes a new url on current remote (special case of git remote set_url)
280@@ -667,7 +687,7 @@ class Remote(LazyMixin, IterableObj):
281         return out_refs
282
283     @ classmethod
284-    def create(cls, repo: 'Repo', name: str, url: str, **kwargs: Any) -> 'Remote':
285+    def create(cls, repo: 'Repo', name: str, url: str, allow_unsafe_protocols: bool = False, *kwargs: Any) -> 'Remote':
286         """Create a new remote to the given repository
287         :param repo: Repository instance that is to receive the new remote
288         :param name: Desired name of the remote
289@@ -677,7 +697,10 @@ class Remote(LazyMixin, IterableObj):
290         :raise GitCommandError: in case an origin with that name already exists"""
291         scmd = 'add'
292         kwargs['insert_kwargs_after'] = scmd
293-        repo.git.remote(scmd, name, Git.polish_url(url), **kwargs)
294+        url = Git.polish_url(url)
295+        if not allow_unsafe_protocols:
296+            Git.check_unsafe_protocols(url)
297+        repo.git.remote(scmd, "--", name, url, **kwargs)
298         return cls(repo, name)
299
300     # add is an alias
301@@ -840,6 +863,8 @@ class Remote(LazyMixin, IterableObj):
302               progress: Union[RemoteProgress, None, 'UpdateProgress'] = None,
303               verbose: bool = True,
304               kill_after_timeout: Union[None, float] = None,
305+              allow_unsafe_protocols: bool = False,
306+              allow_unsafe_options: bool = False,
307               **kwargs: Any) -> IterableList[FetchInfo]:
308         """Fetch the latest changes for this remote
309
310@@ -881,6 +906,14 @@ class Remote(LazyMixin, IterableObj):
311         else:
312             args = [refspec]
313
314+        if not allow_unsafe_protocols:
315+            for ref in args:
316+                if ref:
317+                    Git.check_unsafe_protocols(ref)
318+
319+        if not allow_unsafe_options:
320+            Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_fetch_options)
321+
322         proc = self.repo.git.fetch("--", self, *args, as_process=True, with_stdout=False,
323                                    universal_newlines=True, v=verbose, **kwargs)
324         res = self._get_fetch_info_from_stderr(proc, progress,
325@@ -892,6 +925,8 @@ class Remote(LazyMixin, IterableObj):
326     def pull(self, refspec: Union[str, List[str], None] = None,
327              progress: Union[RemoteProgress, 'UpdateProgress', None] = None,
328              kill_after_timeout: Union[None, float] = None,
329+             allow_unsafe_protocols: bool = False,
330+             allow_unsafe_options: bool = False,
331              **kwargs: Any) -> IterableList[FetchInfo]:
332         """Pull changes from the given branch, being the same as a fetch followed
333         by a merge of branch with your local branch.
334@@ -905,6 +940,15 @@ class Remote(LazyMixin, IterableObj):
335             # No argument refspec, then ensure the repo's config has a fetch refspec.
336             self._assert_refspec()
337         kwargs = add_progress(kwargs, self.repo.git, progress)
338+
339+        refspec = Git._unpack_args(refspec or [])
340+        if not allow_unsafe_protocols:
341+            for ref in refspec:
342+                Git.check_unsafe_protocols(ref)
343+
344+        if not allow_unsafe_options:
345+            Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_pull_options)
346+
347         proc = self.repo.git.pull("--", self, refspec, with_stdout=False, as_process=True,
348                                   universal_newlines=True, v=True, **kwargs)
349         res = self._get_fetch_info_from_stderr(proc, progress,
350@@ -916,6 +960,8 @@ class Remote(LazyMixin, IterableObj):
351     def push(self, refspec: Union[str, List[str], None] = None,
352              progress: Union[RemoteProgress, 'UpdateProgress', Callable[..., RemoteProgress], None] = None,
353              kill_after_timeout: Union[None, float] = None,
354+             allow_unsafe_protocols: bool = False,
355+             allow_unsafe_options: bool = False,
356              **kwargs: Any) -> IterableList[PushInfo]:
357         """Push changes from source branch in refspec to target branch in refspec.
358
359@@ -945,6 +991,15 @@ class Remote(LazyMixin, IterableObj):
360             If the operation fails completely, the length of the returned IterableList will
361             be 0."""
362         kwargs = add_progress(kwargs, self.repo.git, progress)
363+
364+        refspec = Git._unpack_args(refspec or [])
365+        if not allow_unsafe_protocols:
366+            for ref in refspec:
367+                Git.check_unsafe_protocols(ref)
368+
369+        if not allow_unsafe_options:
370+            Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_push_options)
371+
372         proc = self.repo.git.push("--", self, refspec, porcelain=True, as_process=True,
373                                   universal_newlines=True,
374                                   kill_after_timeout=kill_after_timeout,
375diff --git a/git/repo/base.py b/git/repo/base.py
376index f14f929..7b3565b 100644
377--- a/git/repo/base.py
378+++ b/git/repo/base.py
379@@ -24,7 +24,11 @@ from git.compat import (
380 )
381 from git.config import GitConfigParser
382 from git.db import GitCmdObjectDB
383-from git.exc import InvalidGitRepositoryError, NoSuchPathError, GitCommandError
384+from git.exc import (
385+    GitCommandError,
386+    InvalidGitRepositoryError,
387+    NoSuchPathError,
388+)
389 from git.index import IndexFile
390 from git.objects import Submodule, RootModule, Commit
391 from git.refs import HEAD, Head, Reference, TagReference
392@@ -97,6 +101,18 @@ class Repo(object):
393     re_author_committer_start = re.compile(r'^(author|committer)')
394     re_tab_full_line = re.compile(r'^\t(.*)$')
395
396+    unsafe_git_clone_options = [
397+        # This option allows users to execute arbitrary commands.
398+        # https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---upload-packltupload-packgt
399+        "--upload-pack",
400+        "-u",
401+        # Users can override configuration variables
402+        # like `protocol.allow` or `core.gitProxy` to execute arbitrary commands.
403+        # https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---configltkeygtltvaluegt
404+        "--config",
405+        "-c",
406+    ]
407+
408     # invariants
409     # represents the configuration level of a configuration file
410     config_level: ConfigLevels_Tup = ("system", "user", "global", "repository")
411@@ -1049,7 +1065,8 @@ class Repo(object):
412     @ classmethod
413     def _clone(cls, git: 'Git', url: PathLike, path: PathLike, odb_default_type: Type[GitCmdObjectDB],
414                progress: Union['RemoteProgress', 'UpdateProgress', Callable[..., 'RemoteProgress'], None] = None,
415-               multi_options: Optional[List[str]] = None, **kwargs: Any
416+               multi_options: Optional[List[str]] = None, allow_unsafe_protocols: bool = False,
417+               allow_unsafe_options: bool = False, **kwargs: Any
418                ) -> 'Repo':
419         odbt = kwargs.pop('odbt', odb_default_type)
420
421@@ -1072,6 +1089,12 @@ class Repo(object):
422         multi = None
423         if multi_options:
424             multi = shlex.split(' '.join(multi_options))
425+
426+        if not allow_unsafe_protocols:
427+            Git.check_unsafe_protocols(str(url))
428+        if not allow_unsafe_options and multi_options:
429+            Git.check_unsafe_options(options=multi_options, unsafe_options=cls.unsafe_git_clone_options)
430+
431         proc = git.clone("--", multi, Git.polish_url(str(url)), clone_path, with_extended_output=True, as_process=True,
432                          v=True, universal_newlines=True, **add_progress(kwargs, git, progress))
433         if progress:
434@@ -1107,7 +1130,9 @@ class Repo(object):
435         return repo
436
437     def clone(self, path: PathLike, progress: Optional[Callable] = None,
438-              multi_options: Optional[List[str]] = None, **kwargs: Any) -> 'Repo':
439+              multi_options: Optional[List[str]] = None, unsafe_protocols: bool = False,
440+              allow_unsafe_protocols: bool = False, allow_unsafe_options: bool = False,
441+              **kwargs: Any) -> 'Repo':
442         """Create a clone from this repository.
443
444         :param path: is the full path of the new repo (traditionally ends with ./<name>.git).
445@@ -1116,18 +1141,21 @@ class Repo(object):
446             option per list item which is passed exactly as specified to clone.
447             For example ['--config core.filemode=false', '--config core.ignorecase',
448             '--recurse-submodule=repo1_path', '--recurse-submodule=repo2_path']
449+        :param unsafe_protocols: Allow unsafe protocols to be used, like ex
450         :param kwargs:
451             * odbt = ObjectDatabase Type, allowing to determine the object database
452               implementation used by the returned Repo instance
453             * All remaining keyword arguments are given to the git-clone command
454
455         :return: ``git.Repo`` (the newly cloned repo)"""
456-        return self._clone(self.git, self.common_dir, path, type(self.odb), progress, multi_options, **kwargs)
457+        return self._clone(self.git, self.common_dir, path, type(self.odb), progress, multi_options,
458+                allow_unsafe_protocols=allow_unsafe_protocols, allow_unsafe_options=allow_unsafe_options, **kwargs)
459
460     @ classmethod
461     def clone_from(cls, url: PathLike, to_path: PathLike, progress: Optional[Callable] = None,
462-                   env: Optional[Mapping[str, str]] = None,
463-                   multi_options: Optional[List[str]] = None, **kwargs: Any) -> 'Repo':
464+                   env: Optional[Mapping[str, str]] = None, multi_options: Optional[List[str]] = None,
465+                   unsafe_protocols: bool = False, allow_unsafe_protocols: bool = False,
466+                   allow_unsafe_options: bool = False, **kwargs: Any) -> 'Repo':
467         """Create a clone from the given URL
468
469         :param url: valid git url, see http://www.kernel.org/pub/software/scm/git/docs/git-clone.html#URLS
470@@ -1140,12 +1168,14 @@ class Repo(object):
471             If you want to unset some variable, consider providing empty string
472             as its value.
473         :param multi_options: See ``clone`` method
474+        :param unsafe_protocols: Allow unsafe protocols to be used, like ext
475         :param kwargs: see the ``clone`` method
476         :return: Repo instance pointing to the cloned directory"""
477         git = cls.GitCommandWrapperType(os.getcwd())
478         if env is not None:
479             git.update_environment(**env)
480-        return cls._clone(git, url, to_path, GitCmdObjectDB, progress, multi_options, **kwargs)
481+        return cls._clone(git, url, to_path, GitCmdObjectDB, progress, multi_options,
482+                allow_unsafe_protocols=allow_unsafe_protocols, allow_unsafe_options=allow_unsafe_options, **kwargs)
483
484     def archive(self, ostream: Union[TextIO, BinaryIO], treeish: Optional[str] = None,
485                 prefix: Optional[str] = None, **kwargs: Any) -> Repo:
486--
4872.34.1
488
489