Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit2b47933

Browse files
committed
Correct the "not from cwd" test and add more cases
This shows thatCVE-2023-40590 is only partially patched. Thiscommit does not fix the vulnerbility, only the test.The problem is that, when shell=True, the environment of the shellsubprocess, instead of the GitPython process, determines whethersearching for the "git" command will use the current directory.Currently NoDefaultCurrentDirectoryInExePath is set only in thecurrent process, and this is done after the environment for thesubprocess (env) is computed. So when git is an indirect subprocessdue to shell=True, Windows still checks a CWD when looking it up.(Note that this should not be a problem for indirect subprocessesstarted by git itself. When a standard git command is implementedas an external executable, when git runs a custom command, and whenone git command delegates some of its work to another -- e.g."git clone" running git-remote-https -- Git for Windows alreadydoes not search the current directory. Custom git commands thatstart their own git subprocesses could have an analogous pathsearch bug, but this would be separate from the bug in GitPython.)This is an exploitable vulnerability in GitPython. Althoughshell=True is rarer and inherently less secure than the default ofshell=False, it still ought to avoid automatically running anexecutable that may exist only due to having been cloned as part ofan untrusted repository. In addition, historically programs onWindows had sometimes used shell=True as a workaround for consolewindows being displayed for subprocesses, and some such code maystill be in use.Furthermore, even when GitPython's current working directory isoutside the repository being worked on, the Git object in a Repoinstance's "git" attribute holds the repository directory as its"_working_dir" attribute, which Git.execute consults to determinethe value of "cwd" to pass to subprocess.Popen. When the createddirect subprocess is really a shell, this is the CWD where git.exeis looked for before searching PATH directories.This is also why previous, more limited testing (includingaccompanying manual testing with shell=True) didn't discover thebug. Even when modified to test with shell=True, the old test hadthe "impostor" git.exe in the GitPython process's own currentdirectory (which was changed to a temporary directory for the test,where the "impostor" was created, but this was separate from theworking tree of the self.git repository). This was effective forthe shell=False case, but not the shell=True case where theimpostor would be found and run in the repository directory evenwhen it differs from the GitPython process's CWD.
1 parent1c65efb commit2b47933

File tree

2 files changed

+32
-23
lines changed

2 files changed

+32
-23
lines changed

‎test/lib/helper.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,11 @@ def with_rw_directory(func):
8888
test succeeds, but leave it otherwise to aid additional debugging."""
8989

9090
@wraps(func)
91-
defwrapper(self):
91+
defwrapper(self,*args,**kwargs):
9292
path=tempfile.mkdtemp(prefix=func.__name__)
9393
keep=False
9494
try:
95-
returnfunc(self,path)
95+
returnfunc(self,path,*args,**kwargs)
9696
exceptException:
9797
log.info(
9898
"Test %s.%s failed, output is at %r\n",

‎test/test_git.py

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# This module is part of GitPython and is released under the
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

6+
importcontextlib
67
importgc
78
importinspect
89
importlogging
@@ -12,7 +13,7 @@
1213
importshutil
1314
importsubprocess
1415
importsys
15-
fromtempfileimportTemporaryDirectory,TemporaryFile
16+
fromtempfileimportTemporaryFile
1617
fromunittestimportskipUnless
1718

1819
ifsys.version_info>= (3,8):
@@ -135,27 +136,35 @@ def test_it_executes_git_and_returns_result(self):
135136
self.assertRegex(self.git.execute(["git","version"]),r"^git version [\d\.]{2}.*$")
136137

137138
@ddt.data(
138-
(["git","version"],False),
139-
("git version",True),
139+
(False,False, ["git","version"]),
140+
(False,True,"git version"),
141+
(True,False, ["git","version"]),
142+
(True,True,"git version"),
140143
)
141-
deftest_it_executes_git_not_from_cwd(self,case):
142-
command,shell=case
143-
144-
withTemporaryDirectory()astmpdir:
145-
ifos.name=="nt":
146-
# Copy an actual binary executable that is not git.
147-
other_exe_path=os.path.join(os.getenv("WINDIR"),"system32","hostname.exe")
148-
impostor_path=os.path.join(tmpdir,"git.exe")
149-
shutil.copy(other_exe_path,impostor_path)
150-
else:
151-
# Create a shell script that doesn't do anything.
152-
impostor_path=os.path.join(tmpdir,"git")
153-
withopen(impostor_path,mode="w",encoding="utf-8")asfile:
154-
print("#!/bin/sh",file=file)
155-
os.chmod(impostor_path,0o755)
156-
157-
withcwd(tmpdir):
158-
output=self.git.execute(command,shell=shell)
144+
@with_rw_directory
145+
deftest_it_executes_git_not_from_cwd(self,rw_dir,case):
146+
chdir_to_repo,shell,command=case
147+
148+
repo=Repo.init(rw_dir)
149+
150+
ifos.name=="nt":
151+
# Copy an actual binary executable that is not git. (On Windows, running
152+
# "hostname" only displays the hostname, it never tries to change it.)
153+
other_exe_path=os.path.join(os.getenv("WINDIR"),"system32","hostname.exe")
154+
impostor_path=os.path.join(rw_dir,"git.exe")
155+
shutil.copy(other_exe_path,impostor_path)
156+
else:
157+
# Create a shell script that doesn't do anything.
158+
impostor_path=os.path.join(rw_dir,"git")
159+
withopen(impostor_path,mode="w",encoding="utf-8")asfile:
160+
print("#!/bin/sh",file=file)
161+
os.chmod(impostor_path,0o755)
162+
163+
withcwd(rw_dir)ifchdir_to_repoelsecontextlib.nullcontext():
164+
# Run the command without raising an exception on failure, as the exception
165+
# message is currently misleading when the command is a string rather than a
166+
# sequence of strings (it really runs "git", but then wrongly reports "g").
167+
output=repo.git.execute(command,with_exceptions=False,shell=shell)
159168

160169
self.assertRegex(output,r"^git version\b")
161170

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp