diff --git a/CLAUDE.md b/CLAUDE.md index 2aff0139..b1c7d0b3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,6 +68,15 @@ make worker # Start Launchpad worker (TaskWorker mode) - **Empty `__init__.py` files** preferred - **Comments**: Only add comments for complex logic or important business decisions +## Security: Path Traversal Prevention + +When handling paths derived from untrusted input (zip member names, plist values, manifest entries, user-supplied filenames), use `SafeDirectory` from `launchpad.artifacts.providers.safe_directory` — never use raw `Path` operations. + +- **`SafeDirectory(base).resolve(untrusted)`** — validates and returns a safe `Path`, raises `UnsafePathError` on traversal attempts +- **`SafeDirectory(base).child(untrusted)`** — returns a new `SafeDirectory` scoped to a validated subdirectory +- Trusted operations (glob, `/`, iterdir) are delegated directly and don't need validation +- Import `UnsafePathError` from `launchpad.artifacts.providers.exceptions` + ## Testing Strategy - **Prefer integration tests** using real artifacts from `tests/_fixtures/` over unit tests with mocks diff --git a/src/launchpad/artifacts/android/aab.py b/src/launchpad/artifacts/android/aab.py index 332c191b..f13a7190 100644 --- a/src/launchpad/artifacts/android/aab.py +++ b/src/launchpad/artifacts/android/aab.py @@ -19,7 +19,8 @@ from launchpad.utils.logging import get_logger from ..artifact import AndroidArtifact -from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path +from ..providers.exceptions import UnsafePathError +from ..providers.zip_provider import ZipProvider from .apk import APK from .manifest.manifest import AndroidManifest from .manifest.proto_xml import ProtoXmlUtils @@ -156,11 +157,8 @@ def get_app_icon(self) -> bytes | None: logger.info("No icon path found in manifest") return None - base_dir = self._extract_dir / "base" - if not is_safe_path(base_dir, icon_path_str): - raise UnsafePathError(f"Unsafe icon path in manifest: {icon_path_str}") - - icon_path = base_dir / icon_path_str + base_dir = self._extract_dir.child("base") + icon_path = base_dir.resolve(icon_path_str) if not icon_path.exists(): logger.info(f"Icon not found in AAB: {icon_path_str}") @@ -171,7 +169,7 @@ def get_app_icon(self) -> bytes | None: try: proto_res_tables = self.get_resource_tables() - proto_xml_drawable_parser = ProtoXmlDrawableParser(self._extract_dir / "base", proto_res_tables) + proto_xml_drawable_parser = ProtoXmlDrawableParser(self._extract_dir.child("base"), proto_res_tables) icon = proto_xml_drawable_parser.render_from_path(icon_path) if icon: diff --git a/src/launchpad/artifacts/android/apk.py b/src/launchpad/artifacts/android/apk.py index 436180f8..204f3a9f 100644 --- a/src/launchpad/artifacts/android/apk.py +++ b/src/launchpad/artifacts/android/apk.py @@ -18,7 +18,9 @@ from ...parsers.android.dex.types import ClassDefinition from ...utils.logging import get_logger from ..artifact import AndroidArtifact -from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path +from ..providers.exceptions import UnsafePathError +from ..providers.safe_directory import SafeDirectory +from ..providers.zip_provider import ZipProvider from .manifest.axml import AxmlUtils from .manifest.manifest import AndroidManifest from .resources.binary import BinaryResourceTable @@ -109,7 +111,7 @@ def get_class_definitions(self) -> list[ClassDefinition]: return self._class_definitions - def get_extract_path(self) -> Path: + def get_extract_path(self) -> SafeDirectory: return self._extract_dir def get_apksigner_certs(self) -> str: @@ -128,10 +130,7 @@ def get_app_icon(self) -> bytes | None: logger.info("No icon path found in manifest") return None - if not is_safe_path(self._extract_dir, icon_path_str): - raise UnsafePathError(f"Unsafe icon path in manifest: {icon_path_str}") - - icon_path = self._extract_dir / icon_path_str + icon_path = self._extract_dir.resolve(icon_path_str) if not icon_path.exists(): logger.info(f"Icon not found in APK: {icon_path_str}") diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index c6f31e18..88cb1d7d 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -1,5 +1,4 @@ import json -import os import plistlib import shutil import subprocess @@ -17,7 +16,9 @@ from launchpad.utils.logging import get_logger from ..artifact import AppleArtifact -from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path +from ..providers.exceptions import UnsafePathError +from ..providers.safe_directory import SafeDirectory +from ..providers.zip_provider import ZipProvider logger = get_logger(__name__) @@ -66,7 +67,7 @@ def __init__(self, path: Path) -> None: super().__init__(path) self._zip_provider = ZipProvider(path) self._extract_dir = self._zip_provider.extract_to_temp_directory() - self._app_bundle_path: Path | None = None + self._app_bundle_path: SafeDirectory | None = None self._plist: dict[str, Any] | None = None self._archive_plist: dict[str, Any] | None = None self._provisioning_profile: dict[str, Any] | None = None @@ -74,7 +75,7 @@ def __init__(self, path: Path) -> None: self._binary_uuid_cache: dict[Path, str] = {} self._lief_cache: dict[Path, lief.MachO.FatBinary] = {} - def get_extract_dir(self) -> Path: + def get_extract_dir(self) -> SafeDirectory: return self._extract_dir @sentry_sdk.trace @@ -127,16 +128,15 @@ def get_app_icon(self) -> bytes | None: logger.warning("No icon files found in CFBundleIconFiles") return None - app_bundle_path = self.get_app_bundle_path() + app_bundle = self.get_app_bundle_path() for icon_name in icon_info.primary_icon_files: - if not is_safe_path(app_bundle_path, icon_name): - raise UnsafePathError(f"Unsafe icon name in plist: {icon_name}") + app_bundle.resolve(icon_name) # iOS lists base names without extensions or resolution modifiers (@2x, @3x, ~ipad) # Search for files matching the base name with any suffix # e.g., "AppIcon60x60" matches "AppIcon60x60@2x.png" or "AppIcon60x60.png" - matching_files = list(app_bundle_path.glob(f"{icon_name}*.png")) + matching_files = list(app_bundle.glob(f"{icon_name}*.png")) if not matching_files: continue @@ -287,13 +287,10 @@ def get_binary_path(self) -> Path | None: if not executable_name: return None - if not is_safe_path(app_bundle_path, executable_name): - raise UnsafePathError(f"Unsafe CFBundleExecutable in plist: {executable_name}") - - return app_bundle_path / executable_name + return app_bundle_path.resolve(executable_name) @sentry_sdk.trace - def get_app_bundle_path(self) -> Path: + def get_app_bundle_path(self) -> SafeDirectory: """Get the path to the .app bundle.""" if self._app_bundle_path is not None: return self._app_bundle_path @@ -301,7 +298,8 @@ def get_app_bundle_path(self) -> Path: for path in self._extract_dir.rglob("*.xcarchive/Products/**/*.app"): if path.is_dir() and "__MACOSX" not in str(path): logger.debug(f"Found Apple app bundle: {path}") - return path + self._app_bundle_path = SafeDirectory(path) + return self._app_bundle_path raise FileNotFoundError(f"No .app bundle found in {self._extract_dir}") @@ -403,7 +401,7 @@ def get_asset_catalog_details(self, relative_path: Path) -> List[AssetCatalogEle logger.exception(f"Failed to get asset catalog details for {relative_path}") return [] - def _discover_framework_binaries(self, app_bundle_path: Path) -> List[Path]: + def _discover_framework_binaries(self, app_bundle_path: SafeDirectory) -> List[Path]: framework_binaries: List[Path] = [] for framework_path in app_bundle_path.rglob("*.framework"): if framework_path.is_dir(): @@ -415,7 +413,7 @@ def _discover_framework_binaries(self, app_bundle_path: Path) -> List[Path]: logger.warning("Framework binary not found", extra={"path": framework_binary_path}) return framework_binaries - def _discover_extension_binaries(self, app_bundle_path: Path) -> List[Path]: + def _discover_extension_binaries(self, app_bundle_path: SafeDirectory) -> List[Path]: extension_binaries: List[Path] = [] for extension_path in app_bundle_path.rglob("*.appex"): if extension_path.is_dir(): @@ -426,11 +424,7 @@ def _discover_extension_binaries(self, app_bundle_path: Path) -> List[Path]: extension_plist = plistlib.load(f) extension_executable = extension_plist.get("CFBundleExecutable") if extension_executable: - if not is_safe_path(extension_path, extension_executable): - raise UnsafePathError( - f"Unsafe CFBundleExecutable in extension plist: {extension_executable}" - ) - extension_binary_path = extension_path / extension_executable + extension_binary_path = SafeDirectory(extension_path).resolve(extension_executable) if extension_binary_path.exists(): extension_binaries.append(extension_binary_path) else: @@ -441,7 +435,7 @@ def _discover_extension_binaries(self, app_bundle_path: Path) -> List[Path]: logger.exception(f"Failed to read extension Info.plist at {extension_path}") return extension_binaries - def _discover_watch_binaries(self, app_bundle_path: Path) -> List[Path]: + def _discover_watch_binaries(self, app_bundle_path: SafeDirectory) -> List[Path]: watch_binaries: List[Path] = [] for watch_path in app_bundle_path.rglob("Watch/*.app"): if watch_path.is_dir(): @@ -452,9 +446,7 @@ def _discover_watch_binaries(self, app_bundle_path: Path) -> List[Path]: watch_plist = plistlib.load(f) watch_executable = watch_plist.get("CFBundleExecutable") if watch_executable: - if not is_safe_path(watch_path, watch_executable): - raise UnsafePathError(f"Unsafe CFBundleExecutable in Watch plist: {watch_executable}") - watch_binary_path = watch_path / watch_executable + watch_binary_path = SafeDirectory(watch_path).resolve(watch_executable) if watch_binary_path.exists(): watch_binaries.append(watch_binary_path) else: @@ -484,9 +476,7 @@ def _get_main_binary_path(self) -> Path: main_executable = self.get_plist().get("CFBundleExecutable") if main_executable is None: raise RuntimeError("CFBundleExecutable not found in Info.plist") - if not is_safe_path(app_bundle_path, main_executable): - raise UnsafePathError(f"Unsafe CFBundleExecutable in plist: {main_executable}") - return Path(os.path.join(str(app_bundle_path), main_executable)) + return app_bundle_path.resolve(main_executable) def _parse_asset_element(self, item: dict[str, Any], parent_path: Path) -> AssetCatalogElement: """Parse a dictionary item into an AssetCatalogElement.""" @@ -504,9 +494,7 @@ def _parse_asset_element(self, item: dict[str, Any], parent_path: Path) -> Asset file_extension = Path(filename).suffix.lower() if filename and file_extension in {".png", ".jpg", ".jpeg", ".heic", ".heif", ".pdf", ".svg"}: candidate = f"{image_id}{file_extension}" - if not is_safe_path(parent_path, candidate): - raise UnsafePathError(f"Unsafe asset imageId in catalog: {image_id}") - potential_path = parent_path / candidate + potential_path = SafeDirectory(parent_path).resolve(candidate) if potential_path.exists(): full_path = potential_path else: diff --git a/src/launchpad/artifacts/providers/exceptions.py b/src/launchpad/artifacts/providers/exceptions.py new file mode 100644 index 00000000..04bba3e4 --- /dev/null +++ b/src/launchpad/artifacts/providers/exceptions.py @@ -0,0 +1,10 @@ +class UnreasonableZipError(ValueError): + """Raised when a zip file exceeds reasonable limits.""" + + pass + + +class UnsafePathError(ValueError): + """Raised when a zip file contains unsafe path entries that could lead to path traversal attacks.""" + + pass diff --git a/src/launchpad/artifacts/providers/safe_directory.py b/src/launchpad/artifacts/providers/safe_directory.py new file mode 100644 index 00000000..a85881cd --- /dev/null +++ b/src/launchpad/artifacts/providers/safe_directory.py @@ -0,0 +1,105 @@ +import os + +from pathlib import Path +from typing import Generator + +from .exceptions import UnsafePathError + + +class SafeDirectory: + """A directory wrapper that validates all path construction stays within bounds. + + Every path built via / or .resolve() is checked against the base directory. + glob and rglob validate that each result resolves within the base directory, + since glob patterns containing ".." can escape via recursive wildcards. + """ + + def __init__(self, base: Path) -> None: + self._base = base.resolve() + + @property + def path(self) -> Path: + return self._base + + def resolve(self, untrusted: str) -> Path: + """Resolve an untrusted path within this directory. + + Raises UnsafePathError if the resolved path escapes the base. + """ + try: + target = (self._base / untrusted).resolve() + except RuntimeError: + raise UnsafePathError(f"Path traversal attempt: {untrusted}") + if not target.is_relative_to(self._base): + raise UnsafePathError(f"Path traversal attempt: {untrusted}") + return target + + def child(self, untrusted: str) -> "SafeDirectory": + """Return a new SafeDirectory scoped to a validated subdirectory.""" + return SafeDirectory(self.resolve(untrusted)) + + def __truediv__(self, other: str | os.PathLike[str]) -> Path: + result = self._base / other + try: + resolved = result.resolve() + except RuntimeError: + raise UnsafePathError(f"Path traversal attempt: {other}") + if not resolved.is_relative_to(self._base): + raise UnsafePathError(f"Path traversal attempt: {other}") + # Return the non-resolved path so callers can still detect symlinks + return result + + def __str__(self) -> str: + return str(self._base) + + def __repr__(self) -> str: + return f"SafeDirectory({self._base})" + + def __fspath__(self) -> str: + return str(self._base) + + def glob(self, pattern: str) -> Generator[Path, None, None]: + for path in self._base.glob(pattern): + if path.resolve().is_relative_to(self._base): + yield path + + def rglob(self, pattern: str) -> Generator[Path, None, None]: + for path in self._base.rglob(pattern): + if path.resolve().is_relative_to(self._base): + yield path + + def iterdir(self) -> Generator[Path, None, None]: + for path in self._base.iterdir(): + if path.resolve().is_relative_to(self._base): + yield path + + def stat(self) -> os.stat_result: + return self._base.stat() + + def exists(self) -> bool: + return self._base.exists() + + def is_dir(self) -> bool: + return self._base.is_dir() + + def is_file(self) -> bool: + return self._base.is_file() + + def relative_to(self, other: str | os.PathLike[str]) -> Path: + return self._base.relative_to(other) + + @property + def name(self) -> str: + return self._base.name + + @property + def stem(self) -> str: + return self._base.stem + + @property + def suffix(self) -> str: + return self._base.suffix + + @property + def parent(self) -> Path: + return self._base.parent diff --git a/src/launchpad/artifacts/providers/zip_provider.py b/src/launchpad/artifacts/providers/zip_provider.py index 8ae8d72a..5ab1e938 100644 --- a/src/launchpad/artifacts/providers/zip_provider.py +++ b/src/launchpad/artifacts/providers/zip_provider.py @@ -6,24 +6,15 @@ from launchpad.utils.file_utils import cleanup_directory, create_temp_directory from launchpad.utils.logging import get_logger +from .exceptions import UnreasonableZipError +from .safe_directory import SafeDirectory + logger = get_logger(__name__) DEFAULT_MAX_FILE_COUNT = 100000 DEFAULT_MAX_UNCOMPRESSED_SIZE = 10 * 1024 * 1024 * 1024 -class UnreasonableZipError(ValueError): - """Raised when a zip file exceeds reasonable limits.""" - - pass - - -class UnsafePathError(ValueError): - """Raised when a zip file contains unsafe path entries that could lead to path traversal attacks.""" - - pass - - def check_reasonable_zip( zf: zipfile.ZipFile, max_file_count: int = DEFAULT_MAX_FILE_COUNT, @@ -54,20 +45,6 @@ def check_reasonable_zip( ) -def is_safe_path(base_dir: Path, requested_path: str) -> bool: - """ - Ensure file operations occur within the intended directory - Based on: https://medium.com/@contactomyna/securing-zip-file-operations-understanding-and-preventing-path-traversal-attacks-74d79f696c46 - """ - try: - base_dir = base_dir.resolve() - target_path = Path(base_dir, requested_path).resolve() - return target_path.is_relative_to(base_dir) - except RuntimeError: - # Resolve raises RuntimeError prior to 3.13 for symlink loops. - return False - - class ZipProvider: """Provider for handling zip file operations.""" @@ -80,13 +57,13 @@ def __init__(self, path: Path) -> None: self.path = path self._temp_dirs: List[Path] = [] - def extract_to_temp_directory(self) -> Path: + def extract_to_temp_directory(self) -> SafeDirectory: """Extract the zip contents to a temporary directory. Creates a temporary directory and extracts the zip contents to it. A new temporary directory is created for each call to this method. Returns: - Path to the temporary directory containing extracted files + SafeDirectory wrapping the temporary directory containing extracted files """ temp_dir = create_temp_directory("zip-extract-") self._temp_dirs.append(temp_dir) @@ -94,21 +71,19 @@ def extract_to_temp_directory(self) -> Path: self._safe_extract(str(self.path), str(temp_dir)) logger.debug(f"Extracted zip contents to {temp_dir}") - return temp_dir + return SafeDirectory(temp_dir) def _safe_extract(self, zip_path: str, extract_path: str): """Extract the zip contents to a temporary directory, ensuring that the paths are safe from path traversal attacks. Supports both standard compression methods and Zstandard compression. """ - base_dir = Path(extract_path) + safe_dir = SafeDirectory(Path(extract_path)) with zipfile.ZipFile(zip_path, "r") as zip_ref: check_reasonable_zip(zip_ref) for member in zip_ref.namelist(): - if is_safe_path(base_dir, member): - zip_ref.extract(member, extract_path) - else: - raise UnsafePathError(f"Potential path traversal attack: {member}") + safe_dir.resolve(member) + zip_ref.extract(member, extract_path) def __del__(self) -> None: """Clean up resources when object is destroyed.""" diff --git a/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py b/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py index 2a9f075c..ff71b1f2 100644 --- a/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py +++ b/src/launchpad/parsers/android/icon/binary_xml_drawable_parser.py @@ -1,9 +1,8 @@ from __future__ import annotations -from pathlib import Path - from launchpad.artifacts.android.manifest.axml import AxmlUtils from launchpad.artifacts.android.resources.binary import BinaryResourceTable +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.parsers.android.binary.android_binary_parser import AndroidBinaryParser from launchpad.parsers.android.binary.types import XmlNode from launchpad.utils.logging import get_logger @@ -16,7 +15,7 @@ class BinaryXmlDrawableParser(IconParser): def __init__( self, - extract_dir: Path, + extract_dir: SafeDirectory, binary_res_tables: list[BinaryResourceTable], ) -> None: super().__init__(extract_dir) diff --git a/src/launchpad/parsers/android/icon/icon_parser.py b/src/launchpad/parsers/android/icon/icon_parser.py index 153b6e1d..b2a4ae27 100644 --- a/src/launchpad/parsers/android/icon/icon_parser.py +++ b/src/launchpad/parsers/android/icon/icon_parser.py @@ -10,7 +10,8 @@ from PIL import Image, ImageDraw -from launchpad.artifacts.providers.zip_provider import UnsafePathError, is_safe_path +from launchpad.artifacts.providers.exceptions import UnsafePathError +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.parsers.android.binary.types import XmlNode from launchpad.utils.logging import get_logger @@ -57,8 +58,8 @@ class GradientInfo: class IconParser: - def __init__(self, extract_dir: Path) -> None: - self.extract_dir = extract_dir + def __init__(self, extract_dir: SafeDirectory) -> None: + self._safe_dir = extract_dir def _get_attr_value(self, attributes: list, name: str, required: bool = False) -> str | None: raise NotImplementedError @@ -461,23 +462,19 @@ def _interpolate_gradient_color( return None def _find_file(self, filename: str) -> Path | None: - if not is_safe_path(self.extract_dir, filename): - raise UnsafePathError(f"Unsafe file path in drawable: {filename}") - - # Try exact match first - exact_path = self.extract_dir / filename + exact_path = self._safe_dir.resolve(filename) if exact_path.exists(): return exact_path # Try with res/ prefix if not filename.startswith("res/"): - res_path = self.extract_dir / "res" / filename + res_path = self._safe_dir.resolve("res/" + filename) if res_path.exists(): return res_path # Search recursively (last resort) filename_lower = filename.lower() - for file_path in self.extract_dir.rglob("*"): + for file_path in self._safe_dir.rglob("*"): if file_path.is_file() and str(file_path).lower().endswith(filename_lower): return file_path diff --git a/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py b/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py index 4559b31f..3d661930 100644 --- a/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py +++ b/src/launchpad/parsers/android/icon/proto_xml_drawable_parser.py @@ -1,7 +1,5 @@ from __future__ import annotations -from pathlib import Path - from launchpad.artifacts.android.manifest.proto_xml import ProtoXmlUtils from launchpad.artifacts.android.resources.proto import ProtobufResourceTable from launchpad.artifacts.android.resources.protos.Resources_pb2 import ( # type: ignore[attr-defined] @@ -13,6 +11,7 @@ from launchpad.artifacts.android.resources.protos.Resources_pb2 import ( XmlNode as PbXmlNode, # type: ignore[attr-defined] ) +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.parsers.android.binary.types import ( NodeType, TypedValue, @@ -27,7 +26,7 @@ class ProtoXmlDrawableParser(IconParser): - def __init__(self, extract_dir: Path, proto_res_tables: list[ProtobufResourceTable]) -> None: + def __init__(self, extract_dir: SafeDirectory, proto_res_tables: list[ProtobufResourceTable]) -> None: super().__init__(extract_dir) self.proto_res_tables = proto_res_tables diff --git a/src/launchpad/size/analyzers/android.py b/src/launchpad/size/analyzers/android.py index 24f57126..09656c4b 100644 --- a/src/launchpad/size/analyzers/android.py +++ b/src/launchpad/size/analyzers/android.py @@ -276,7 +276,7 @@ def _get_hermes_reports(self, apks: list[APK]) -> dict[str, HermesReport]: all_reports: dict[str, HermesReport] = {} for apk in apks: extract_path = apk.get_extract_path() - apk_reports = make_hermes_reports(extract_path) + apk_reports = make_hermes_reports(extract_path.path) for relative_path, report in apk_reports.items(): if relative_path in all_reports: logger.warning(f"Duplicate Hermes report key found: {relative_path}, overwriting") diff --git a/src/launchpad/size/analyzers/apple.py b/src/launchpad/size/analyzers/apple.py index 7e91c42b..89f07ee5 100644 --- a/src/launchpad/size/analyzers/apple.py +++ b/src/launchpad/size/analyzers/apple.py @@ -131,7 +131,7 @@ def analyze(self, artifact: AppleArtifact) -> AppleAnalysisResults: file_analysis = analyze_apple_files(artifact) logger.debug(f"Found {len(file_analysis.files)} files, total size: {file_analysis.total_size} bytes") - app_bundle_path = artifact.get_app_bundle_path() + app_bundle_path = artifact.get_app_bundle_path().path bundle_sizes = calculate_bundle_sizes(app_bundle_path, app_info.name, app_info.app_id) total_download_size = bundle_sizes.total_download diff --git a/src/launchpad/size/utils/file_analysis.py b/src/launchpad/size/utils/file_analysis.py index cd242be7..be3d4526 100644 --- a/src/launchpad/size/utils/file_analysis.py +++ b/src/launchpad/size/utils/file_analysis.py @@ -34,7 +34,7 @@ def analyze_apple_files( logger.debug("Analyzing files in app bundle") - app_bundle_path = xcarchive.get_app_bundle_path() + app_bundle_path = xcarchive.get_app_bundle_path().path files: Dict[str, FileInfo] = {} dirs: Dict[str, FileInfo] = {} diff --git a/src/launchpad/utils/apple/code_signature_validator.py b/src/launchpad/utils/apple/code_signature_validator.py index ddea8a90..c8a81923 100644 --- a/src/launchpad/utils/apple/code_signature_validator.py +++ b/src/launchpad/utils/apple/code_signature_validator.py @@ -12,6 +12,7 @@ import lief from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.parsers.apple.code_signature_parser import CodeSignInformation from launchpad.parsers.apple.macho_parser import MachOParser @@ -84,7 +85,7 @@ def __init__(self, archive: ZippedXCArchive) -> None: self.archive = archive self.plist = self.archive.get_plist() self.executable_name: str = self.archive.get_plist().get("CFBundleExecutable", "") - self.app_root: Path = self.archive.get_app_bundle_path() + self.app_root: SafeDirectory = self.archive.get_app_bundle_path() self.macho_parser: Optional[MachOParser] = None def validate(self) -> Tuple[bool, List[str]]: diff --git a/tests/unit/artifacts/android/test_aab.py b/tests/unit/artifacts/android/test_aab.py index 48ac0834..18e3809e 100644 --- a/tests/unit/artifacts/android/test_aab.py +++ b/tests/unit/artifacts/android/test_aab.py @@ -5,7 +5,7 @@ from launchpad.artifacts.android.aab import AAB from launchpad.artifacts.android.manifest.manifest import AndroidApplication, AndroidManifest -from launchpad.artifacts.providers.zip_provider import UnsafePathError +from launchpad.artifacts.providers.exceptions import UnsafePathError @pytest.fixture diff --git a/tests/unit/artifacts/android/test_apk.py b/tests/unit/artifacts/android/test_apk.py index feea0895..a102ecd6 100644 --- a/tests/unit/artifacts/android/test_apk.py +++ b/tests/unit/artifacts/android/test_apk.py @@ -5,7 +5,7 @@ from launchpad.artifacts.android.apk import APK from launchpad.artifacts.android.manifest.manifest import AndroidApplication, AndroidManifest -from launchpad.artifacts.providers.zip_provider import UnsafePathError +from launchpad.artifacts.providers.exceptions import UnsafePathError @pytest.fixture diff --git a/tests/unit/artifacts/apple/test_zipped_xcarchive.py b/tests/unit/artifacts/apple/test_zipped_xcarchive.py index 14228232..7bca2387 100644 --- a/tests/unit/artifacts/apple/test_zipped_xcarchive.py +++ b/tests/unit/artifacts/apple/test_zipped_xcarchive.py @@ -8,7 +8,8 @@ import pytest from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive -from launchpad.artifacts.providers.zip_provider import UnsafePathError +from launchpad.artifacts.providers.exceptions import UnsafePathError +from launchpad.artifacts.providers.safe_directory import SafeDirectory class TestZippedXCArchive: @@ -16,7 +17,7 @@ class TestZippedXCArchive: def test_top_level_asset_catalog_parsing(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - tmpdir_path = Path(tmpdir) + tmpdir_path = Path(tmpdir).resolve() xcarchive_dir = tmpdir_path / "Test.xcarchive" parsed_assets_dir = xcarchive_dir / "ParsedAssets" / "Products" / "Applications" / "Test.app" @@ -40,12 +41,12 @@ def test_top_level_asset_catalog_parsing(self) -> None: with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) - archive._extract_dir = tmpdir_path + archive._extract_dir = SafeDirectory(tmpdir_path) with patch.object( archive, "get_app_bundle_path", - return_value=xcarchive_dir / "Products" / "Applications" / "Test.app", + return_value=SafeDirectory(xcarchive_dir / "Products" / "Applications" / "Test.app"), ): elements = archive.get_asset_catalog_details(Path("Assets.car")) @@ -60,7 +61,7 @@ def test_top_level_asset_catalog_parsing(self) -> None: def test_nested_bundle_asset_catalog_parsing(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - tmpdir_path = Path(tmpdir) + tmpdir_path = Path(tmpdir).resolve() xcarchive_dir = tmpdir_path / "Test.xcarchive" parsed_assets_dir = xcarchive_dir / "ParsedAssets" / "Products" / "Applications" / "Test.app" @@ -85,12 +86,12 @@ def test_nested_bundle_asset_catalog_parsing(self) -> None: with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) - archive._extract_dir = tmpdir_path + archive._extract_dir = SafeDirectory(tmpdir_path) with patch.object( archive, "get_app_bundle_path", - return_value=xcarchive_dir / "Products" / "Applications" / "Test.app", + return_value=SafeDirectory(xcarchive_dir / "Products" / "Applications" / "Test.app"), ): elements = archive.get_asset_catalog_details(Path("PlugIns/TestExtension.appex/Assets.car")) @@ -106,7 +107,7 @@ def test_nested_bundle_asset_catalog_parsing(self) -> None: def test_framework_bundle_asset_catalog_parsing(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - tmpdir_path = Path(tmpdir) + tmpdir_path = Path(tmpdir).resolve() xcarchive_dir = tmpdir_path / "Test.xcarchive" parsed_assets_dir = xcarchive_dir / "ParsedAssets" / "Products" / "Applications" / "Test.app" @@ -131,12 +132,12 @@ def test_framework_bundle_asset_catalog_parsing(self) -> None: with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) - archive._extract_dir = tmpdir_path + archive._extract_dir = SafeDirectory(tmpdir_path) with patch.object( archive, "get_app_bundle_path", - return_value=xcarchive_dir / "Products" / "Applications" / "Test.app", + return_value=SafeDirectory(xcarchive_dir / "Products" / "Applications" / "Test.app"), ): elements = archive.get_asset_catalog_details(Path("MyFramework.bundle/Assets.car")) @@ -152,8 +153,8 @@ def test_framework_bundle_asset_catalog_parsing(self) -> None: def test_get_binary_path_rejects_path_traversal(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - app_bundle_path = Path(tmpdir) / "Test.app" - app_bundle_path.mkdir() + app_bundle_path = SafeDirectory(Path(tmpdir) / "Test.app") + app_bundle_path.path.mkdir() with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) @@ -167,8 +168,8 @@ def test_get_binary_path_rejects_path_traversal(self) -> None: def test_get_main_binary_path_rejects_path_traversal(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - app_bundle_path = Path(tmpdir) / "Test.app" - app_bundle_path.mkdir() + app_bundle_path = SafeDirectory(Path(tmpdir) / "Test.app") + app_bundle_path.path.mkdir() with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) @@ -229,12 +230,12 @@ def test_asset_catalog_rejects_path_traversal_in_image_id(self) -> None: with patch.object(ZippedXCArchive, "__init__", lambda self, path: None): archive = ZippedXCArchive(Path("dummy")) - archive._extract_dir = tmpdir_path + archive._extract_dir = SafeDirectory(tmpdir_path) with patch.object( archive, "get_app_bundle_path", - return_value=xcarchive_dir / "Products" / "Applications" / "Test.app", + return_value=SafeDirectory(xcarchive_dir / "Products" / "Applications" / "Test.app"), ): with pytest.raises(UnsafePathError): archive.get_asset_catalog_details(Path("Assets.car")) diff --git a/tests/unit/artifacts/providers/test_zip_provider.py b/tests/unit/artifacts/providers/test_zip_provider.py index 458c9ec8..93058d8b 100644 --- a/tests/unit/artifacts/providers/test_zip_provider.py +++ b/tests/unit/artifacts/providers/test_zip_provider.py @@ -5,12 +5,12 @@ import pytest +from launchpad.artifacts.providers.exceptions import UnsafePathError +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.artifacts.providers.zip_provider import ( UnreasonableZipError, - UnsafePathError, ZipProvider, check_reasonable_zip, - is_safe_path, ) @@ -32,43 +32,42 @@ def test_init(self, hackernews_xcarchive: Path) -> None: def test_extract_to_temp_directory_ios(self, hackernews_xcarchive: Path) -> None: provider = ZipProvider(hackernews_xcarchive) - temp_dir = provider.extract_to_temp_directory() + safe_dir = provider.extract_to_temp_directory() - assert temp_dir.exists() - assert temp_dir.is_dir() + assert isinstance(safe_dir, SafeDirectory) + assert safe_dir.exists() + assert safe_dir.is_dir() assert len(provider._temp_dirs) == 1 - assert provider._temp_dirs[0] == temp_dir - extracted_files = list(temp_dir.rglob("*")) + extracted_files = list(safe_dir.rglob("*")) assert len(extracted_files) > 0 def test_extract_to_temp_directory_android(self, zipped_apk: Path) -> None: provider = ZipProvider(zipped_apk) - temp_dir = provider.extract_to_temp_directory() + safe_dir = provider.extract_to_temp_directory() - assert temp_dir.exists() - assert temp_dir.is_dir() + assert isinstance(safe_dir, SafeDirectory) + assert safe_dir.exists() + assert safe_dir.is_dir() assert len(provider._temp_dirs) == 1 - extracted_files = list(temp_dir.rglob("*")) + extracted_files = list(safe_dir.rglob("*")) assert len(extracted_files) > 0 def test_multiple_extractions(self, hackernews_xcarchive: Path) -> None: provider = ZipProvider(hackernews_xcarchive) - temp_dir1 = provider.extract_to_temp_directory() - temp_dir2 = provider.extract_to_temp_directory() + safe_dir1 = provider.extract_to_temp_directory() + safe_dir2 = provider.extract_to_temp_directory() - assert temp_dir1 != temp_dir2 + assert safe_dir1.path != safe_dir2.path assert len(provider._temp_dirs) == 2 - assert temp_dir1 in provider._temp_dirs - assert temp_dir2 in provider._temp_dirs - assert temp_dir1.exists() - assert temp_dir2.exists() + assert safe_dir1.exists() + assert safe_dir2.exists() def test_safe_extract_blocks_traversal(self, malicious_zip: Path) -> None: provider = ZipProvider(malicious_zip) - with pytest.raises(UnsafePathError, match="Potential path traversal attack"): + with pytest.raises(UnsafePathError, match="Path traversal attempt"): provider.extract_to_temp_directory() def test_nonexistent_zip_file(self) -> None: @@ -89,24 +88,88 @@ def test_invalid_zip_file(self) -> None: provider.extract_to_temp_directory() -class TestIsSafePath: - def test_valid_paths(self) -> None: - base_dir = Path("/tmp/test") - assert is_safe_path(base_dir, "file.txt") - assert is_safe_path(base_dir, "subdir/file.txt") - assert is_safe_path(base_dir, "a/b/c/file.txt") - - def test_path_traversal_attempts(self) -> None: - base_dir = Path("/tmp/test") - assert not is_safe_path(base_dir, "../file.txt") - assert not is_safe_path(base_dir, "../../file.txt") - assert not is_safe_path(base_dir, "../../../etc/passwd") - assert not is_safe_path(base_dir, "subdir/../../file.txt") - - def test_absolute_paths(self) -> None: - base_dir = Path("/tmp/test") - assert not is_safe_path(base_dir, "/etc/passwd") - assert not is_safe_path(base_dir, "/tmp/other/file.txt") +class TestSafeDirectory: + def test_resolve_valid_paths(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + assert safe_dir.resolve("file.txt") == Path(tmpdir).resolve() / "file.txt" + assert safe_dir.resolve("subdir/file.txt") == Path(tmpdir).resolve() / "subdir" / "file.txt" + + def test_resolve_rejects_traversal(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + with pytest.raises(UnsafePathError): + safe_dir.resolve("../file.txt") + with pytest.raises(UnsafePathError): + safe_dir.resolve("../../etc/passwd") + with pytest.raises(UnsafePathError): + safe_dir.resolve("subdir/../../file.txt") + + def test_resolve_rejects_absolute_paths(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + with pytest.raises(UnsafePathError): + safe_dir.resolve("/etc/passwd") + + def test_child_creates_scoped_directory(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + (base / "sub").mkdir() + safe_dir = SafeDirectory(base) + child = safe_dir.child("sub") + assert child.path == base.resolve() / "sub" + with pytest.raises(UnsafePathError): + child.resolve("../../etc/passwd") + + def test_path_property(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + assert safe_dir.path == Path(tmpdir).resolve() + + def test_truediv_validates_path(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + result = safe_dir / "subdir" / "file.txt" + assert result == Path(tmpdir).resolve() / "subdir" / "file.txt" + assert isinstance(result, Path) + + def test_truediv_rejects_traversal(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + with pytest.raises(UnsafePathError): + safe_dir / "../../../etc/passwd" + with pytest.raises(UnsafePathError): + safe_dir / "/etc/passwd" + + def test_glob_and_rglob(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + (base / "a.txt").write_text("a") + (base / "sub").mkdir() + (base / "sub" / "b.txt").write_text("b") + safe_dir = SafeDirectory(base) + assert len(list(safe_dir.glob("*.txt"))) == 1 + assert len(list(safe_dir.rglob("*.txt"))) == 2 + + def test_glob_filters_escaped_paths(self) -> None: + """Glob patterns with recursive wildcards and .. must not escape the base.""" + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + inner = base / "inner" + inner.mkdir() + (base / "outside.txt").write_text("should not appear") + (inner / "inside.txt").write_text("ok") + safe_dir = SafeDirectory(inner) + results = list(safe_dir.glob("../*.txt")) + assert all(p.resolve().is_relative_to(inner.resolve()) for p in results) + assert not any("outside" in p.name for p in results) + + def test_fspath(self) -> None: + import os + + with tempfile.TemporaryDirectory() as tmpdir: + safe_dir = SafeDirectory(Path(tmpdir)) + assert os.fspath(safe_dir) == str(Path(tmpdir).resolve()) class TestCheckReasonableZip: @@ -137,10 +200,10 @@ def test_extract_zstd_zip(self) -> None: try: provider = ZipProvider(temp_path) - temp_dir = provider.extract_to_temp_directory() + safe_dir = provider.extract_to_temp_directory() - assert temp_dir.exists() - assert (temp_dir / "test.txt").exists() - assert (temp_dir / "test.txt").read_text() == "content" + assert safe_dir.exists() + assert (safe_dir / "test.txt").exists() + assert (safe_dir / "test.txt").read_text() == "content" finally: temp_path.unlink(missing_ok=True) diff --git a/tests/unit/parsers/android/icon/test_icon_parser.py b/tests/unit/parsers/android/icon/test_icon_parser.py index 6d50ca96..871f0d7d 100644 --- a/tests/unit/parsers/android/icon/test_icon_parser.py +++ b/tests/unit/parsers/android/icon/test_icon_parser.py @@ -4,15 +4,15 @@ import pytest -from launchpad.artifacts.providers.zip_provider import UnsafePathError +from launchpad.artifacts.providers.exceptions import UnsafePathError +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.parsers.android.icon.icon_parser import IconParser class TestIconParserFindFile: def test_find_file_rejects_path_traversal(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: - extract_dir = Path(tmpdir) - parser = IconParser(extract_dir) + parser = IconParser(SafeDirectory(Path(tmpdir))) with pytest.raises(UnsafePathError): parser._find_file("../../etc/passwd") diff --git a/tests/unit/size/utils/test_file_analysis.py b/tests/unit/size/utils/test_file_analysis.py index 53436fa5..479489cf 100644 --- a/tests/unit/size/utils/test_file_analysis.py +++ b/tests/unit/size/utils/test_file_analysis.py @@ -9,6 +9,7 @@ import pytest from launchpad.artifacts.apple.zipped_xcarchive import AssetCatalogElement, ZippedXCArchive +from launchpad.artifacts.providers.safe_directory import SafeDirectory from launchpad.size.constants import APPLE_FILESYSTEM_BLOCK_SIZE from launchpad.size.models.common import FileAnalysis from launchpad.size.models.treemap import TreemapType @@ -61,7 +62,7 @@ def temp_app_bundle(self): (resources_dir / "image.png").write_bytes(b"fake_png_data" * 20) (resources_dir / "data.json").write_text('{"key": "value"}') - yield app_path + yield SafeDirectory(app_path) def test_basic_file_analysis(self, mock_xcarchive, temp_app_bundle): """Test basic file and directory analysis.""" @@ -292,8 +293,8 @@ def test_hash_consistency(self, mock_xcarchive, temp_app_bundle): def test_empty_bundle(self, mock_xcarchive): """Test analysis of an empty app bundle.""" with tempfile.TemporaryDirectory() as temp_dir: - empty_bundle = Path(temp_dir) / "Empty.app" - empty_bundle.mkdir() + empty_bundle = SafeDirectory(Path(temp_dir) / "Empty.app") + empty_bundle.path.mkdir() mock_xcarchive.get_app_bundle_path.return_value = empty_bundle mock_xcarchive.get_asset_catalog_details.return_value = []