diff --git a/security/tar_safe.py b/security/tar_safe.py index 2aa3c3ad..5474f859 100644 --- a/security/tar_safe.py +++ b/security/tar_safe.py @@ -130,30 +130,24 @@ def _validate_member( target_path = self._get_safe_path(member, extract_path) - # Check for path traversal + # Check for path traversal using Path.relative_to() try: target_abs = target_path.resolve() extract_abs = extract_path.resolve() - if os.path.commonpath([str(target_abs), str(extract_abs)]) != str(extract_abs): - raise UnsafeTarError( - f"Path traversal detected: {member.name} -> {target_path}" - ) - except (OSError, RuntimeError): - target_str = str(target_path.absolute()) - extract_str = str(extract_path.absolute()) - try: - if os.path.commonpath([target_str, extract_str]) != extract_str: - raise UnsafeTarError( - f"Path traversal detected: {member.name} -> {target_path}" - ) - except ValueError: - raise UnsafeTarError( - f"Path traversal detected (different drives): {member.name} -> {target_path}" - ) - except ValueError: - # os.path.commonpath raises ValueError on Windows for different drives + target_abs.relative_to(extract_abs) + except (ValueError, OSError, RuntimeError): raise UnsafeTarError( - f"Path traversal detected (different drives): {member.name} -> {target_path}" + f"Path traversal detected: {member.name} -> {target_path}" + ) + + if target_path.exists() and not overwrite: + raise UnsafeTarError( + f"File already exists and overwrite=False: {target_path}" + ) + + if member.type in self.blocked_types: + raise UnsafeTarError( + f"Blocked file type for {member.name}: {member.type}" ) if target_path.exists() and not overwrite: diff --git a/tests/test_security.py b/tests/test_security.py index cab638b9..1d6759d7 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -174,7 +174,27 @@ def test_sibling_directory_traversal(self, tmp_path): with pytest.raises(UnsafeTarError): extractor.extract(tar_file, extract_dir) - def test_absolute_path_traversal(self, tmp_path): + def test_prefix_bypass_path_traversal(self, tmp_path): + """Test that prefix-based bypass (e.g., /tmp/a vs /tmp/ab) is caught.""" + tar_file = tmp_path / "prefix_bypass.tar" + + with tarfile.open(tar_file, 'w') as tar: + info = tarfile.TarInfo(name='../ab/evil.txt') + info.size = 10 + info.type = tarfile.REGTYPE + content = b'x' * 10 + fileobj = io.BytesIO(content) + tar.addfile(info, fileobj) + + extract_dir = tmp_path / "a" + extract_dir.mkdir(exist_ok=True) + sibling_dir = tmp_path / "ab" + sibling_dir.mkdir(exist_ok=True) + + extractor = SafeTarExtractor() + + with pytest.raises(UnsafeTarError): + extractor.extract(tar_file, extract_dir) """Test absolute path traversal prevention.""" tar_file = tmp_path / "absolute_traversal.tar"