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

fix(refs): bound symbolic reference resolution and updates · gitpython-developers/GitPython@c99207e · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

Commit c99207e

Browse files
codex
authored and
Byron
committed
fix(refs): bound symbolic reference resolution and updates
<!-- Byron --> rubber-stamp, after checking diff quickly. <!-- agent --> Cyclic loose symbolic refs could keep `dereference_recursive()` reading forever. Related updates and construction of custom symbolic refs could recurse without a useful limit (finding 21496). Bound reads and preflight updates to five references, including the terminal reference, matching Git's `SYMREF_MAXDEPTH`. Determine detachment from the current ref's contents without recursively constructing targets. Reuse validated ref paths for reads to reject escaping symlinks. Preserve public update signatures and subclass dispatch, including HEAD reflog updates and creation of unborn terminal references. The write preflight handles static chains; it does not introduce a transaction for concurrent external repository changes.
1 parent 3ca35bf commit c99207e

2 files changed

Lines changed: 193 additions & 16 deletions

File tree

‎git/refs/symbolic.py‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ class SymbolicReference:
7676
_common_path_default = ""
7777
_remote_common_path_default = "refs/remotes"
7878
_id_attribute_ = "name"
79+
# Match Git's SYMREF_MAXDEPTH, counting the terminal reference as well.
80+
_max_symref_depth = 5
7981

8082
def __init__(self, repo: "Repo", path: PathLike, check_path: bool = False) -> None:
8183
self.repo = repo
@@ -191,13 +193,18 @@ def dereference_recursive(cls, repo: "Repo", ref_path: Union[PathLike, None]) ->
191193
192194
:param repo:
193195
The repository containing the reference at `ref_path`.
196+
197+
:raise ValueError:
198+
If the reference is missing, invalid, or exceeds Git's limit of five
199+
references in a symbolic reference chain (including the terminal ref).
194200
"""
195201

196-
while True:
202+
for _ in range(cls._max_symref_depth):
197203
hexsha, ref_path = cls._get_ref_info(repo, ref_path)
198204
if hexsha is not None:
199205
return hexsha
200206
# END recursive dereferencing
207+
raise ValueError("Too many levels of symbolic references at %r" % ref_path)
201208

202209
@staticmethod
203210
def _check_ref_name_valid(ref_path: PathLike) -> None:
@@ -258,13 +265,13 @@ def _get_ref_info_helper(
258265
* *sha* is of the file at rela_path points to if available, or ``None``.
259266
* *target_ref_path* is the reference we point to, or ``None``.
260267
"""
261-
if ref_path:
262-
cls._check_ref_name_valid(ref_path)
268+
if ref_path is None:
269+
raise ValueError("Reference at %r does not exist" % ref_path)
270+
ref_file = cls._get_validated_ref_path(repo, ref_path)
263271

264272
tokens: Union[None, List[str], Tuple[str, str]] = None
265-
repodir = _git_dir(repo, ref_path)
266273
try:
267-
with open(os.path.join(repodir, ref_path), "rt", encoding="UTF-8") as fp: # type: ignore[arg-type]
274+
with open(ref_file, "rt", encoding="UTF-8") as fp:
268275
value = fp.read().rstrip()
269276
# Don't only split on spaces, but on whitespace, which allows to parse lines like:
270277
# 60b64ef992065e2600bfef6187a97f92398a9144 branch 'master' of git-server:/path/to/repo
@@ -398,7 +405,23 @@ def set_object(
398405
399406
:return:
400407
self
408+
409+
:raise ValueError:
410+
If the symbolic reference chain exceeds Git's limit of five references.
401411
"""
412+
# Validate before recursing through the public method, preserving subclass
413+
# dispatch and Reference's HEAD reflog updates.
414+
ref_path: Union[PathLike, None] = self.path
415+
for _ in range(self._max_symref_depth):
416+
try:
417+
hexsha, ref_path = self._get_ref_info(self.repo, ref_path)
418+
except ValueError:
419+
break # Allow creating an unborn terminal reference.
420+
if hexsha is not None:
421+
break
422+
else:
423+
raise ValueError("Too many levels of symbolic references at %r" % ref_path)
424+
402425
if isinstance(object, SymbolicReference):
403426
object = object.object # @ReservedAssignment
404427
# END resolve references
@@ -584,11 +607,9 @@ def is_detached(self) -> bool:
584607
``True`` if we are a detached reference, hence we point to a specific commit
585608
instead to another reference.
586609
"""
587-
try:
588-
self.ref # noqa: B018
589-
return False
590-
except TypeError:
591-
return True
610+
# Inspect only this ref. Constructing its target would recursively inspect
611+
# symbolic references with nonstandard names through from_path().
612+
return self._get_ref_info(self.repo, self.path)[1] is None
592613

593614
def log(self) -> "RefLog":
594615
"""

‎test/test_refs.py‎

Lines changed: 162 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,32 @@
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

66
import contextlib
7-
from itertools import chain
87
import os.path as osp
9-
from pathlib import Path
108
import tempfile
9+
import unittest
10+
from itertools import chain
11+
from pathlib import Path
12+
from unittest.mock import patch
1113

1214
from gitdb.exc import BadName
1315

16+
import git.refs as refs
1417
from git import (
1518
Commit,
1619
GitCommandError,
1720
GitConfigParser,
1821
Head,
19-
RefLog,
2022
Reference,
23+
RefLog,
2124
RemoteReference,
2225
Repo,
2326
SymbolicReference,
2427
TagReference,
2528
)
2629
from git.exc import UnsafeOptionError
2730
from git.objects.tag import TagObject
28-
import git.refs as refs
2931
from git.util import Actor, rmtree
30-
31-
from test.lib import TestBase, requires_symlinks, with_rw_repo, PathLikeMock
32+
from test.lib import PathLikeMock, TestBase, requires_symlinks, with_rw_repo
3233

3334

3435
class TestRefs(TestBase):
@@ -912,3 +913,158 @@ def test_validity_ref_names(self):
912913

913914
# Valid reference name should not raise.
914915
check_ref("valid/ref/name")
916+
917+
918+
class TestSymbolicReferenceSecurity(unittest.TestCase):
919+
def setUp(self):
920+
self.tmp = tempfile.TemporaryDirectory()
921+
self.addCleanup(self.tmp.cleanup)
922+
self.repo = Repo.init(Path(self.tmp.name) / "repo")
923+
self.addCleanup(self.repo.close)
924+
actor = Actor("Reference Test", "reference@example.invalid")
925+
self.commit = self.repo.index.commit("initial", author=actor, committer=actor)
926+
self.next_commit = self.repo.index.commit("next", author=actor, committer=actor, head=False)
927+
928+
def write_ref(self, path, value):
929+
file = Path(self.repo.git_dir) / path
930+
file.parent.mkdir(parents=True, exist_ok=True)
931+
file.write_text(value + "\n", encoding="utf-8")
932+
933+
def chain(self, count, terminal=None):
934+
paths = ["HEAD"] + [f"refs/heads/link-{i}" for i in range(1, count)]
935+
for path, target in zip(paths, paths[1:]):
936+
self.write_ref(path, "ref: " + target)
937+
self.write_ref(paths[-1], self.commit.hexsha if terminal is None else terminal)
938+
return paths
939+
940+
@contextlib.contextmanager
941+
def bounded_reads(self):
942+
# Make the regression fail deterministically instead of hanging on a cycle.
943+
read_ref = SymbolicReference._get_ref_info
944+
reads = 0
945+
946+
def read(repo, path):
947+
nonlocal reads
948+
reads += 1
949+
if reads > 100:
950+
self.fail("Reference traversal exceeded its read budget")
951+
return read_ref(repo, path)
952+
953+
with patch.object(SymbolicReference, "_get_ref_info", side_effect=read):
954+
yield
955+
956+
def test_cycles_fail_in_all_object_read_entry_points(self):
957+
for target in ("refs/heads/a", "refs/heads/b"):
958+
self.write_ref("HEAD", "ref: refs/heads/a")
959+
self.write_ref("refs/heads/a", "ref: " + target)
960+
self.write_ref("refs/heads/b", "ref: refs/heads/a")
961+
self.write_ref("refs/tags/loop", "ref: refs/heads/a")
962+
for read in (
963+
lambda: SymbolicReference.dereference_recursive(self.repo, "HEAD"),
964+
lambda: self.repo.head.commit,
965+
lambda: self.repo.head.object,
966+
lambda: Head(self.repo, "refs/heads/a").commit,
967+
lambda: TagReference(self.repo, "refs/tags/loop").object,
968+
):
969+
with self.subTest(target=target, read=read), self.bounded_reads():
970+
with self.assertRaisesRegex(ValueError, "symbolic reference"):
971+
read()
972+
for read in (lambda: self.repo.commit("HEAD"), lambda: self.repo.rev_parse("a")):
973+
with self.subTest(target=target, read=read), self.bounded_reads():
974+
with self.assertRaises(BadName):
975+
read()
976+
with self.bounded_reads():
977+
self.assertFalse(self.repo.head.is_valid())
978+
979+
def test_depth_limit_matches_git_including_terminal_reference(self):
980+
for count in range(1, 7):
981+
self.chain(count)
982+
with self.subTest(count=count), self.bounded_reads():
983+
if count <= 5:
984+
self.assertEqual(self.repo.head.commit, self.commit)
985+
self.assertEqual(self.repo.git.rev_parse("--verify", "HEAD"), self.commit.hexsha)
986+
else:
987+
with self.assertRaises(ValueError):
988+
self.repo.head.commit
989+
with self.assertRaises(GitCommandError):
990+
self.repo.git.rev_parse("--verify", "HEAD")
991+
992+
def test_packed_terminal_reference_resolves(self):
993+
paths = self.chain(5)
994+
(Path(self.repo.git_dir) / paths[-1]).unlink()
995+
self.write_ref("packed-refs", f"{self.commit.hexsha} {paths[-1]}")
996+
self.assertEqual(self.repo.head.commit, self.commit)
997+
998+
def test_writes_reject_cycles_and_excessive_depth_without_side_effects(self):
999+
for terminal in ("ref: refs/heads/link-1", self.commit.hexsha):
1000+
paths = self.chain(6, terminal)
1001+
before = {path: (Path(self.repo.git_dir) / path).read_bytes() for path in paths}
1002+
head_log = Path(self.repo.git_dir) / "logs" / "HEAD"
1003+
old_log = head_log.read_bytes()
1004+
for logmsg in (None, "must not write"):
1005+
with self.subTest(terminal=terminal, logmsg=logmsg), self.bounded_reads():
1006+
with self.assertRaisesRegex(ValueError, "symbolic reference"):
1007+
self.repo.head.set_object(self.next_commit, logmsg)
1008+
self.assertEqual(before, {path: (Path(self.repo.git_dir) / path).read_bytes() for path in paths})
1009+
self.assertEqual(head_log.read_bytes(), old_log)
1010+
1011+
def test_valid_chain_updates_preserve_symbols_and_head_reflog(self):
1012+
paths = self.chain(5)
1013+
self.repo.head.set_object(self.next_commit, "updated through chain")
1014+
self.assertEqual(self.repo.head.commit, self.next_commit)
1015+
for path, target in zip(paths, paths[1:]):
1016+
self.assertEqual((Path(self.repo.git_dir) / path).read_text(), "ref: " + target + "\n")
1017+
for path in ("HEAD", paths[-1]):
1018+
entry = SymbolicReference(self.repo, path).log()[-1]
1019+
self.assertEqual(entry.oldhexsha, self.commit.hexsha)
1020+
self.assertEqual(entry.newhexsha, self.next_commit.hexsha)
1021+
self.assertEqual(entry.message, "updated through chain")
1022+
1023+
def test_writes_create_unborn_terminal_reference(self):
1024+
paths = self.chain(5)
1025+
(Path(self.repo.git_dir) / paths[-1]).unlink()
1026+
self.repo.head.set_object(self.next_commit)
1027+
self.assertEqual(self.repo.head.commit, self.next_commit)
1028+
self.assertEqual(self.repo.head.reference.path, paths[1])
1029+
1030+
def test_set_object_preserves_subclass_override_signature_and_dispatch(self):
1031+
calls = []
1032+
1033+
class CustomReference(Reference):
1034+
@classmethod
1035+
def from_path(cls, repo, path):
1036+
return cls(repo, path, check_path=False)
1037+
1038+
def set_object(self, object, logmsg=None):
1039+
calls.append(self.path)
1040+
return super().set_object(object, logmsg)
1041+
1042+
paths = self.chain(5)
1043+
ref = CustomReference(self.repo, "HEAD", check_path=False)
1044+
self.assertIs(ref.set_object(self.next_commit, "custom reference"), ref)
1045+
self.assertEqual(calls, paths)
1046+
self.assertEqual(self.repo.head.commit, self.next_commit)
1047+
self.assertEqual(self.repo.head.log()[-1].message, "custom reference")
1048+
1049+
def test_custom_symbolic_reference_names_do_not_recurse_during_construction(self):
1050+
self.write_ref("CUSTOM_A", "ref: CUSTOM_B")
1051+
self.write_ref("CUSTOM_B", "ref: CUSTOM_A")
1052+
with self.bounded_reads():
1053+
ref = SymbolicReference.from_path(self.repo, "CUSTOM_A")
1054+
self.assertEqual(ref.reference.path, "CUSTOM_B")
1055+
self.assertFalse(ref.is_detached)
1056+
self.assertFalse(ref.is_valid())
1057+
with self.assertRaises(ValueError):
1058+
ref.set_object(self.next_commit)
1059+
1060+
def test_reference_reads_reject_symlinks_outside_repository(self):
1061+
outside = Path(self.tmp.name) / "outside"
1062+
outside.write_text(self.commit.hexsha + "\n", encoding="utf-8")
1063+
path = Path(self.repo.git_dir) / "refs" / "heads" / "external"
1064+
try:
1065+
path.symlink_to(outside)
1066+
except OSError:
1067+
self.skipTest("Symlinks are unavailable")
1068+
self.write_ref("HEAD", "ref: refs/heads/external")
1069+
with self.assertRaisesRegex(ValueError, "escapes the repository"):
1070+
self.repo.head.commit

0 commit comments

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