Skip to content

Commit

Permalink
Start on test_hook_uses_shell_not_from_cwd
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EliahKagan committed Jan 9, 2024
1 parent d2506c7 commit 61b4dda
Showing 1 changed file with 36 additions and 1 deletion.
37 changes: 36 additions & 1 deletion test/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@
# This module is part of GitPython and is released under the
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/

import contextlib
from io import BytesIO
import logging
import os
import os.path as osp
from pathlib import Path
import re
import shutil
from stat import S_ISLNK, ST_MODE
import subprocess
import tempfile

import ddt
import pytest
from sumtypes import constructor, sumtype

Expand All @@ -36,7 +39,7 @@
from git.index.typ import BaseIndexEntry, IndexEntry
from git.index.util import TemporaryFileSwap
from git.objects import Blob
from git.util import Actor, hex_to_bin, rmtree
from git.util import Actor, cwd, hex_to_bin, rmtree
from gitdb.base import IStream
from test.lib import TestBase, fixture, fixture_path, with_rw_directory, with_rw_repo

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


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

# FIXME: Figure out a way to make this test also work with Absent and WslNoDistro.
@pytest.mark.xfail(
type(_win_bash_status) is WinBashStatus.WslNoDistro,
reason="Currently uses the bash.exe of WSL, even with no WSL distro installed",
raises=HookExecutionError,
)
@ddt.data((False,), (True,))
@with_rw_directory
def test_hook_uses_shell_not_from_cwd(self, rw_dir, case):
(chdir_to_repo,) = case

repo = Repo.init(rw_dir)
_make_hook(repo.git_dir, "fake-hook", "echo 'ran fake hook' >output.txt")

if os.name == "nt":
# Copy an actual binary that is not bash.
other_exe_path = Path(os.environ["SystemRoot"], "system32", "hostname.exe")
impostor_path = Path(rw_dir, "bash.exe")
shutil.copy(other_exe_path, impostor_path)
else:
# Create a shell script that doesn't do anything.
impostor_path = Path(rw_dir, "sh")
impostor_path.write_text("#!/bin/sh\n", encoding="utf-8")
os.chmod(impostor_path, 0o755)

with cwd(rw_dir) if chdir_to_repo else contextlib.nullcontext():
run_commit_hook("fake-hook", repo.index)

output = Path(rw_dir, "output.txt").read_text(encoding="utf-8")
self.assertEqual(output, "ran fake hook\n")

@pytest.mark.xfail(
type(_win_bash_status) is WinBashStatus.Absent,
reason="Can't run a hook on Windows without bash.exe.",
Expand Down

0 comments on commit 61b4dda

Please sign in to comment.