FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Merge pull request #2208 from gitpython-developers/security-fixes · gitpython-developers/GitPython@9729ed3 · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

Commit 9729ed3

Browse files
Byron
authored
Merge pull request #2208 from gitpython-developers/security-fixes
Block file-reading Git options
2 parents 4ba9ce6 + ce9d8e8 commit 9729ed3

10 files changed

Lines changed: 90 additions & 18 deletions

File tree

‎doc/source/changes.rst‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,20 @@
22
Changelog
33
=========
44

5+
3.1.59
6+
======
7+
8+
Security fixes for
9+
10+
* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-5xxx-qhh7-9287
11+
* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-3wxw-xv34-2frg
12+
13+
If you can, also try and provide feedback on the upcoming v4 branch
14+
https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome.
15+
16+
See the following for all changes.
17+
https://github.com/gitpython-developers/GitPython/releases/tag/3.1.59
18+
519
3.1.58
620
======
721

‎git/cmd.py‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -652,6 +652,7 @@ class Git(metaclass=_GitMeta):
652652
unsafe_git_ls_remote_options = [
653653
# This option allows arbitrary command execution in git-ls-remote.
654654
"--upload-pack",
655+
"--exec",
655656
]
656657

657658
unsafe_git_pathspec_from_file_options = [
@@ -976,7 +977,9 @@ def _canonicalize_option_name(cls, option: str) -> str:
976977
return dashify(option_tokens[0])
977978

978979
@classmethod
979-
def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> None:
980+
def check_unsafe_options(
981+
cls, options: List[str], unsafe_options: List[str], clusterable_short_options: str = "46flnqsv"
982+
) -> None:
980983
"""Raise :class:`~git.exc.UnsafeOptionError` for blocked option spellings.
981984
982985
In addition to exact matches, this rejects abbreviated long options accepted
@@ -1011,7 +1014,7 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) ->
10111014
# These value-less Git flags can be clustered before another short option
10121015
# (for example, ``-fuVALUE``). Stop at any other character because it may
10131016
# begin an attached value, as ``o`` does in the safe option ``-oupstream``.
1014-
clusterable_short_options = frozenset("46flnqsv")
1017+
clusterable_short_options_set = frozenset(clusterable_short_options)
10151018
options_are_kwargs = all(not option.startswith("-") for option in options)
10161019
for option in options:
10171020
candidate = cls._canonicalize_option_name(option)
@@ -1028,7 +1031,7 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) ->
10281031
raise UnsafeOptionError(
10291032
f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it."
10301033
)
1031-
if option_char not in clusterable_short_options:
1034+
if option_char not in clusterable_short_options_set:
10321035
break
10331036
if not (option.startswith("--") or (options_are_kwargs and len(candidate) > 1)):
10341037
continue
@@ -1133,7 +1136,7 @@ def ls_remote(
11331136
"""List references in a remote repository.
11341137
11351138
:param allow_unsafe_options:
1136-
Allow unsafe options, like ``--upload-pack``.
1139+
Allow unsafe options, like ``--upload-pack`` or ``--exec``.
11371140
"""
11381141
if not allow_unsafe_options:
11391142
candidate_options = self._option_candidates(args, kwargs)

‎git/diff.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,8 @@ def diff(
220220
to be read and diffed.
221221
222222
:param allow_unsafe_options:
223-
If ``True``, allow options such as ``--output`` that can write to arbitrary
224-
filesystem paths.
223+
If ``True``, allow options such as ``--output`` and ``-O`` that can write to
224+
or read from arbitrary filesystem paths.
225225
226226
:param kwargs:
227227
Additional arguments passed to :manpage:`git-diff(1)`, such as ``R=True`` to
@@ -238,7 +238,8 @@ def diff(
238238
if not allow_unsafe_options:
239239
Git.check_unsafe_options(
240240
options=Git._option_candidates([other], kwargs),
241-
unsafe_options=self.repo.unsafe_git_revision_options,
241+
unsafe_options=self.repo.unsafe_git_diff_options,
242+
clusterable_short_options="46abceflmnpqrstuvwzBCDMNRW",
242243
)
243244

244245
args: List[Union[PathLike, Diffable]] = []

‎git/index/base.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1567,7 +1567,8 @@ def diff(
15671567
if not allow_unsafe_options:
15681568
Git.check_unsafe_options(
15691569
options=Git._option_candidates([other], kwargs),
1570-
unsafe_options=self.repo.unsafe_git_revision_options,
1570+
unsafe_options=self.repo.unsafe_git_diff_options,
1571+
clusterable_short_options="46abceflmnpqrstuvwzBCDMNRW",
15711572
)
15721573

15731574
# Only run if we are the default repository index.

‎git/refs/tag.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,15 +134,17 @@ def create(
134134
:return:
135135
A new :class:`TagReference`.
136136
"""
137+
legacy_ref = kwargs.pop("ref", None)
138+
if legacy_ref:
139+
reference = legacy_ref
140+
137141
if not allow_unsafe_options:
138142
Git.check_unsafe_options(
139-
options=Git._option_candidates([], kwargs),
143+
options=Git._option_candidates([path, reference], kwargs),
140144
unsafe_options=cls.unsafe_git_tag_options,
145+
clusterable_short_options="46adefilnqsv",
141146
)
142147

143-
if "ref" in kwargs and kwargs["ref"]:
144-
reference = kwargs["ref"]
145-
146148
if "message" in kwargs and kwargs["message"]:
147149
kwargs["m"] = kwargs["message"]
148150
del kwargs["message"]

‎git/repo/base.py‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,19 @@ class Repo:
199199
"-o",
200200
]
201201

202+
unsafe_git_blame_options = unsafe_git_revision_options + [
203+
# These options read from arbitrary files and expose their contents through blame output.
204+
"--contents",
205+
"-S",
206+
"--ignore-revs-file",
207+
]
208+
209+
unsafe_git_diff_options = unsafe_git_revision_options + [
210+
# Reads caller-controlled order patterns from an arbitrary file.
211+
"-O",
212+
"--orderfile",
213+
]
214+
202215
# Invariants
203216
config_level: ConfigLevels_Tup = ("system", "user", "global", "repository")
204217
"""Represents the configuration level of a configuration file."""
@@ -1149,7 +1162,7 @@ def blame_incremental(
11491162
:manpage:`git-rev-parse(1)` is a valid option.
11501163
11511164
:param allow_unsafe_options:
1152-
Allow unsafe options in revision argument, like ``--output``.
1165+
Allow unsafe options in revision argument, like ``--output`` or ``--contents``.
11531166
11541167
:return:
11551168
Lazy iterator of :class:`BlameEntry` tuples, where the commit indicates the
@@ -1161,7 +1174,9 @@ def blame_incremental(
11611174
"""
11621175
if not allow_unsafe_options:
11631176
Git.check_unsafe_options(
1164-
options=Git._option_candidates([rev], kwargs), unsafe_options=self.unsafe_git_revision_options
1177+
options=Git._option_candidates([rev], kwargs),
1178+
unsafe_options=self.unsafe_git_blame_options,
1179+
clusterable_short_options="46bceflnpqstvw",
11651180
)
11661181

11671182
data: bytes = self.git.blame(rev, "--", file, p=True, incremental=True, stdout_as_string=False, **kwargs)
@@ -1253,7 +1268,7 @@ def blame(
12531268
:manpage:`git-rev-parse(1)` is a valid option.
12541269
12551270
:param allow_unsafe_options:
1256-
Allow unsafe options in revision argument, like ``--output``.
1271+
Allow unsafe options in revision argument, like ``--output`` or ``--contents``.
12571272
12581273
:return:
12591274
list: [git.Commit, list: [<line>]]
@@ -1269,7 +1284,8 @@ def blame(
12691284
if not allow_unsafe_options:
12701285
Git.check_unsafe_options(
12711286
options=Git._option_candidates([rev, rev_opts_list], kwargs),
1272-
unsafe_options=self.unsafe_git_revision_options,
1287+
unsafe_options=self.unsafe_git_blame_options,
1288+
clusterable_short_options="46bceflnpqstvw",
12731289
)
12741290
data: bytes = self.git.blame(rev, *rev_opts_list, "--", file, p=True, stdout_as_string=False, **kwargs)
12751291
commits: Dict[str, Commit] = {}

‎test/test_diff.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,11 +376,25 @@ def test_diff_submodule(self):
376376
def test_diff_rejects_unsafe_output_options(self):
377377
commit = self.rorepo.head.commit
378378

379+
commit.diff(S="needle")
380+
379381
calls = (
380382
lambda target: commit.diff(output=target),
381383
lambda target: commit.diff(other=f"--output={target}"),
384+
lambda target: commit.diff(O=target),
385+
lambda target: commit.diff(orderfile=target),
386+
lambda target: commit.diff(other=f"--orderfile={target}"),
387+
lambda target: commit.diff(other=f"-pO{target}"),
388+
lambda target: commit.diff(other=f"-uO{target}"),
389+
lambda target: commit.diff(other=f"-DO{target}"),
382390
lambda target: self.rorepo.index.diff(NULL_TREE, output=target),
383391
lambda target: self.rorepo.index.diff(f"--output={target}"),
392+
lambda target: self.rorepo.index.diff(NULL_TREE, O=target),
393+
lambda target: self.rorepo.index.diff(NULL_TREE, orderfile=target),
394+
lambda target: self.rorepo.index.diff(f"--orderfile={target}"),
395+
lambda target: self.rorepo.index.diff(f"-pO{target}"),
396+
lambda target: self.rorepo.index.diff(f"-uO{target}"),
397+
lambda target: self.rorepo.index.diff(f"-DO{target}"),
384398
)
385399
for index, call in enumerate(calls):
386400
target = osp.join(self.repo_dir, f"diff-output-{index}")

‎test/test_refs.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,21 @@ def test_tag_create_rejects_unsafe_file_options(self, rw_repo):
7070
with self.assertRaises(UnsafeOptionError):
7171
TagReference.create(rw_repo, f"unsafe-{index}", **option)
7272

73+
for args in (
74+
("unsafe-reference", f"--file={message.name}"),
75+
(f"--file={message.name}", "HEAD"),
76+
(f"-eF{message.name}", "HEAD"),
77+
(f"-iF{message.name}", "HEAD"),
78+
):
79+
with self.assertRaises(UnsafeOptionError):
80+
TagReference.create(rw_repo, *args)
81+
82+
with self.assertRaises(UnsafeOptionError):
83+
TagReference.create(rw_repo, "unsafe-ref-kwarg", ref=f"--file={message.name}")
84+
85+
tag = TagReference.create(rw_repo, "legacy-ref", ref="HEAD", allow_unsafe_options=True)
86+
self.assertEqual(tag.commit, rw_repo.head.commit)
87+
7388
tag = TagReference.create(rw_repo, "allowed-file", F=message.name, allow_unsafe_options=True)
7489
self.assertEqual(tag.tag.message, "private tag message")
7590

‎test/test_remote.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,7 @@ def test_ls_remote_unsafe_options(self, rw_repo):
10351035
{"upload-pack": f"touch {tmp_file}"},
10361036
{"upload_pack": f"touch {tmp_file}"},
10371037
{"upl": f"touch {tmp_file}"},
1038+
{"exec": f"touch {tmp_file}"},
10381039
]
10391040
for unsafe_option in unsafe_options:
10401041
with self.assertRaises(UnsafeOptionError):
@@ -1047,6 +1048,8 @@ def test_ls_remote_unsafe_options(self, rw_repo):
10471048
rw_repo.git.ls_remote(f"--upload-pack={tmp_file}", ".")
10481049
with self.assertRaises(UnsafeOptionError):
10491050
rw_repo.git.ls_remote(f"--upl={tmp_file}", ".")
1051+
with self.assertRaises(UnsafeOptionError):
1052+
rw_repo.git.ls_remote(f"--exec={tmp_file}", ".")
10501053
with self.assertRaises(UnsafeOptionError):
10511054
rw_repo.git.ls_remote("--upload-pack", "touch", ".")
10521055
with self.assertRaises(UnsafeOptionError):

‎test/test_repo.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -590,8 +590,11 @@ def test_blame_real(self):
590590
def test_blame_rejects_unsafe_revision(self):
591591
with tempfile.TemporaryDirectory() as tdir:
592592
output_marker = osp.join(tdir, "pwn")
593-
with self.assertRaises(UnsafeOptionError):
594-
self.rorepo.blame(f"--output={output_marker}", "README.md")
593+
for option in ("--output", "--contents", "-S", "-wS", "--ignore-revs-file"):
594+
with self.assertRaises(UnsafeOptionError):
595+
self.rorepo.blame(f"{option}={output_marker}", "README.md")
596+
with self.assertRaises(UnsafeOptionError):
597+
list(self.rorepo.blame_incremental(f"{option}={output_marker}", "README.md"))
595598
assert not osp.exists(output_marker)
596599

597600
def test_blame_rejects_unsafe_options(self):

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL