Skip to content

Commit 61b4dda

Browse files
committed
Start on test_hook_uses_shell_not_from_cwd
This shows that run_commit_hook is vulnerable to an untrusted search path bug on Windows, when running script hooks: bash.exe is run without setting NoDefaultCurrentDirectoryInExePath or otherwise excluding the current directory from the path search. The new test uses a non-bare repo, even though the surrounding tests use bare repos. Although the test also works if the repo is initialized with `Repo.init(rw_dir, bare=True)`, using a bare repo would obscure how the bug this test reveals would typically be exploited, where a command that uses a hook is run after a malicious bash.exe is checked out into the working tree from an untrusted branch. Running hooks that are themselves provided by an untrusted repository or branch is of course never safe. If an attacker can deceive a user into doing that, then this vulnerability is not needed. Instead, an attack that leverages this untrusted search path vulnerability would most likely be of roughly this form: 1. The user clones a trusted repository and installs hooks. 2. A malicious party offers a contribution to the project. 3. The user checks out the malicious party's untrusted branch. 4. The user performs an action that runs a hook. The hook the user runs is still trusted, but it runs with the malicious bash.exe found in the current directory (which is the working tree or perhaps some subdirectory of it). The test added in this commit should, if possible, be improved to be able to run and detect the bug (or its absence) even when bash is absent from the Windows system and, preferably, also even when the WSL bash.exe is present but no WSL distribution is installed.
1 parent d2506c7 commit 61b4dda

File tree

1 file changed

+36
-1
lines changed

1 file changed

+36
-1
lines changed

Diff for: test/test_index.py

+36-1
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,19 @@
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+
import contextlib
67
from io import BytesIO
78
import logging
89
import os
910
import os.path as osp
1011
from pathlib import Path
1112
import re
13+
import shutil
1214
from stat import S_ISLNK, ST_MODE
1315
import subprocess
1416
import tempfile
1517

18+
import ddt
1619
import pytest
1720
from sumtypes import constructor, sumtype
1821

@@ -36,7 +39,7 @@
3639
from git.index.typ import BaseIndexEntry, IndexEntry
3740
from git.index.util import TemporaryFileSwap
3841
from git.objects import Blob
39-
from git.util import Actor, hex_to_bin, rmtree
42+
from git.util import Actor, cwd, hex_to_bin, rmtree
4043
from gitdb.base import IStream
4144
from test.lib import TestBase, fixture, fixture_path, with_rw_directory, with_rw_repo
4245

@@ -172,6 +175,7 @@ def _make_hook(git_dir, name, content, make_exec=True):
172175
return hp
173176

174177

178+
@ddt.ddt
175179
class TestIndex(TestBase):
176180
def __init__(self, *args):
177181
super().__init__(*args)
@@ -1012,6 +1016,37 @@ def test_run_commit_hook(self, rw_repo):
10121016
output = Path(rw_repo.git_dir, "output.txt").read_text(encoding="utf-8")
10131017
self.assertEqual(output, "ran fake hook\n")
10141018

1019+
# FIXME: Figure out a way to make this test also work with Absent and WslNoDistro.
1020+
@pytest.mark.xfail(
1021+
type(_win_bash_status) is WinBashStatus.WslNoDistro,
1022+
reason="Currently uses the bash.exe of WSL, even with no WSL distro installed",
1023+
raises=HookExecutionError,
1024+
)
1025+
@ddt.data((False,), (True,))
1026+
@with_rw_directory
1027+
def test_hook_uses_shell_not_from_cwd(self, rw_dir, case):
1028+
(chdir_to_repo,) = case
1029+
1030+
repo = Repo.init(rw_dir)
1031+
_make_hook(repo.git_dir, "fake-hook", "echo 'ran fake hook' >output.txt")
1032+
1033+
if os.name == "nt":
1034+
# Copy an actual binary that is not bash.
1035+
other_exe_path = Path(os.environ["SystemRoot"], "system32", "hostname.exe")
1036+
impostor_path = Path(rw_dir, "bash.exe")
1037+
shutil.copy(other_exe_path, impostor_path)
1038+
else:
1039+
# Create a shell script that doesn't do anything.
1040+
impostor_path = Path(rw_dir, "sh")
1041+
impostor_path.write_text("#!/bin/sh\n", encoding="utf-8")
1042+
os.chmod(impostor_path, 0o755)
1043+
1044+
with cwd(rw_dir) if chdir_to_repo else contextlib.nullcontext():
1045+
run_commit_hook("fake-hook", repo.index)
1046+
1047+
output = Path(rw_dir, "output.txt").read_text(encoding="utf-8")
1048+
self.assertEqual(output, "ran fake hook\n")
1049+
10151050
@pytest.mark.xfail(
10161051
type(_win_bash_status) is WinBashStatus.Absent,
10171052
reason="Can't run a hook on Windows without bash.exe.",

0 commit comments

Comments
 (0)