Consolidated CVE fixes

AUTHORS

@@ -33,5 +33,6 @@ Contributors are:
 -Steven Whitman <ninloot _at_ gmail.com>
 -Stefan Stancu <stefan.stancu _at_ gmail.com>
 -César Izurieta <cesar _at_ caih.org>
+-Santos Gallegos <stsewd _at_ proton.me>
 
 Portions derived from other open source works and are clearly marked.

git/cmd.py

@@ -4,7 +4,9 @@
 # This module is part of GitPython and is released under
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 
-from contextlib import contextmanager
+import contextlib
+import re
+
 import io
 import logging
 import os
@@ -19,6 +21,7 @@
 import threading
 from collections import OrderedDict
 from textwrap import dedent
+import mock
 
 from git.compat import (
     string_types,
@@ -31,7 +34,7 @@
     is_posix,
     is_win,
 )
-from git.exc import CommandError
+from git.exc import CommandError, UnsafeOptionError, UnsafeProtocolError
 from git.util import is_cygwin_git, cygpath, expand_path
 
 from .exc import (
@@ -59,6 +62,11 @@
 __all__ = ('Git',)
 
 
+@contextlib.contextmanager
+def nullcontext(enter_result=None):
+    yield enter_result
+
+
 # ==============================================================================
 ## @name Utilities
 # ------------------------------------------------------------------------------
@@ -124,6 +132,59 @@ def pump_stream(cmdline, name, stream, is_decode, handler):
         return finalizer(process)
 
 
+def _safer_popen_windows(command, shell, env=None, **kwargs):
+    """Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search.
+    This avoids an untrusted search path condition where a file like ``git.exe`` in a
+    malicious repository would be run when GitPython operates on the repository. The
+    process using GitPython may have an untrusted repository's working tree as its
+    current working directory. Some operations may temporarily change to that directory
+    before running a subprocess. In addition, while by default GitPython does not run
+    external commands with a shell, it can be made to do so, in which case the CWD of
+    the subprocess, which GitPython usually sets to a repository working tree, can
+    itself be searched automatically by the shell. This wrapper covers all those cases.
+    :note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath``
+        environment variable during subprocess creation. It also takes care of passing
+        Windows-specific process creation flags, but that is unrelated to path search.
+    :note: The current implementation contains a race condition on :attr:`os.environ`.
+        GitPython isn't thread-safe, but a program using it on one thread should ideally
+        be able to mutate :attr:`os.environ` on another, without unpredictable results.
+        See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
+    """
+    # CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
+    # https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
+    # https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
+    creationflags = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
+
+    # When using a shell, the shell is the direct subprocess, so the variable must be
+    # set in its environment, to affect its search behavior. (The "1" can be any value.)
+    if shell:
+        safer_env = {} if env is None else dict(env)
+        safer_env["NoDefaultCurrentDirectoryInExePath"] = "1"
+    else:
+        safer_env = env
+
+    # When not using a shell, the current process does the search in a CreateProcessW
+    # API call, so the variable must be set in our environment. With a shell, this is
+    # unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
+    # patched. If not, in the rare case the ComSpec environment variable is unset, the
+    # shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all
+    # cases, as here, is simpler and protects against that. (The "1" can be any value.)
+    with patch_env("NoDefaultCurrentDirectoryInExePath", "1"):
+        return Popen(
+            command,
+            shell=shell,
+            env=safer_env,
+            creationflags=creationflags,
+            **kwargs
+        )
+
+
+if os.name == "nt":
+    safer_popen = _safer_popen_windows
+else:
+    safer_popen = Popen
+
+
 def dashify(string):
     return string.replace('_', '-')
 
@@ -144,11 +205,6 @@ def dict_to_slots_and__excluded_are_none(self, d, excluded=()):
 # value of Windows process creation flag taken from MSDN
 CREATE_NO_WINDOW = 0x08000000
 
-## CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards,
-# see https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
-PROC_CREATIONFLAGS = (CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
-                      if is_win else 0)
-
 
 class Git(LazyMixin):
 
@@ -171,9 +227,49 @@ class Git(LazyMixin):
 
     _excluded_ = ('cat_file_all', 'cat_file_header', '_version_info')
 
+    re_unsafe_protocol = re.compile("(.+)::.+")
+
     def __getstate__(self):
         return slots_to_dict(self, exclude=self._excluded_)
 
+    @classmethod
+    def check_unsafe_protocols(cls, url):
+        """
+        Check for unsafe protocols.
+        Apart from the usual protocols (http, git, ssh),
+        Git allows "remote helpers" that have the form `<transport>::<address>`,
+        one of these helpers (`ext::`) can be used to invoke any arbitrary command.
+        See:
+        - https://git-scm.com/docs/gitremote-helpers
+        - https://git-scm.com/docs/git-remote-ext
+        """
+        match = cls.re_unsafe_protocol.match(url)
+        if match:
+            protocol = match.group(1)
+            raise UnsafeProtocolError(
+                "The `" + protocol + "::` protocol looks suspicious, use `allow_unsafe_protocols=True` to allow it."
+            )
+
+    @classmethod
+    def check_unsafe_options(cls, options, unsafe_options):
+        """
+        Check for unsafe options.
+        Some options that are passed to `git <command>` can be used to execute
+        arbitrary commands, this are blocked by default.
+        """
+        # Options can be of the form `foo` or `--foo bar` `--foo=bar`,
+        # so we need to check if they start with "--foo" or if they are equal to "foo".
+        bare_unsafe_options = [
+            option.lstrip("-")
+            for option in unsafe_options
+        ]
+        for option in options:
+            for unsafe_option, bare_option in zip(unsafe_options, bare_unsafe_options):
+                if option.startswith(unsafe_option) or option == bare_option:
+                    raise UnsafeOptionError(
+                        unsafe_option +" is not allowed, use `allow_unsafe_options=True` to allow it."
+                    )
+
     def __setstate__(self, d):
         dict_to_slots_and__excluded_are_none(self, d, excluded=self._excluded_)
 
@@ -705,11 +801,15 @@ def execute(self, command,
             cmd_not_found_exception = OSError
             if kill_after_timeout:
                 raise GitCommandError(command, '"kill_after_timeout" feature is not supported on Windows.')
+
+            # Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value.
+            patch_caller_env = unittest.mock.patch.dict(os.environ, {"NoDefaultCurrentDirectoryInExePath": "1"})
         else:
             if sys.version_info[0] > 2:
                 cmd_not_found_exception = FileNotFoundError  # NOQA # exists, flake8 unknown @UndefinedVariable
             else:
                 cmd_not_found_exception = OSError
+            patch_caller_env = nullcontext()
         # end handle
 
         stdout_sink = (PIPE
@@ -721,19 +821,18 @@ def execute(self, command,
         log.debug("Popen(%s, cwd=%s, universal_newlines=%s, shell=%s, istream=%s)",
                   command, cwd, universal_newlines, shell, istream_ok)
         try:
-            proc = Popen(command,
-                         env=env,
-                         cwd=cwd,
-                         bufsize=-1,
-                         stdin=istream,
-                         stderr=PIPE,
-                         stdout=stdout_sink,
-                         shell=shell is not None and shell or self.USE_SHELL,
-                         close_fds=is_posix,  # unsupported on windows
-                         universal_newlines=universal_newlines,
-                         creationflags=PROC_CREATIONFLAGS,
-                         **subprocess_kwargs
-                         )
+            proc = safer_popen(
+                command,
+                env=env,
+                cwd=cwd,
+                bufsize=-1,
+                stdin=istream,
+                stderr=PIPE,
+                stdout=stdout_sink,
+                shell=shell is not None and shell or self.USE_SHELL,
+                universal_newlines=universal_newlines,
+                **subprocess_kwargs
+            )
         except cmd_not_found_exception as err:
             raise GitCommandNotFound(command, err)
 
@@ -862,7 +961,7 @@ def update_environment(self, **kwargs):
                 del self._environment[key]
         return old_env
 
-    @contextmanager
+    @contextlib.contextmanager
     def custom_environment(self, **kwargs):
         """
         A context manager around the above ``update_environment`` method to restore the
@@ -1082,7 +1181,7 @@ def get_object_data(self, ref):
         :note: not threadsafe"""
         hexsha, typename, size, stream = self.stream_object_data(ref)
         data = stream.read(size)
-        del(stream)
+        del (stream)
         return (hexsha, typename, size, data)
 
     def stream_object_data(self, ref):

git/exc.py

@@ -25,6 +25,14 @@ class NoSuchPathError(GitError, OSError):
     """ Thrown if a path could not be access by the system. """
 
 
+class UnsafeProtocolError(GitError):
+    """Thrown if unsafe protocols are passed without being explicitly allowed."""
+
+
+class UnsafeOptionError(GitError):
+    """Thrown if unsafe options are passed without being explicitly allowed."""
+
+
 class CommandError(UnicodeMixin, GitError):
     """Base class for exceptions thrown at every stage of `Popen()` execution.