Repository: gitpython-developers/GitPython
Issue category: Security Vulnerability

Issue:
CVE-2023-40590: Untrusted search path on Windows systems leading to arbitrary code execution 
This appeared in the CVE additional information here https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-wfm5-v35h-vwf4.

I found it reported already. I am reporting it here just in case.



Hints:
Thanks. This advisory originated in this repository and is thus known: https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-wfm5-v35h-vwf4 .

However, it seems hard to communicate using an advisory, so we can keep this issue open to collect comments.

Patch:
diff --git a/git/cmd.py b/git/cmd.py
index 3d170facd..3665eb029 100644
--- a/git/cmd.py
+++ b/git/cmd.py
@@ -5,7 +5,7 @@
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
 from __future__ import annotations
 import re
-from contextlib import contextmanager
+import contextlib
 import io
 import logging
 import os
@@ -14,6 +14,7 @@
 import subprocess
 import threading
 from textwrap import dedent
+import unittest.mock
 
 from git.compat import (
     defenc,
@@ -963,8 +964,11 @@ def execute(
                     redacted_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:
             cmd_not_found_exception = FileNotFoundError  # NOQA # exists, flake8 unknown @UndefinedVariable
+            patch_caller_env = contextlib.nullcontext()
         # end handle
 
         stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb")
@@ -980,21 +984,21 @@ def execute(
             istream_ok,
         )
         try:
-            proc = Popen(
-                command,
-                env=env,
-                cwd=cwd,
-                bufsize=-1,
-                stdin=istream or DEVNULL,
-                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,
-            )
-
+            with patch_caller_env:
+                proc = Popen(
+                    command,
+                    env=env,
+                    cwd=cwd,
+                    bufsize=-1,
+                    stdin=istream or DEVNULL,
+                    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,
+                )
         except cmd_not_found_exception as err:
             raise GitCommandNotFound(redacted_command, err) from err
         else:
@@ -1144,7 +1148,7 @@ def update_environment(self, **kwargs: Any) -> Dict[str, Union[str, None]]:
                 del self._environment[key]
         return old_env
 
-    @contextmanager
+    @contextlib.contextmanager
     def custom_environment(self, **kwargs: Any) -> Iterator[None]:
         """
         A context manager around the above ``update_environment`` method to restore the


Test patch:
diff --git a/test/test_git.py b/test/test_git.py
index c5d871f08..540ea9f41 100644
--- a/test/test_git.py
+++ b/test/test_git.py
@@ -4,10 +4,12 @@
 #
 # This module is part of GitPython and is released under
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
+import contextlib
 import os
+import shutil
 import subprocess
 import sys
-from tempfile import TemporaryFile
+from tempfile import TemporaryDirectory, TemporaryFile
 from unittest import mock
 
 from git import Git, refresh, GitCommandError, GitCommandNotFound, Repo, cmd
@@ -20,6 +22,17 @@
 from git.compat import is_win
 
 
+@contextlib.contextmanager
+def _chdir(new_dir):
+    """Context manager to temporarily change directory. Not reentrant."""
+    old_dir = os.getcwd()
+    os.chdir(new_dir)
+    try:
+        yield
+    finally:
+        os.chdir(old_dir)
+
+
 class TestGit(TestBase):
     @classmethod
     def setUpClass(cls):
@@ -75,6 +88,23 @@ def test_it_transforms_kwargs_into_git_command_arguments(self):
     def test_it_executes_git_to_shell_and_returns_result(self):
         self.assertRegex(self.git.execute(["git", "version"]), r"^git version [\d\.]{2}.*$")
 
+    def test_it_executes_git_not_from_cwd(self):
+        with TemporaryDirectory() as tmpdir:
+            if is_win:
+                # Copy an actual binary executable that is not git.
+                other_exe_path = os.path.join(os.getenv("WINDIR"), "system32", "hostname.exe")
+                impostor_path = os.path.join(tmpdir, "git.exe")
+                shutil.copy(other_exe_path, impostor_path)
+            else:
+                # Create a shell script that doesn't do anything.
+                impostor_path = os.path.join(tmpdir, "git")
+                with open(impostor_path, mode="w", encoding="utf-8") as file:
+                    print("#!/bin/sh", file=file)
+                os.chmod(impostor_path, 0o755)
+
+            with _chdir(tmpdir):
+                self.assertRegex(self.git.execute(["git", "version"]), r"^git version\b")
+
     def test_it_accepts_stdin(self):
         filename = fixture_path("cat_file_blob")
         with open(filename, "r") as fh: