| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -33,5 +33,6 @@ Contributors are: | |||
| 33 | 33 | -Steven Whitman <ninloot _at_ gmail.com> | |
| 34 | 34 | -Stefan Stancu <stefan.stancu _at_ gmail.com> | |
| 35 | 35 | -César Izurieta <cesar _at_ caih.org> | |
| 36 | + -Santos Gallegos <stsewd _at_ proton.me> | ||
| 36 | 37 | ||
| 37 | 38 | Portions derived from other open source works and are clearly marked. | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -4,7 +4,9 @@ | |||
| 4 | 4 | # This module is part of GitPython and is released under | |
| 5 | 5 | # the BSD License: http://www.opensource.org/licenses/bsd-license.php | |
| 6 | 6 | ||
| 7 | - from contextlib import contextmanager | ||
| 7 | + import contextlib | ||
| 8 | + import re | ||
| 9 | + | ||
| 8 | 10 | import io | |
| 9 | 11 | import logging | |
| 10 | 12 | import os | |
@@ -19,6 +21,7 @@ | |||
| 19 | 21 | import threading | |
| 20 | 22 | from collections import OrderedDict | |
| 21 | 23 | from textwrap import dedent | |
| 24 | + import mock | ||
| 22 | 25 | ||
| 23 | 26 | from git.compat import ( | |
| 24 | 27 | string_types, | |
@@ -31,7 +34,7 @@ | |||
| 31 | 34 | is_posix, | |
| 32 | 35 | is_win, | |
| 33 | 36 | ) | |
| 34 | - from git.exc import CommandError | ||
| 37 | + from git.exc import CommandError, UnsafeOptionError, UnsafeProtocolError | ||
| 35 | 38 | from git.util import is_cygwin_git, cygpath, expand_path | |
| 36 | 39 | ||
| 37 | 40 | from .exc import ( | |
@@ -59,6 +62,11 @@ | |||
| 59 | 62 | __all__ = ('Git',) | |
| 60 | 63 | ||
| 61 | 64 | ||
| 65 | + @contextlib.contextmanager | ||
| 66 | + def nullcontext(enter_result=None): | ||
| 67 | + yield enter_result | ||
| 68 | + | ||
| 69 | + | ||
| 62 | 70 | # ============================================================================== | |
| 63 | 71 | ## @name Utilities | |
| 64 | 72 | # ------------------------------------------------------------------------------ | |
@@ -124,6 +132,59 @@ def pump_stream(cmdline, name, stream, is_decode, handler): | |||
| 124 | 132 | return finalizer(process) | |
| 125 | 133 | ||
| 126 | 134 | ||
| 135 | + def _safer_popen_windows(command, shell, env=None, **kwargs): | ||
| 136 | + """Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search. | ||
| 137 | + This avoids an untrusted search path condition where a file like ``git.exe`` in a | ||
| 138 | + malicious repository would be run when GitPython operates on the repository. The | ||
| 139 | + process using GitPython may have an untrusted repository's working tree as its | ||
| 140 | + current working directory. Some operations may temporarily change to that directory | ||
| 141 | + before running a subprocess. In addition, while by default GitPython does not run | ||
| 142 | + external commands with a shell, it can be made to do so, in which case the CWD of | ||
| 143 | + the subprocess, which GitPython usually sets to a repository working tree, can | ||
| 144 | + itself be searched automatically by the shell. This wrapper covers all those cases. | ||
| 145 | + :note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath`` | ||
| 146 | + environment variable during subprocess creation. It also takes care of passing | ||
| 147 | + Windows-specific process creation flags, but that is unrelated to path search. | ||
| 148 | + :note: The current implementation contains a race condition on :attr:`os.environ`. | ||
| 149 | + GitPython isn't thread-safe, but a program using it on one thread should ideally | ||
| 150 | + be able to mutate :attr:`os.environ` on another, without unpredictable results. | ||
| 151 | + See comments in https://github.com/gitpython-developers/GitPython/pull/1650. | ||
| 152 | + """ | ||
| 153 | + # CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See: | ||
| 154 | + # https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal | ||
| 155 | + # https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP | ||
| 156 | + creationflags = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP | ||
| 157 | + | ||
| 158 | + # When using a shell, the shell is the direct subprocess, so the variable must be | ||
| 159 | + # set in its environment, to affect its search behavior. (The "1" can be any value.) | ||
| 160 | + if shell: | ||
| 161 | + safer_env = {} if env is None else dict(env) | ||
| 162 | + safer_env["NoDefaultCurrentDirectoryInExePath"] = "1" | ||
| 163 | + else: | ||
| 164 | + safer_env = env | ||
| 165 | + | ||
| 166 | + # When not using a shell, the current process does the search in a CreateProcessW | ||
| 167 | + # API call, so the variable must be set in our environment. With a shell, this is | ||
| 168 | + # unnecessary, in versions where https://github.com/python/cpython/issues/101283 is | ||
| 169 | + # patched. If not, in the rare case the ComSpec environment variable is unset, the | ||
| 170 | + # shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all | ||
| 171 | + # cases, as here, is simpler and protects against that. (The "1" can be any value.) | ||
| 172 | + with patch_env("NoDefaultCurrentDirectoryInExePath", "1"): | ||
| 173 | + return Popen( | ||
| 174 | + command, | ||
| 175 | + shell=shell, | ||
| 176 | + env=safer_env, | ||
| 177 | + creationflags=creationflags, | ||
| 178 | + **kwargs | ||
| 179 | + ) | ||
| 180 | + | ||
| 181 | + | ||
| 182 | + if os.name == "nt": | ||
| 183 | + safer_popen = _safer_popen_windows | ||
| 184 | + else: | ||
| 185 | + safer_popen = Popen | ||
| 186 | + | ||
| 187 | + | ||
| 127 | 188 | def dashify(string): | |
| 128 | 189 | return string.replace('_', '-') | |
| 129 | 190 | ||
@@ -144,11 +205,6 @@ def dict_to_slots_and__excluded_are_none(self, d, excluded=()): | |||
| 144 | 205 | # value of Windows process creation flag taken from MSDN | |
| 145 | 206 | CREATE_NO_WINDOW = 0x08000000 | |
| 146 | 207 | ||
| 147 | - ## CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards, | ||
| 148 | - # see https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal | ||
| 149 | - PROC_CREATIONFLAGS = (CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP | ||
| 150 | - if is_win else 0) | ||
| 151 | - | ||
| 152 | 208 | ||
| 153 | 209 | class Git(LazyMixin): | |
| 154 | 210 | ||
@@ -171,9 +227,49 @@ class Git(LazyMixin): | |||
| 171 | 227 | ||
| 172 | 228 | _excluded_ = ('cat_file_all', 'cat_file_header', '_version_info') | |
| 173 | 229 | ||
| 230 | + re_unsafe_protocol = re.compile("(.+)::.+") | ||
| 231 | + | ||
| 174 | 232 | def __getstate__(self): | |
| 175 | 233 | return slots_to_dict(self, exclude=self._excluded_) | |
| 176 | 234 | ||
| 235 | + @classmethod | ||
| 236 | + def check_unsafe_protocols(cls, url): | ||
| 237 | + """ | ||
| 238 | + Check for unsafe protocols. | ||
| 239 | + Apart from the usual protocols (http, git, ssh), | ||
| 240 | + Git allows "remote helpers" that have the form `<transport>::<address>`, | ||
| 241 | + one of these helpers (`ext::`) can be used to invoke any arbitrary command. | ||
| 242 | + See: | ||
| 243 | + - https://git-scm.com/docs/gitremote-helpers | ||
| 244 | + - https://git-scm.com/docs/git-remote-ext | ||
| 245 | + """ | ||
| 246 | + match = cls.re_unsafe_protocol.match(url) | ||
| 247 | + if match: | ||
| 248 | + protocol = match.group(1) | ||
| 249 | + raise UnsafeProtocolError( | ||
| 250 | + "The `" + protocol + "::` protocol looks suspicious, use `allow_unsafe_protocols=True` to allow it." | ||
| 251 | + ) | ||
| 252 | + | ||
| 253 | + @classmethod | ||
| 254 | + def check_unsafe_options(cls, options, unsafe_options): | ||
| 255 | + """ | ||
| 256 | + Check for unsafe options. | ||
| 257 | + Some options that are passed to `git <command>` can be used to execute | ||
| 258 | + arbitrary commands, this are blocked by default. | ||
| 259 | + """ | ||
| 260 | + # Options can be of the form `foo` or `--foo bar` `--foo=bar`, | ||
| 261 | + # so we need to check if they start with "--foo" or if they are equal to "foo". | ||
| 262 | + bare_unsafe_options = [ | ||
| 263 | + option.lstrip("-") | ||
| 264 | + for option in unsafe_options | ||
| 265 | + ] | ||
| 266 | + for option in options: | ||
| 267 | + for unsafe_option, bare_option in zip(unsafe_options, bare_unsafe_options): | ||
| 268 | + if option.startswith(unsafe_option) or option == bare_option: | ||
| 269 | + raise UnsafeOptionError( | ||
| 270 | + unsafe_option +" is not allowed, use `allow_unsafe_options=True` to allow it." | ||
| 271 | + ) | ||
| 272 | + | ||
| 177 | 273 | def __setstate__(self, d): | |
| 178 | 274 | dict_to_slots_and__excluded_are_none(self, d, excluded=self._excluded_) | |
| 179 | 275 | ||
@@ -705,11 +801,15 @@ def execute(self, command, | |||
| 705 | 801 | cmd_not_found_exception = OSError | |
| 706 | 802 | if kill_after_timeout: | |
| 707 | 803 | raise GitCommandError(command, '"kill_after_timeout" feature is not supported on Windows.') | |
| 804 | + | ||
| 805 | + # Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value. | ||
| 806 | + patch_caller_env = unittest.mock.patch.dict(os.environ, {"NoDefaultCurrentDirectoryInExePath": "1"}) | ||
| 708 | 807 | else: | |
| 709 | 808 | if sys.version_info[0] > 2: | |
| 710 | 809 | cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable | |
| 711 | 810 | else: | |
| 712 | 811 | cmd_not_found_exception = OSError | |
| 812 | + patch_caller_env = nullcontext() | ||
| 713 | 813 | # end handle | |
| 714 | 814 | ||
| 715 | 815 | stdout_sink = (PIPE | |
@@ -721,19 +821,18 @@ def execute(self, command, | |||
| 721 | 821 | log.debug("Popen(%s, cwd=%s, universal_newlines=%s, shell=%s, istream=%s)", | |
| 722 | 822 | command, cwd, universal_newlines, shell, istream_ok) | |
| 723 | 823 | try: | |
| 724 | - proc = Popen(command, | ||
| 725 | - env=env, | ||
| 726 | - cwd=cwd, | ||
| 727 | - bufsize=-1, | ||
| 728 | - stdin=istream, | ||
| 729 | - stderr=PIPE, | ||
| 730 | - stdout=stdout_sink, | ||
| 731 | - shell=shell is not None and shell or self.USE_SHELL, | ||
| 732 | - close_fds=is_posix, # unsupported on windows | ||
| 733 | - universal_newlines=universal_newlines, | ||
| 734 | - creationflags=PROC_CREATIONFLAGS, | ||
| 735 | - **subprocess_kwargs | ||
| 736 | - ) | ||
| 824 | + proc = safer_popen( | ||
| 825 | + command, | ||
| 826 | + env=env, | ||
| 827 | + cwd=cwd, | ||
| 828 | + bufsize=-1, | ||
| 829 | + stdin=istream, | ||
| 830 | + stderr=PIPE, | ||
| 831 | + stdout=stdout_sink, | ||
| 832 | + shell=shell is not None and shell or self.USE_SHELL, | ||
| 833 | + universal_newlines=universal_newlines, | ||
| 834 | + **subprocess_kwargs | ||
| 835 | + ) | ||
| 737 | 836 | except cmd_not_found_exception as err: | |
| 738 | 837 | raise GitCommandNotFound(command, err) | |
| 739 | 838 | ||
@@ -862,7 +961,7 @@ def update_environment(self, **kwargs): | |||
| 862 | 961 | del self._environment[key] | |
| 863 | 962 | return old_env | |
| 864 | 963 | ||
| 865 | - @contextmanager | ||
| 964 | + @contextlib.contextmanager | ||
| 866 | 965 | def custom_environment(self, **kwargs): | |
| 867 | 966 | """ | |
| 868 | 967 | A context manager around the above ``update_environment`` method to restore the | |
@@ -1082,7 +1181,7 @@ def get_object_data(self, ref): | |||
| 1082 | 1181 | :note: not threadsafe""" | |
| 1083 | 1182 | hexsha, typename, size, stream = self.stream_object_data(ref) | |
| 1084 | 1183 | data = stream.read(size) | |
| 1085 | - del(stream) | ||
| 1184 | + del (stream) | ||
| 1086 | 1185 | return (hexsha, typename, size, data) | |
| 1087 | 1186 | ||
| 1088 | 1187 | def stream_object_data(self, ref): | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -25,6 +25,14 @@ class NoSuchPathError(GitError, OSError): | |||
| 25 | 25 | """ Thrown if a path could not be access by the system. """ | |
| 26 | 26 | ||
| 27 | 27 | ||
| 28 | + class UnsafeProtocolError(GitError): | ||
| 29 | + """Thrown if unsafe protocols are passed without being explicitly allowed.""" | ||
| 30 | + | ||
| 31 | + | ||
| 32 | + class UnsafeOptionError(GitError): | ||
| 33 | + """Thrown if unsafe options are passed without being explicitly allowed.""" | ||
| 34 | + | ||
| 35 | + | ||
| 28 | 36 | class CommandError(UnicodeMixin, GitError): | |
| 29 | 37 | """Base class for exceptions thrown at every stage of `Popen()` execution. | |
| 30 | 38 | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments