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

fix(tree): validate entry names and record boundaries · gitpython-developers/GitPython@d1576c4 · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

Commit d1576c4

Browse files
codex
authored and
Byron
committed
fix(tree): validate entry names and record boundaries
<!-- Byron --> rubber-stamp, after checking diff quickly. <!-- agent --> `TreeModifier.add()` checked only for slashes in names. Embedded NULs could change serialized record boundaries, while dot components and Git metadata aliases produced unsafe tree paths (finding 21507). Reuse the repository path validator for single-component tree names and check both the public modifier and shared serializer. Validate object ID lengths and object modes at serialization, including entries inserted through unchecked or direct-cache APIs. Validate incoming records in `tree_entries_from_data()` as well: use bounded delimiter searches, require octal modes and complete 20-byte object IDs, and reject invalid names before exposing entries. Preserve `bytearray` inputs, immutable returned object IDs, undecodable filenames, and valid whitespace, Unicode, and POSIX backslash and colon names. Check drive/root restrictions on complete paths, not isolated tree components. Update the existing decoding fixture to contain a complete object ID.
1 parent a75aedf commit d1576c4

4 files changed

Lines changed: 86 additions & 51 deletions

File tree

‎git/objects/fun.py‎

Lines changed: 32 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from stat import S_ISDIR
1414

1515
from git.compat import safe_decode, defenc
16+
from git.util import _validate_repo_path
1617

1718
# typing ----------------------------------------------
1819

@@ -38,6 +39,13 @@
3839
# ---------------------------------------------------
3940

4041

42+
def _validate_tree_entry_name(name: str) -> None:
43+
if "/" in name:
44+
raise ValueError("Tree entry names must not contain '/' characters")
45+
# A tree name is a component, not a rooted path; a colon cannot select a drive.
46+
_validate_repo_path("tree/" + name)
47+
48+
4149
def tree_to_stream(entries: Sequence[EntryTup], write: Callable[["ReadableBuffer"], Union[int, None]]) -> None:
4250
"""Write the given list of entries into a stream using its ``write`` method.
4351
@@ -51,6 +59,10 @@ def tree_to_stream(entries: Sequence[EntryTup], write: Callable[["ReadableBuffer
5159
bit_mask = 7 # 3 bits set.
5260

5361
for binsha, mode, name in entries:
62+
if len(binsha) != 20:
63+
raise ValueError("Tree entry object IDs must be exactly 20 bytes")
64+
if mode >> 12 not in (4, 8, 10, 14):
65+
raise ValueError("Invalid tree entry mode")
5466
mode_str = b""
5567
for i in range(6):
5668
mode_str = bytes([((mode >> (i * 3)) & bit_mask) + ord_zero]) + mode_str
@@ -70,59 +82,39 @@ def tree_to_stream(entries: Sequence[EntryTup], write: Callable[["ReadableBuffer
7082
name_bytes = name.encode(defenc)
7183
else:
7284
name_bytes = name # type: ignore[unreachable] # check runtime types - is always str?
85+
_validate_tree_entry_name(safe_decode(name_bytes))
7386
write(b"".join((mode_str, b" ", name_bytes, b"\0", binsha)))
7487
# END for each item
7588

7689

7790
def tree_entries_from_data(data: bytes) -> List[EntryTup]:
78-
"""Read the binary representation of a tree and returns tuples of
79-
:class:`~git.objects.tree.Tree` items.
91+
"""Read complete tree records, rejecting invalid names and truncated fields.
8092
8193
:param data:
8294
Data block with tree data (as bytes).
8395
8496
:return:
8597
list(tuple(binsha, mode, tree_relative_path), ...)
8698
"""
87-
ord_zero = ord("0")
88-
space_ord = ord(" ")
89-
len_data = len(data)
90-
i = 0
9199
out = []
92-
while i < len_data:
93-
mode = 0
94-
95-
# Read Mode
96-
# Some git versions truncate the leading 0, some don't.
97-
# The type will be extracted from the mode later.
98-
while data[i] != space_ord:
99-
# Move existing mode integer up one level being 3 bits and add the actual
100-
# ordinal value of the character.
101-
mode = (mode << 3) + (data[i] - ord_zero)
102-
i += 1
103-
# END while reading mode
104-
105-
# Byte is space now, skip it.
106-
i += 1
107-
108-
# Parse name, it is NULL separated.
109-
110-
ns = i
111-
while data[i] != 0:
112-
i += 1
113-
# END while not reached NULL
114-
115-
# Default encoding for strings in git is UTF-8.
116-
# Only use the respective unicode object if the byte stream was encoded.
117-
name_bytes = data[ns:i]
118-
name = safe_decode(bytes(name_bytes))
119-
120-
# Byte is NULL, get next 20.
121-
i += 1
122-
sha = bytes(data[i : i + 20])
123-
i = i + 20
124-
out.append((sha, mode, name))
125-
# END for each byte in data stream
100+
offset = 0
101+
while offset < len(data):
102+
mode_end = data.find(b" ", offset)
103+
if mode_end < 0:
104+
raise ValueError("Unterminated tree entry mode")
105+
mode_bytes = data[offset:mode_end]
106+
if not mode_bytes or mode_bytes.strip(b"01234567"):
107+
raise ValueError("Invalid tree entry mode")
108+
mode = int(mode_bytes, 8)
109+
if mode >> 12 not in (4, 8, 10, 14):
110+
raise ValueError("Invalid tree entry mode")
111+
name_end = data.find(b"\0", mode_end + 1)
112+
if name_end < 0 or name_end + 21 > len(data):
113+
raise ValueError("Truncated tree entry")
114+
name = safe_decode(bytes(data[mode_end + 1 : name_end]))
115+
_validate_tree_entry_name(name)
116+
offset = name_end + 21
117+
out.append((bytes(data[name_end + 1 : offset]), mode, name))
126118
return out
127119

128120

‎git/objects/tree.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from . import util
1414
from .base import IndexObjUnion, IndexObject
1515
from .blob import Blob
16-
from .fun import tree_entries_from_data, tree_to_stream
16+
from .fun import tree_entries_from_data, tree_to_stream, _validate_tree_entry_name
1717
from .submodule.base import Submodule
1818

1919
# typing -------------------------------------------------
@@ -110,8 +110,7 @@ def add(self, sha: bytes, mode: int, name: str, force: bool = False) -> "TreeMod
110110
:return:
111111
self
112112
"""
113-
if "/" in name:
114-
raise ValueError("Name must not contain '/' characters")
113+
_validate_tree_entry_name(name)
115114
if (mode >> 12) not in Tree._map_id_to_type:
116115
raise ValueError("Invalid object type according to mode %o" % mode)
117116

‎test/test_fun.py‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
# This module is part of GitPython and is released under the
22
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
33

4+
import os.path as osp
45
from io import BytesIO
5-
from stat import S_IFDIR, S_IFLNK, S_IFREG, S_IXUSR
66
from os import stat
7-
import os.path as osp
7+
from stat import S_IFDIR, S_IFLNK, S_IFREG, S_IXUSR
88

9+
import ddt
910
from gitdb.base import IStream
1011
from gitdb.typ import str_tree_type
1112

@@ -20,10 +21,10 @@
2021
)
2122
from git.repo.fun import find_worktree_git_dir
2223
from git.util import bin_to_hex, cygpath, join_path_native
23-
2424
from test.lib import TestBase, with_rw_directory, with_rw_repo
2525

2626

27+
@ddt.ddt
2728
class TestFun(TestBase):
2829
def _assert_index_entries(self, entries, trees):
2930
index = IndexFile.from_tree(self.rorepo, *[self.rorepo.tree(bin_to_hex(t).decode("ascii")) for t in trees])
@@ -304,9 +305,25 @@ def test_linked_worktree_traversal(self, rw_dir):
304305
self.assertTrue(statbuf.st_mode & S_IFDIR)
305306

306307
def test_tree_entries_from_data_with_failing_name_decode(self):
307-
r = tree_entries_from_data(b"100644 \x9f\0aaa")
308-
assert r == [(b"aaa", 33188, "\udc9f")], r
308+
r = tree_entries_from_data(b"100644 \x9f\0" + b"a" * 20)
309+
assert r == [(b"a" * 20, 33188, "\udc9f")], r
309310

310311
def test_tree_entries_from_bytearray(self):
311312
r = tree_entries_from_data(bytearray(b"100644 name\0abcdefghijklmnopqrst"))
312313
assert r == [(b"abcdefghijklmnopqrst", 33188, "name")], r
314+
assert isinstance(r[0][0], bytes)
315+
316+
@ddt.data(b"", b".", b"..", b".git", b"a/b")
317+
def test_tree_reader_rejects_paths_that_cannot_be_tree_components(self, name):
318+
with self.assertRaises(ValueError):
319+
tree_entries_from_data(b"100644 " + name + b"\0" + b"a" * 20)
320+
321+
@ddt.data(b"100644", b"100644 missing-nul", b"100644 name\0short", b"xyz name\0" + b"a" * 20)
322+
def test_malformed_tree_records_fail_cleanly(self, data):
323+
with self.assertRaises(ValueError):
324+
tree_entries_from_data(data)
325+
326+
@ddt.data(b"", b"a" * 19, b"a" * 20 + b"100644 extra\0" + b"b" * 20)
327+
def test_tree_serializer_rejects_wrong_length_object_ids(self, sha):
328+
with self.assertRaises(ValueError):
329+
tree_to_stream([(sha, 0o100644, "file")], BytesIO().write)

‎test/test_tree.py‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,26 @@
33
# This module is part of GitPython and is released under the
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

6-
from io import BytesIO
6+
import os
77
import os.path as osp
8-
from pathlib import Path
98
import subprocess
9+
from io import BytesIO
10+
from pathlib import Path
1011

12+
import ddt
1113
import pytest
1214

1315
from git.objects import Blob, Tree
16+
from git.objects.fun import tree_entries_from_data, tree_to_stream
17+
from git.objects.tree import TreeModifier
1418
from git.repo import Repo
1519
from git.util import cwd
16-
1720
from test.lib import TestBase, with_rw_directory
21+
1822
from .lib.helper import PathLikeMock, with_rw_repo
1923

2024

25+
@ddt.ddt
2126
class TestTree(TestBase):
2227
def test_serializable(self):
2328
# Tree at the given commit contains a submodule as well.
@@ -47,6 +52,19 @@ def test_serializable(self):
4752
testtree._deserialize(stream)
4853
# END for each item in tree
4954

55+
def test_valid_unusual_tree_names_round_trip(self):
56+
names = ["a b", "a\nb", "a\tb", "café", ".gitignore"]
57+
if os.name != "nt":
58+
names.extend(["a\\b", "a:b", "C:relative"])
59+
cache = []
60+
modifier = TreeModifier(cache)
61+
for name in names:
62+
modifier.add(b"a" * 20, 0o100644, name)
63+
modifier.set_done()
64+
data = BytesIO()
65+
tree_to_stream(cache, data.write)
66+
assert tree_entries_from_data(data.getvalue()) == cache
67+
5068
@with_rw_directory
5169
def _get_git_ordered_files(self, rw_dir):
5270
"""Get files as git orders them, to compare in test_tree_modifier_ordering."""
@@ -111,6 +129,15 @@ def names_in_mod_cache():
111129
mod.set_done()
112130
assert names_in_mod_cache() == git_file_names_in_order, "set_done() performs git-sorting"
113131

132+
@ddt.data("", ".", "..", ".git", ".GIT", "git~1", ".git. ", ".g\u200cit", "a/b", "a\0b")
133+
def test_tree_names_are_checked_at_construction_and_serialization(self, name):
134+
cache = []
135+
with pytest.raises(ValueError):
136+
TreeModifier(cache).add(b"a" * 20, 0o100644, name)
137+
assert not cache
138+
with pytest.raises(ValueError):
139+
tree_to_stream([(b"a" * 20, 0o100644, name)], BytesIO().write)
140+
114141
def test_traverse(self):
115142
root = self.rorepo.tree("0.1.6")
116143
num_recursive = 0

0 commit comments

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