Skip to content

Commit a75aedf

Browse files
codexByron
authored andcommitted
fix(index): validate paths at native index boundaries
<!-- Byron --> rubber-stamp, after checking diff quickly. <!-- agent --> Native tree merging and index serialization could install paths that Git's own index writers reject. A subsequent checkout trusts that index and can write outside the working tree or into Git metadata (finding 21506 and the shared write-side cause of finding 21501). Validate complete paths when importing tree entries, reading or writing index entries, and building trees from the index. Reject traversal, absolute/drive paths, embedded NULs, and Git metadata aliases on NTFS/HFS while retaining ordinary POSIX filename characters. Validate before creating trees, and preserve index writer rollback on failure. Refresh the on-disk index before checkout, including when entries were cached. Parse full sentinel-length names and check their terminators and padding so validation sees the same paths as Git. Reject unsupported mandatory extensions such as split indexes: validating only their inline entries would overlook paths from external shared indexes. QA review follow-up: reject unsupported index versions with an explicit exception, preserving `AssertionError` for compatibility. Python's `-O` mode removed the former assertion, allowing version 4 prefix-compressed paths to be parsed with version 2/3 rules before native checkout. Validation must never accept a format whose paths Git interprets differently. Optimized subprocess regressions for unsupported versions fail before this fix and pass afterward. All 76 selected index tests pass, with two platform skips; locked Basedpyright, mypy, Codespell, and Ruff checks pass.
1 parent 428d205 commit a75aedf

4 files changed

Lines changed: 209 additions & 25 deletions

File tree

‎git/index/base.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,6 +1390,9 @@ def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLik
13901390

13911391
# END stderr handler
13921392

1393+
# Read and validate the index before Git trusts its paths for checkout.
1394+
self._delete_entries_cache()
1395+
self.entries # noqa: B018
13931396
if paths is None:
13941397
args.append("--all")
13951398
kwargs["as_process"] = 1

‎git/index/fun.py‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
traverse_trees_recursive,
3535
tree_to_stream,
3636
)
37-
from git.util import IndexFileSHA1Writer, finalize_process
37+
from git.util import IndexFileSHA1Writer, finalize_process, _validate_repo_path
3838

3939
from .typ import CE_EXTENDED, BaseIndexEntry, IndexEntry, CE_NAMEMASK, CE_STAGESHIFT
4040
from .util import pack, unpack
@@ -279,13 +279,13 @@ def write_cache(
279279

280280
# Body
281281
for entry in entries:
282+
_validate_repo_path(entry.path)
282283
beginoffset = tell()
283284
write(entry.ctime_bytes) # ctime
284285
write(entry.mtime_bytes) # mtime
285-
path_str = str(entry.path)
286+
path_str = os.fspath(entry.path)
286287
path: bytes = force_bytes(path_str, encoding=defenc)
287-
plen = len(path) & CE_NAMEMASK # Path length
288-
assert plen == len(path), "Path %s too long to fit into index" % entry.path
288+
plen = min(len(path), CE_NAMEMASK) # Longer names use a sentinel.
289289
flags = plen | (entry.flags & CE_NAMEMASK_INV) # Clear possible previous values.
290290
if entry.extended_flags:
291291
flags |= CE_EXTENDED
@@ -325,7 +325,8 @@ def read_header(stream: IO[bytes]) -> Tuple[int, int]:
325325
unpacked = cast(Tuple[int, int], unpack(">LL", stream.read(4 * 2)))
326326
version, num_entries = unpacked
327327

328-
assert version in (1, 2, 3), "Unsupported git index version %i, only 1, 2, and 3 are supported" % version
328+
if version not in (1, 2, 3):
329+
raise AssertionError("Unsupported git index version %i, only 1, 2, and 3 are supported" % version)
329330
return version, num_entries
330331

331332

@@ -382,10 +383,23 @@ def read_cache(
382383
if flags & CE_EXTENDED:
383384
extended_flags = unpack(">H", read(2))[0]
384385
path_size = flags & CE_NAMEMASK
385-
path = read(path_size).decode(defenc)
386-
387-
real_size = (tell() - beginoffset + 8) & ~7
388-
read((beginoffset + real_size) - tell())
386+
path_bytes = bytearray(read(path_size))
387+
if len(path_bytes) != path_size:
388+
raise ValueError("Truncated index entry path")
389+
terminator = read(1)
390+
if path_size == CE_NAMEMASK:
391+
while terminator and terminator != b"\0":
392+
path_bytes.extend(terminator)
393+
terminator = read(1)
394+
if terminator != b"\0":
395+
raise ValueError("Unterminated index entry path")
396+
path = path_bytes.decode(defenc)
397+
_validate_repo_path(path)
398+
399+
real_size = (tell() - beginoffset + 7) & ~7
400+
padding_size = beginoffset + real_size - tell()
401+
if read(padding_size) != b"\0" * padding_size:
402+
raise ValueError("Invalid index entry padding")
389403
entry = IndexEntry((mode, sha, flags, path, ctime, mtime, dev, ino, uid, gid, size, extended_flags))
390404
# entry_key would be the method to use, but we save the effort.
391405
entries[(path, entry.stage)] = entry
@@ -408,6 +422,18 @@ def read_cache(
408422
# Truncate the sha in the end as we will dynamically create it anyway.
409423
extension_data = extension_data[:-20]
410424

425+
offset = 0
426+
while offset < len(extension_data):
427+
header = extension_data[offset : offset + 8]
428+
if len(header) != 8:
429+
raise ValueError("Truncated index extension header")
430+
signature, size = unpack(">4sL", header)
431+
if not b"A" <= signature[:1] <= b"Z":
432+
raise ValueError("Unsupported mandatory index extension %r" % signature)
433+
offset += 8 + size
434+
if offset > len(extension_data):
435+
raise ValueError("Truncated index extension %r" % signature)
436+
411437
return (version, entries, extension_data, content_sha)
412438

413439

@@ -434,6 +460,9 @@ def write_tree_from_cache(
434460
435461
A tuple of a sha and a list of tree entries being a tuple of hexsha, mode, name.
436462
"""
463+
if si == 0:
464+
for entry in entries[sl]:
465+
_validate_repo_path(entry.path)
437466
tree_items: List["TreeCacheTup"] = []
438467

439468
ci = sl.start
@@ -481,6 +510,7 @@ def write_tree_from_cache(
481510

482511

483512
def _tree_entry_to_baseindexentry(tree_entry: "TreeCacheTup", stage: int) -> BaseIndexEntry:
513+
_validate_repo_path(tree_entry[2])
484514
return BaseIndexEntry((tree_entry[1], tree_entry[0], stage << CE_STAGESHIFT, tree_entry[2]))
485515

486516

‎git/util.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
from functools import wraps
3535
import getpass
3636
import logging
37+
import ntpath
3738
import os
3839
import os.path as osp
3940
from pathlib import Path
@@ -380,6 +381,33 @@ def _to_relative_path(root: PathLike, path: PathLike) -> str:
380381
return relative_path
381382

382383

384+
_HFS_IGNORABLES = str.maketrans(
385+
"", "", "\u200c\u200d\u200e\u200f\u202a\u202b\u202c\u202d\u202e\u206a\u206b\u206c\u206d\u206e\u206f\ufeff"
386+
)
387+
388+
389+
def _validate_repo_path(path: PathLike) -> None:
390+
"""Reject unsafe tree/index paths without normalizing away their components.
391+
392+
Protect Git metadata aliases on NTFS and HFS even when writing on another
393+
platform. Other POSIX filename characters, including newlines, remain valid.
394+
"""
395+
name = os.fspath(path)
396+
if not name or "\0" in name or ntpath.splitdrive(name)[0] or name.startswith("/"):
397+
raise ValueError("Invalid repository path %r" % name)
398+
if os.name == "nt" and "\\" in name:
399+
raise ValueError("Index paths must use '/' separators: %r" % name)
400+
for component in name.split("/"):
401+
if component in ("", ".", ".."):
402+
raise ValueError("Invalid repository path %r" % name)
403+
# NTFS recognizes backslashes and alternate data streams in metadata names.
404+
for part in component.split("\\"):
405+
ntfs_name = part.split(":", 1)[0].rstrip(" .").lower()
406+
hfs_name = part.translate(_HFS_IGNORABLES).lower()
407+
if ntfs_name in (".git", "git~1") or hfs_name == ".git":
408+
raise ValueError("Repository path aliases Git metadata: %r" % name)
409+
410+
383411
def assure_directory_exists(path: PathLike, is_file: bool = False) -> bool:
384412
"""Make sure that the directory pointed to by path exists.
385413

‎test/test_index.py‎

Lines changed: 139 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,28 @@
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

66
import contextlib
7-
from dataclasses import dataclass
8-
from io import BytesIO
97
import logging
108
import os
119
import os.path as osp
12-
from pathlib import Path
1310
import re
1411
import shutil
15-
from stat import S_ISLNK, ST_MODE
12+
import struct
1613
import subprocess
1714
import sys
1815
import tempfile
16+
from dataclasses import dataclass
17+
from hashlib import sha1
18+
from io import BytesIO
19+
from itertools import product
20+
from pathlib import Path
21+
from stat import S_ISLNK, ST_MODE
1922
from unittest import mock
2023

21-
from gitdb.base import IStream
22-
2324
import ddt
2425
import pytest
26+
from gitdb.base import IStream
2527

26-
from git import BlobFilter, Diff, Git, IndexFile, NULL_TREE, Object, Repo, Tree
28+
from git import NULL_TREE, BlobFilter, Diff, Git, IndexFile, Object, Repo, Tree
2729
from git.diff import NULL_TREE_SHA
2830
from git.exc import (
2931
CheckoutError,
@@ -33,13 +35,12 @@
3335
UnmergedEntriesError,
3436
UnsafeOptionError,
3537
)
36-
from git.index.fun import _git_for_windows_bash, _which_from_path, hook_path, run_commit_hook
38+
from git.index.fun import _git_for_windows_bash, _which_from_path, hook_path, read_cache, run_commit_hook, write_cache
3739
from git.index.typ import BaseIndexEntry, IndexEntry
3840
from git.index.util import TemporaryFileSwap
3941
from git.objects import Blob
4042
from git.util import Actor, cwd, hex_to_bin, rmtree
41-
42-
from test.lib import TestBase, VirtualEnvironment, fixture, fixture_path, with_rw_directory, with_rw_repo, PathLikeMock
43+
from test.lib import PathLikeMock, TestBase, VirtualEnvironment, fixture, fixture_path, with_rw_directory, with_rw_repo
4344
from test.lib.helper import symlinks_supported, xfail_if_raises
4445

4546
HOOKS_SHEBANG = "#!/usr/bin/env sh\n"
@@ -188,6 +189,15 @@ def _make_hook(git_dir, name, content, make_exec=True):
188189
return hp
189190

190191

192+
def _raw_index(path):
193+
"""Build an index without using the writer under test."""
194+
name = path.encode("utf-8")
195+
entry = struct.pack(">10L20sH", 0, 0, 0, 0, 0, 0, 0o100644, 0, 0, 0, b"a" * 20, min(len(name), 0xFFF)) + name
196+
entry += b"\0" * (8 - len(entry) % 8)
197+
data = b"DIRC" + struct.pack(">LL", 2, 1) + entry
198+
return data + sha1(data).digest()
199+
200+
191201
@ddt.ddt
192202
class TestIndex(TestBase):
193203
@with_rw_repo("HEAD")
@@ -303,6 +313,54 @@ def test_index_file_base(self):
303313
self.assertEqual(fp.read(), fixture("index_merge"))
304314
os.remove(tmpfile)
305315

316+
@ddt.data(
317+
"",
318+
".",
319+
"..",
320+
"../outside",
321+
"a/../outside",
322+
"/absolute",
323+
"C:relative",
324+
"a//b",
325+
"a/./b",
326+
"a/",
327+
"nul\0name",
328+
".git/config",
329+
"a/.GiT/hooks/hook",
330+
"git~1/config",
331+
".git. /config",
332+
".git:stream",
333+
".g\u200cit/config",
334+
"a\\.git\\config",
335+
)
336+
def test_index_reader_and_writer_reject_unsafe_paths(self, path):
337+
with pytest.raises(ValueError):
338+
read_cache(BytesIO(_raw_index(path)))
339+
entry = IndexEntry((0o100644, b"a" * 20, 0, path))
340+
with pytest.raises(ValueError):
341+
write_cache([entry], BytesIO())
342+
343+
def test_valid_unusual_index_names_round_trip(self):
344+
names = ["a b", "a\nb", "a\tb", "name:value", "dir/.gitignore", "café"]
345+
if os.name != "nt":
346+
names.append("a\\b")
347+
entries = [IndexEntry((0o100644, b"a" * 20, 0, name)) for name in sorted(names)]
348+
stream = BytesIO()
349+
write_cache(entries, stream)
350+
stream.seek(0)
351+
assert [entry.path for entry in read_cache(stream)[1].values()] == sorted(names)
352+
353+
def test_long_index_names_are_fully_validated(self):
354+
prefix = "a/" + "nested/" * 650
355+
with pytest.raises(ValueError):
356+
read_cache(BytesIO(_raw_index(prefix + "../outside")))
357+
name = prefix + "file"
358+
assert next(iter(read_cache(BytesIO(_raw_index(name)))[1])) == (name, 0)
359+
stream = BytesIO()
360+
write_cache([IndexEntry((0o100644, b"a" * 20, 0, name))], stream)
361+
stream.seek(0)
362+
assert next(iter(read_cache(stream)[1])) == (name, 0)
363+
306364
def _cmp_tree_index(self, tree, index):
307365
# Fail unless both objects contain the same paths and blobs.
308366
if isinstance(tree, str):
@@ -637,6 +695,30 @@ def _count_existing(self, repo, files):
637695

638696
# END num existing helper
639697

698+
@ddt.data("write", "write_tree", "checkout", "cached_checkout")
699+
@with_rw_directory
700+
def test_index_boundaries_reject_injected_entries_before_side_effects(self, rw_dir, operation):
701+
tmp_path = Path(rw_dir) / "repo"
702+
with Repo.init(tmp_path) as repo:
703+
index_path = Path(repo.index.path)
704+
if operation == "checkout":
705+
index_path.write_bytes(_raw_index("../outside"))
706+
else:
707+
repo.index.write()
708+
before = index_path.read_bytes()
709+
index = repo.index
710+
if operation == "cached_checkout":
711+
assert not index.entries
712+
index_path.write_bytes(_raw_index("../outside"))
713+
before = index_path.read_bytes()
714+
operation = "checkout"
715+
elif operation != "checkout":
716+
index.entries[("../outside", 0)] = IndexEntry((0o100644, b"a" * 20, 0, "../outside"))
717+
with pytest.raises(ValueError):
718+
getattr(index, operation)()
719+
assert index_path.read_bytes() == before
720+
assert not (tmp_path.parent / "outside").exists()
721+
640722
@with_rw_repo("0.1.6")
641723
def test_index_mutation(self, rw_repo):
642724
with xfail_if_raises(
@@ -1035,6 +1117,20 @@ def test_index_new(self):
10351117
assert isinstance(index, IndexFile)
10361118
# END for each arg tuple
10371119

1120+
@ddt.data(*product(("../outside", ".git/hooks/pre-commit", "a/.GIT/config"), (1, 2, 3)))
1121+
@ddt.unpack
1122+
@with_rw_directory
1123+
def test_native_tree_merge_rejects_unsafe_paths(self, rw_dir, path, tree_count):
1124+
tmp_path = Path(rw_dir)
1125+
with Repo.init(tmp_path) as repo:
1126+
blob = repo.odb.store(IStream("blob", 4, BytesIO(b"data"))).binsha
1127+
data = b"100755 " + path.encode() + b"\0" + blob
1128+
tree = repo.odb.store(IStream("tree", len(data), BytesIO(data))).binsha
1129+
empty = repo.odb.store(IStream("tree", 0, BytesIO())).binsha
1130+
with pytest.raises(ValueError):
1131+
IndexFile.new(repo, *([empty] * (tree_count - 1) + [tree]))
1132+
assert not (tmp_path / ".git" / "index").exists()
1133+
10381134
@with_rw_repo("HEAD", bare=True)
10391135
def test_index_bare_add(self, rw_bare_repo):
10401136
# Something is wrong after cloning to a bare repo, reading the property
@@ -1433,34 +1529,61 @@ def test_commit_msg_hook_fail(self, rw_repo):
14331529

14341530
@with_rw_repo("HEAD")
14351531
def test_index_add_pathlib(self, rw_repo):
1436-
git_dir = Path(rw_repo.git_dir)
1532+
worktree = Path(rw_repo.working_tree_dir)
14371533

1438-
file = git_dir / "file.txt"
1534+
file = worktree / "file.txt"
14391535
file.touch()
14401536

14411537
rw_repo.index.add(file)
14421538

14431539
@with_rw_repo("HEAD")
14441540
def test_index_add_pathlike(self, rw_repo):
1445-
git_dir = Path(rw_repo.git_dir)
1541+
worktree = Path(rw_repo.working_tree_dir)
14461542

1447-
file = git_dir / "file.txt"
1543+
file = worktree / "file.txt"
14481544
file.touch()
14491545

14501546
rw_repo.index.add(PathLikeMock(str(file)))
14511547

14521548
@with_rw_repo("HEAD")
14531549
def test_index_add_non_normalized_path(self, rw_repo):
1454-
git_dir = Path(rw_repo.git_dir)
1550+
worktree = Path(rw_repo.working_tree_dir)
14551551

1456-
file = git_dir / "file.txt"
1552+
file = worktree / "file.txt"
14571553
file.touch()
14581554
non_normalized_path = file.as_posix()
14591555
if os.name != "nt":
14601556
non_normalized_path = "/" + non_normalized_path[1:].replace("/", "//")
14611557

14621558
rw_repo.index.add(non_normalized_path)
14631559

1560+
@ddt.data(0, 4, 5)
1561+
def test_unsupported_index_versions_fail_even_with_optimization(self, version):
1562+
data = b"DIRC" + struct.pack(">LL", version, 0)
1563+
data += sha1(data).digest()
1564+
with pytest.raises(AssertionError, match="Unsupported git index version"):
1565+
read_cache(BytesIO(data))
1566+
code = """
1567+
from io import BytesIO
1568+
import sys
1569+
from git.index.fun import read_cache
1570+
try:
1571+
read_cache(BytesIO(sys.stdin.buffer.read()))
1572+
except AssertionError as error:
1573+
if "Unsupported git index version" not in str(error):
1574+
raise
1575+
else:
1576+
raise SystemExit("Unsupported index version was accepted")
1577+
"""
1578+
result = subprocess.run([sys.executable, "-O", "-c", code], input=data, capture_output=True, timeout=10)
1579+
assert result.returncode == 0, result.stderr.decode()
1580+
1581+
@ddt.data(b"link", b"sdir")
1582+
def test_unsupported_mandatory_index_extensions_fail_closed(self, signature):
1583+
data = b"DIRC" + struct.pack(">LL", 2, 0) + signature + struct.pack(">L", 0)
1584+
with pytest.raises(ValueError, match="extension"):
1585+
read_cache(BytesIO(data + sha1(data).digest()))
1586+
14641587
def test_index_file_v3(self):
14651588
index = IndexFile(self.rorepo, fixture_path("index_extended_flags"))
14661589
assert index.entries

0 commit comments

Comments
 (0)