Skip to content

Commit 944dba3

Browse files
codexByron
authored andcommitted
Reject abbreviated unsafe git options
Git's parse-options machinery accepts an unambiguous prefix of a long option. In the reference Git checkout, parse-options.c records abbreviated matches in parse_long_opt() and dispatches the abbreviated option when it is not ambiguous. GitPython normalized unsafe option names before comparing them to the deny-list, but only exact canonical names were rejected. That allowed forms such as --upl=... to bypass the --upload-pack guard while still being interpreted by Git as the unsafe option. Make the shared unsafe-option check reject non-empty canonical candidates that are prefixes of blocked canonical option names, while preserving exact-match behavior. Add coverage for the shared helper and for clone, fetch, pull, push, and ls-remote abbreviation forms. Validation: - .venv/bin/python -m compileall -q git/cmd.py test/test_git.py test/test_clone.py test/test_remote.py - git diff --check - Focused pytest/unittest execution could not run because the environment is missing pytest, ddt, and gitdb, and PyPI access failed while installing test-requirements.txt.
1 parent f982935 commit 944dba3

4 files changed

Lines changed: 39 additions & 4 deletions

File tree

git/cmd.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,17 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) ->
974974
# Options can be of the form `foo`, `--foo`, `--foo bar`, or `--foo=bar`.
975975
canonical_unsafe_options = {cls._canonicalize_option_name(option): option for option in unsafe_options}
976976
for option in options:
977-
unsafe_option = canonical_unsafe_options.get(cls._canonicalize_option_name(option))
977+
canonical_option = cls._canonicalize_option_name(option)
978+
unsafe_option = canonical_unsafe_options.get(canonical_option)
979+
if unsafe_option is None and canonical_option:
980+
unsafe_option = next(
981+
(
982+
unsafe_option
983+
for canonical_unsafe_option, unsafe_option in canonical_unsafe_options.items()
984+
if canonical_unsafe_option.startswith(canonical_option)
985+
),
986+
None,
987+
)
978988
if unsafe_option is not None:
979989
raise UnsafeOptionError(f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it.")
980990

test/test_clone.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,10 @@ def test_clone_unsafe_options(self, rw_repo):
117117
tmp_file = tmp_dir / "pwn"
118118
unsafe_options = [
119119
f"--upload-pack='touch {tmp_file}'",
120+
f"--upl='touch {tmp_file}'",
120121
f"-u 'touch {tmp_file}'",
121122
"--config=protocol.ext.allow=always",
123+
"--conf=protocol.ext.allow=always",
122124
"-c protocol.ext.allow=always",
123125
]
124126
for unsafe_option in unsafe_options:
@@ -129,8 +131,10 @@ def test_clone_unsafe_options(self, rw_repo):
129131
unsafe_options = [
130132
{"upload-pack": f"touch {tmp_file}"},
131133
{"upload_pack": f"touch {tmp_file}"},
134+
{"upl": f"touch {tmp_file}"},
132135
{"u": f"touch {tmp_file}"},
133136
{"config": "protocol.ext.allow=always"},
137+
{"conf": "protocol.ext.allow=always"},
134138
{"c": "protocol.ext.allow=always"},
135139
]
136140
for unsafe_option in unsafe_options:

test/test_git.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,9 @@ def test_check_unsafe_options_normalizes_kwargs(self):
164164
(["c"], ["-c"]),
165165
(["--upload-pack=/tmp/helper"], ["--upload-pack"]),
166166
(["--config core.filemode=false"], ["--config"]),
167+
(["--upl=/tmp/helper"], ["--upload-pack"]),
168+
(["conf"], ["--config"]),
169+
(["--out=/tmp/output"], ["--output"]),
167170
]
168171

169172
for options, unsafe_options in cases:

test/test_remote.py

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,11 @@ def test_fetch_unsafe_options(self, rw_repo):
832832
remote = rw_repo.remote("origin")
833833
tmp_dir = Path(tdir)
834834
tmp_file = tmp_dir / "pwn"
835-
unsafe_options = [{"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}]
835+
unsafe_options = [
836+
{"upload-pack": f"touch {tmp_file}"},
837+
{"upload_pack": f"touch {tmp_file}"},
838+
{"upl": f"touch {tmp_file}"},
839+
]
836840
for unsafe_option in unsafe_options:
837841
with self.assertRaises(UnsafeOptionError):
838842
remote.fetch(**unsafe_option)
@@ -900,7 +904,11 @@ def test_pull_unsafe_options(self, rw_repo):
900904
remote = rw_repo.remote("origin")
901905
tmp_dir = Path(tdir)
902906
tmp_file = tmp_dir / "pwn"
903-
unsafe_options = [{"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}]
907+
unsafe_options = [
908+
{"upload-pack": f"touch {tmp_file}"},
909+
{"upload_pack": f"touch {tmp_file}"},
910+
{"upl": f"touch {tmp_file}"},
911+
]
904912
for unsafe_option in unsafe_options:
905913
with self.assertRaises(UnsafeOptionError):
906914
remote.pull(**unsafe_option)
@@ -971,7 +979,9 @@ def test_push_unsafe_options(self, rw_repo):
971979
unsafe_options = [
972980
{"receive-pack": f"touch {tmp_file}"},
973981
{"receive_pack": f"touch {tmp_file}"},
982+
{"rec": f"touch {tmp_file}"},
974983
{"exec": f"touch {tmp_file}"},
984+
{"exe": f"touch {tmp_file}"},
975985
]
976986
for unsafe_option in unsafe_options:
977987
assert not tmp_file.exists()
@@ -1012,14 +1022,22 @@ def test_ls_remote_unsafe_options(self, rw_repo):
10121022
with tempfile.TemporaryDirectory() as tdir:
10131023
tmp_dir = Path(tdir)
10141024
tmp_file = tmp_dir / "pwn"
1015-
unsafe_options = [{"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}]
1025+
unsafe_options = [
1026+
{"upload-pack": f"touch {tmp_file}"},
1027+
{"upload_pack": f"touch {tmp_file}"},
1028+
{"upl": f"touch {tmp_file}"},
1029+
]
10161030
for unsafe_option in unsafe_options:
10171031
with self.assertRaises(UnsafeOptionError):
10181032
rw_repo.git.ls_remote(".", **unsafe_option)
10191033
with self.assertRaises(UnsafeOptionError):
10201034
rw_repo.git.ls_remote([f"--upload-pack={tmp_file}"], ".")
1035+
with self.assertRaises(UnsafeOptionError):
1036+
rw_repo.git.ls_remote([f"--upl={tmp_file}"], ".")
10211037
with self.assertRaises(UnsafeOptionError):
10221038
rw_repo.git.ls_remote(f"--upload-pack={tmp_file}", ".")
1039+
with self.assertRaises(UnsafeOptionError):
1040+
rw_repo.git.ls_remote(f"--upl={tmp_file}", ".")
10231041
with self.assertRaises(UnsafeOptionError):
10241042
rw_repo.git.ls_remote("--upload-pack", "touch", ".")
10251043

0 commit comments

Comments
 (0)