Merge pull request #1521 from stsewd/block-insecure-options · gitpython-developers/GitPython@678a8fe · GitHub
Skip to content

Commit 678a8fe

Browse files
ByronByron
authored
Merge pull request #1521 from stsewd/block-insecure-options
Block insecure options and protocols by default
2 parents ae6a6e4 + f4f2658 commit 678a8fe

10 files changed

Lines changed: 752 additions & 21 deletions

File tree

‎AUTHORS‎

Lines changed: 1 addition & 0 deletions

‎git/cmd.py‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# This module is part of GitPython and is released under
55
# the BSD License: http://www.opensource.org/licenses/bsd-license.php
66
from __future__ import annotations
7+
import re
78
from contextlib import contextmanager
89
import io
910
import logging
@@ -24,7 +25,7 @@
2425
from git.exc import CommandError
2526
from git.util import is_cygwin_git, cygpath, expand_path, remove_password_if_present
2627

27-
from .exc import GitCommandError, GitCommandNotFound
28+
from .exc import GitCommandError, GitCommandNotFound, UnsafeOptionError, UnsafeProtocolError
2829
from .util import (
2930
LazyMixin,
3031
stream_copy,
@@ -262,6 +263,8 @@ class Git(LazyMixin):
262263

263264
_excluded_ = ("cat_file_all", "cat_file_header", "_version_info")
264265

266+
re_unsafe_protocol = re.compile("(.+)::.+")
267+
265268
def __getstate__(self) -> Dict[str, Any]:
266269
return slots_to_dict(self, exclude=self._excluded_)
267270

@@ -454,6 +457,48 @@ def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> PathLike:
454457
url = url.replace("\\\\", "\\").replace("\\", "/")
455458
return url
456459

460+
@classmethod
461+
def check_unsafe_protocols(cls, url: str) -> None:
462+
"""
463+
Check for unsafe protocols.
464+
465+
Apart from the usual protocols (http, git, ssh),
466+
Git allows "remote helpers" that have the form `<transport>::<address>`,
467+
one of these helpers (`ext::`) can be used to invoke any arbitrary command.
468+
469+
See:
470+
471+
- https://git-scm.com/docs/gitremote-helpers
472+
- https://git-scm.com/docs/git-remote-ext
473+
"""
474+
match = cls.re_unsafe_protocol.match(url)
475+
if match:
476+
protocol = match.group(1)
477+
raise UnsafeProtocolError(
478+
f"The `{protocol}::` protocol looks suspicious, use `allow_unsafe_protocols=True` to allow it."
479+
)
480+
481+
@classmethod
482+
def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> None:
483+
"""
484+
Check for unsafe options.
485+
486+
Some options that are passed to `git <command>` can be used to execute
487+
arbitrary commands, this are blocked by default.
488+
"""
489+
# Options can be of the form `foo` or `--foo bar` `--foo=bar`,
490+
# so we need to check if they start with "--foo" or if they are equal to "foo".
491+
bare_unsafe_options = [
492+
option.lstrip("-")
493+
for option in unsafe_options
494+
]
495+
for option in options:
496+
for unsafe_option, bare_option in zip(unsafe_options, bare_unsafe_options):
497+
if option.startswith(unsafe_option) or option == bare_option:
498+
raise UnsafeOptionError(
499+
f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it."
500+
)
501+
457502
class AutoInterrupt(object):
458503
"""Kill/Interrupt the stored process instance once this instance goes out of scope. It is
459504
used to prevent processes piling up in case iterators stop reading.
@@ -1148,12 +1193,12 @@ def transform_kwargs(self, split_single_char_options: bool = True, **kwargs: Any
11481193
return args
11491194

11501195
@classmethod
1151-
def __unpack_args(cls, arg_list: Sequence[str]) -> List[str]:
1196+
def _unpack_args(cls, arg_list: Sequence[str]) -> List[str]:
11521197

11531198
outlist = []
11541199
if isinstance(arg_list, (list, tuple)):
11551200
for arg in arg_list:
1156-
outlist.extend(cls.__unpack_args(arg))
1201+
outlist.extend(cls._unpack_args(arg))
11571202
else:
11581203
outlist.append(str(arg_list))
11591204

@@ -1238,7 +1283,7 @@ def _call_process(
12381283
# Prepare the argument list
12391284

12401285
opt_args = self.transform_kwargs(**opts_kwargs)
1241-
ext_args = self.__unpack_args([a for a in args if a is not None])
1286+
ext_args = self._unpack_args([a for a in args if a is not None])
12421287

12431288
if insert_after_this_arg is None:
12441289
args_list = opt_args + ext_args

‎git/exc.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ class NoSuchPathError(GitError, OSError):
3737
"""Thrown if a path could not be access by the system."""
3838

3939

40+
class UnsafeProtocolError(GitError):
41+
"""Thrown if unsafe protocols are passed without being explicitly allowed."""
42+
43+
44+
class UnsafeOptionError(GitError):
45+
"""Thrown if unsafe options are passed without being explicitly allowed."""
46+
47+
4048
class CommandError(GitError):
4149
"""Base class for exceptions thrown at every stage of `Popen()` execution.
4250

‎git/objects/submodule/base.py‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,16 @@ def _module_abspath(cls, parent_repo: "Repo", path: PathLike, name: str) -> Path
272272
# end
273273

274274
@classmethod
275-
def _clone_repo(cls, repo: "Repo", url: str, path: PathLike, name: str, **kwargs: Any) -> "Repo":
275+
def _clone_repo(
276+
cls,
277+
repo: "Repo",
278+
url: str,
279+
path: PathLike,
280+
name: str,
281+
allow_unsafe_options: bool = False,
282+
allow_unsafe_protocols: bool = False,
283+
**kwargs: Any,
284+
) -> "Repo":
276285
""":return: Repo instance of newly cloned repository
277286
:param repo: our parent repository
278287
:param url: url to clone from
@@ -289,7 +298,13 @@ def _clone_repo(cls, repo: "Repo", url: str, path: PathLike, name: str, **kwargs
289298
module_checkout_path = osp.join(str(repo.working_tree_dir), path)
290299
# end
291300

292-
clone = git.Repo.clone_from(url, module_checkout_path, **kwargs)
301+
clone = git.Repo.clone_from(
302+
url,
303+
module_checkout_path,
304+
allow_unsafe_options=allow_unsafe_options,
305+
allow_unsafe_protocols=allow_unsafe_protocols,
306+
**kwargs,
307+
)
293308
if cls._need_gitfile_submodules(repo.git):
294309
cls._write_git_file_and_module_config(module_checkout_path, module_abspath)
295310
# end
@@ -359,6 +374,8 @@ def add(
359374
depth: Union[int, None] = None,
360375
env: Union[Mapping[str, str], None] = None,
361376
clone_multi_options: Union[Sequence[TBD], None] = None,
377+
allow_unsafe_options: bool = False,
378+
allow_unsafe_protocols: bool = False,
362379
) -> "Submodule":
363380
"""Add a new submodule to the given repository. This will alter the index
364381
as well as the .gitmodules file, but will not create a new commit.
@@ -475,7 +492,16 @@ def add(
475492
kwargs["multi_options"] = clone_multi_options
476493

477494
# _clone_repo(cls, repo, url, path, name, **kwargs):
478-
mrepo = cls._clone_repo(repo, url, path, name, env=env, **kwargs)
495+
mrepo = cls._clone_repo(
496+
repo,
497+
url,
498+
path,
499+
name,
500+
env=env,
501+
allow_unsafe_options=allow_unsafe_options,
502+
allow_unsafe_protocols=allow_unsafe_protocols,
503+
**kwargs,
504+
)
479505
# END verify url
480506

481507
## See #525 for ensuring git urls in config-files valid under Windows.
@@ -520,6 +546,8 @@ def update(
520546
keep_going: bool = False,
521547
env: Union[Mapping[str, str], None] = None,
522548
clone_multi_options: Union[Sequence[TBD], None] = None,
549+
allow_unsafe_options: bool = False,
550+
allow_unsafe_protocols: bool = False,
523551
) -> "Submodule":
524552
"""Update the repository of this submodule to point to the checkout
525553
we point at with the binsha of this instance.
@@ -643,6 +671,8 @@ def update(
643671
n=True,
644672
env=env,
645673
multi_options=clone_multi_options,
674+
allow_unsafe_options=allow_unsafe_options,
675+
allow_unsafe_protocols=allow_unsafe_protocols,
646676
)
647677
# END handle dry-run
648678
progress.update(

‎git/remote.py‎

Lines changed: 63 additions & 7 deletions

0 commit comments

Comments
 (0)