Skip to content

Commit a5285cd

Browse files
claudeben-edna
authored andcommitted
Security: fix Windows backslash traversal in symlink checks, rename constant
- archive.py: extract _is_unsafe_symlink_target() helper that checks both PurePosixPath and PureWindowsPath so targets like "..\\..\\evil" are caught on any host OS; apply it to both _check_zip_member_type and _check_tar_member_type, removing the duplicated inline check. Rename _MAX_SYMLINK_TARGET → max_symlink_target to satisfy pylint C0103 (local variable vs. module-level constant naming convention). - test_archive.py: add Windows-backslash traversal cases for both ZIP (test_check_zip_members_symlink_windows_dotdot_target) and TAR (test_check_tar_member_type_windows_dotdot_symlink). https://claude.ai/code/session_01CxKrTZekNRCoSD9PSMekQZ
1 parent 2d1ace7 commit a5285cd

2 files changed

Lines changed: 37 additions & 9 deletions

File tree

dfetch/vcs/archive.py

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,23 @@ def check_zip_members(zf: zipfile.ZipFile) -> list[zipfile.ZipInfo]:
366366
ArchiveLocalRepo._check_zip_member_type(info, zf)
367367
return members
368368

369+
@staticmethod
370+
def _is_unsafe_symlink_target(target: str) -> bool:
371+
"""Return *True* when *target* is an unsafe symlink destination.
372+
373+
A target is unsafe if it is absolute or contains ``..`` components
374+
under either POSIX or Windows path semantics, so that backslash-based
375+
traversals like ``..\\..\\evil`` are caught on any host OS.
376+
"""
377+
posix = pathlib.PurePosixPath(target)
378+
win = pathlib.PureWindowsPath(target)
379+
return (
380+
posix.is_absolute()
381+
or win.is_absolute()
382+
or any(part == ".." for part in posix.parts)
383+
or any(part == ".." for part in win.parts)
384+
)
385+
369386
@staticmethod
370387
def _check_zip_member_type(info: zipfile.ZipInfo, zf: zipfile.ZipFile) -> None:
371388
"""Reject dangerous ZIP member types (mirrors :meth:`_check_tar_member_type`).
@@ -377,20 +394,18 @@ def _check_zip_member_type(info: zipfile.ZipInfo, zf: zipfile.ZipFile) -> None:
377394
Raises:
378395
RuntimeError: When *info* is a symlink with an unsafe target.
379396
"""
380-
_MAX_SYMLINK_TARGET = 4096
397+
max_symlink_target = 4096
381398
unix_mode = info.external_attr >> 16
382399
if stat.S_ISLNK(unix_mode):
383400
with zf.open(info) as member_file:
384-
raw = member_file.read(_MAX_SYMLINK_TARGET + 1)
385-
if len(raw) > _MAX_SYMLINK_TARGET:
401+
raw = member_file.read(max_symlink_target + 1)
402+
if len(raw) > max_symlink_target:
386403
raise RuntimeError(
387404
f"Archive contains a symlink with an oversized target: "
388405
f"{info.filename!r}"
389406
)
390407
target = raw.decode(errors="replace")
391-
if os.path.isabs(target) or any(
392-
part == ".." for part in pathlib.PurePosixPath(target).parts
393-
):
408+
if ArchiveLocalRepo._is_unsafe_symlink_target(target):
394409
raise RuntimeError(
395410
f"Archive contains a symlink with an unsafe target: "
396411
f"{info.filename!r} -> {target!r}"
@@ -422,9 +437,7 @@ def _check_tar_member_type(member: tarfile.TarInfo) -> None:
422437
"""
423438
if member.issym():
424439
target = member.linkname
425-
if os.path.isabs(target) or any(
426-
part == ".." for part in pathlib.PurePosixPath(target).parts
427-
):
440+
if ArchiveLocalRepo._is_unsafe_symlink_target(target):
428441
raise RuntimeError(
429442
f"Archive contains a symlink with an unsafe target: "
430443
f"{member.name!r} -> {target!r}"

tests/test_archive.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,13 @@ def test_check_zip_members_symlink_dotdot_target():
162162
_check_zip_members(zf)
163163

164164

165+
def test_check_zip_members_symlink_windows_dotdot_target():
166+
"""ZIP symlink with Windows-style backslash traversal must be rejected."""
167+
zf = _make_zip_with_symlink("link", "..\\..\\evil")
168+
with pytest.raises(RuntimeError, match="symlink"):
169+
_check_zip_members(zf)
170+
171+
165172
def test_check_zip_members_symlink_safe_relative():
166173
"""ZIP symlink with a safe relative target must be accepted (mirrors TAR behaviour)."""
167174
zf = _make_zip_with_symlink("link", "relative/safe/target")
@@ -268,6 +275,14 @@ def test_check_tar_member_type_dotdot_symlink():
268275
_check_tar_member_type(member)
269276

270277

278+
def test_check_tar_member_type_windows_dotdot_symlink():
279+
"""TAR symlink with Windows-style backslash traversal must be rejected."""
280+
tf = _make_tar_with_member(lambda t: _add_symlink(t, "link", "..\\..\\evil"))
281+
member = tf.getmembers()[0]
282+
with pytest.raises(RuntimeError, match="unsafe target"):
283+
_check_tar_member_type(member)
284+
285+
271286
# ---------------------------------------------------------------------------
272287
# _check_tar_member_type — hardlink validation
273288
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)