Validate log_file paths to prevent arbitrary file writes

A hook manifest can specify a `log_file` with an absolute path or path
traversal sequences (e.g. `../../../etc/cron.d/malicious`), causing
pre-commit to write hook output to arbitrary locations on the host
filesystem via `output.write_line_b`.

Reject absolute paths and paths that traverse above the working directory
during manifest validation.

Fixes #3655
This commit is contained in:
ran 2026-04-11 12:17:27 +08:00
parent 5c0f3024d2
commit dd5ba1ab00
2 changed files with 37 additions and 1 deletions

View file

@ -23,6 +23,22 @@ logger = logging.getLogger('pre_commit')
check_string_regex = cfgv.check_and(cfgv.check_string, cfgv.check_regex) check_string_regex = cfgv.check_and(cfgv.check_string, cfgv.check_regex)
def _check_log_file(val: str) -> None:
if val == '':
return
if os.path.isabs(val):
raise cfgv.ValidationError(
f'log_file must be a relative path, got absolute path: {val!r}',
)
if os.path.normpath(val).startswith('..'):
raise cfgv.ValidationError(
f'log_file must not reference a parent directory: {val!r}',
)
check_log_file = cfgv.check_and(cfgv.check_string, _check_log_file)
HOOK_TYPES = ( HOOK_TYPES = (
'commit-msg', 'commit-msg',
'post-checkout', 'post-checkout',
@ -258,7 +274,7 @@ MANIFEST_HOOK_DICT = cfgv.Map(
cfgv.Optional('pass_filenames', cfgv.check_bool, True), cfgv.Optional('pass_filenames', cfgv.check_bool, True),
cfgv.Optional('description', cfgv.check_string, ''), cfgv.Optional('description', cfgv.check_string, ''),
cfgv.Optional('language_version', cfgv.check_string, C.DEFAULT), cfgv.Optional('language_version', cfgv.check_string, C.DEFAULT),
cfgv.Optional('log_file', cfgv.check_string, ''), cfgv.Optional('log_file', check_log_file, ''),
cfgv.Optional('require_serial', cfgv.check_bool, False), cfgv.Optional('require_serial', cfgv.check_bool, False),
StagesMigration('stages', []), StagesMigration('stages', []),
cfgv.Optional('verbose', cfgv.check_bool, False), cfgv.Optional('verbose', cfgv.check_bool, False),

View file

@ -7,6 +7,7 @@ import cfgv
import pytest import pytest
import pre_commit.constants as C import pre_commit.constants as C
from pre_commit.clientlib import _check_log_file
from pre_commit.clientlib import check_type_tag from pre_commit.clientlib import check_type_tag
from pre_commit.clientlib import CONFIG_HOOK_DICT from pre_commit.clientlib import CONFIG_HOOK_DICT
from pre_commit.clientlib import CONFIG_REPO_DICT from pre_commit.clientlib import CONFIG_REPO_DICT
@ -605,3 +606,22 @@ def test_manifest_v5_forward_compat(tmp_path):
f'=====> pre-commit version 5 is required but version {C.VERSION} ' f'=====> pre-commit version 5 is required but version {C.VERSION} '
f'is installed. Perhaps run `pip install --upgrade pre-commit`.' f'is installed. Perhaps run `pip install --upgrade pre-commit`.'
) )
@pytest.mark.parametrize('value', ('output.log', 'logs/hook.log', ''))
def test_check_log_file_valid(value):
_check_log_file(value)
@pytest.mark.parametrize(
'value',
(
'/tmp/evil.log',
'/etc/cron.d/malicious',
'../../../etc/passwd',
'../outside.log',
),
)
def test_check_log_file_invalid(value):
with pytest.raises(cfgv.ValidationError):
_check_log_file(value)