19
19
defenc ,
20
20
force_bytes ,
21
21
safe_decode ,
22
- is_posix ,
23
22
is_win ,
24
23
)
25
24
from git .exc import CommandError
43
42
Iterator ,
44
43
List ,
45
44
Mapping ,
45
+ Optional ,
46
46
Sequence ,
47
47
TYPE_CHECKING ,
48
48
TextIO ,
@@ -99,7 +99,7 @@ def handle_process_output(
99
99
Callable [[bytes , "Repo" , "DiffIndex" ], None ],
100
100
],
101
101
stderr_handler : Union [None , Callable [[AnyStr ], None ], Callable [[List [AnyStr ]], None ]],
102
- finalizer : Union [None , Callable [[Union [subprocess . Popen , "Git.AutoInterrupt" ]], None ]] = None ,
102
+ finalizer : Union [None , Callable [[Union [Popen , "Git.AutoInterrupt" ]], None ]] = None ,
103
103
decode_streams : bool = True ,
104
104
kill_after_timeout : Union [None , float ] = None ,
105
105
) -> None :
@@ -207,6 +207,68 @@ def pump_stream(
207
207
return None
208
208
209
209
210
+ def _safer_popen_windows (
211
+ command : Union [str , Sequence [Any ]],
212
+ * ,
213
+ shell : bool = False ,
214
+ env : Optional [Mapping [str , str ]] = None ,
215
+ ** kwargs : Any ,
216
+ ) -> Popen :
217
+ """Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search.
218
+
219
+ This avoids an untrusted search path condition where a file like ``git.exe`` in a
220
+ malicious repository would be run when GitPython operates on the repository. The
221
+ process using GitPython may have an untrusted repository's working tree as its
222
+ current working directory. Some operations may temporarily change to that directory
223
+ before running a subprocess. In addition, while by default GitPython does not run
224
+ external commands with a shell, it can be made to do so, in which case the CWD of
225
+ the subprocess, which GitPython usually sets to a repository working tree, can
226
+ itself be searched automatically by the shell. This wrapper covers all those cases.
227
+
228
+ :note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath``
229
+ environment variable during subprocess creation. It also takes care of passing
230
+ Windows-specific process creation flags, but that is unrelated to path search.
231
+
232
+ :note: The current implementation contains a race condition on :attr:`os.environ`.
233
+ GitPython isn't thread-safe, but a program using it on one thread should ideally
234
+ be able to mutate :attr:`os.environ` on another, without unpredictable results.
235
+ See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
236
+ """
237
+ # CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
238
+ # https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
239
+ # https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
240
+ creationflags = subprocess .CREATE_NO_WINDOW | subprocess .CREATE_NEW_PROCESS_GROUP
241
+
242
+ # When using a shell, the shell is the direct subprocess, so the variable must be
243
+ # set in its environment, to affect its search behavior. (The "1" can be any value.)
244
+ if shell :
245
+ safer_env = {} if env is None else dict (env )
246
+ safer_env ["NoDefaultCurrentDirectoryInExePath" ] = "1"
247
+ else :
248
+ safer_env = env
249
+
250
+ # When not using a shell, the current process does the search in a CreateProcessW
251
+ # API call, so the variable must be set in our environment. With a shell, this is
252
+ # unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
253
+ # patched. If not, in the rare case the ComSpec environment variable is unset, the
254
+ # shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all
255
+ # cases, as here, is simpler and protects against that. (The "1" can be any value.)
256
+ with patch_env ("NoDefaultCurrentDirectoryInExePath" , "1" ):
257
+ return Popen (
258
+ command ,
259
+ shell = shell ,
260
+ env = safer_env ,
261
+ creationflags = creationflags ,
262
+ ** kwargs ,
263
+ )
264
+
265
+
266
+ if os .name == "nt" :
267
+ safer_popen = _safer_popen_windows
268
+ else :
269
+ safer_popen = Popen
270
+
271
+
210
272
def dashify (string : str ) -> str :
211
273
return string .replace ("_" , "-" )
212
274
@@ -225,16 +287,6 @@ def dict_to_slots_and__excluded_are_none(self: object, d: Mapping[str, Any], exc
225
287
## -- End Utilities -- @}
226
288
227
289
228
- # value of Windows process creation flag taken from MSDN
229
- CREATE_NO_WINDOW = 0x08000000
230
-
231
- ## CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards,
232
- # see https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
233
- PROC_CREATIONFLAGS = (
234
- CREATE_NO_WINDOW | subprocess .CREATE_NEW_PROCESS_GROUP if is_win else 0 # type: ignore[attr-defined]
235
- ) # mypy error if not windows
236
-
237
-
238
290
class Git (LazyMixin ):
239
291
240
292
"""
@@ -963,11 +1015,8 @@ def execute(
963
1015
redacted_command ,
964
1016
'"kill_after_timeout" feature is not supported on Windows.' ,
965
1017
)
966
- # Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value.
967
- maybe_patch_caller_env = patch_env ("NoDefaultCurrentDirectoryInExePath" , "1" )
968
1018
else :
969
1019
cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable
970
- maybe_patch_caller_env = contextlib .nullcontext ()
971
1020
# end handle
972
1021
973
1022
stdout_sink = PIPE if with_stdout else getattr (subprocess , "DEVNULL" , None ) or open (os .devnull , "wb" )
@@ -983,21 +1032,18 @@ def execute(
983
1032
istream_ok ,
984
1033
)
985
1034
try :
986
- with maybe_patch_caller_env :
987
- proc = Popen (
988
- command ,
989
- env = env ,
990
- cwd = cwd ,
991
- bufsize = - 1 ,
992
- stdin = istream or DEVNULL ,
993
- stderr = PIPE ,
994
- stdout = stdout_sink ,
995
- shell = shell is not None and shell or self .USE_SHELL ,
996
- close_fds = is_posix , # unsupported on windows
997
- universal_newlines = universal_newlines ,
998
- creationflags = PROC_CREATIONFLAGS ,
999
- ** subprocess_kwargs ,
1000
- )
1035
+ proc = safer_popen (
1036
+ command ,
1037
+ env = env ,
1038
+ cwd = cwd ,
1039
+ bufsize = - 1 ,
1040
+ stdin = (istream or DEVNULL ),
1041
+ stderr = PIPE ,
1042
+ stdout = stdout_sink ,
1043
+ shell = shell ,
1044
+ universal_newlines = universal_newlines ,
1045
+ ** subprocess_kwargs ,
1046
+ )
1001
1047
except cmd_not_found_exception as err :
1002
1048
raise GitCommandNotFound (redacted_command , err ) from err
1003
1049
else :
@@ -1010,11 +1056,7 @@ def execute(
1010
1056
1011
1057
def _kill_process (pid : int ) -> None :
1012
1058
"""Callback method to kill a process."""
1013
- p = Popen (
1014
- ["ps" , "--ppid" , str (pid )],
1015
- stdout = PIPE ,
1016
- creationflags = PROC_CREATIONFLAGS ,
1017
- )
1059
+ p = Popen (["ps" , "--ppid" , str (pid )], stdout = PIPE )
1018
1060
child_pids = []
1019
1061
if p .stdout is not None :
1020
1062
for line in p .stdout :
0 commit comments