Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions src/launchpad/artifacts/android/aab.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from launchpad.utils.logging import get_logger

from ..artifact import AndroidArtifact
from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path
from ..providers.zip_provider import UnsafePathError, ZipProvider
from .apk import APK
from .manifest.manifest import AndroidManifest
from .manifest.proto_xml import ProtoXmlUtils
Expand Down Expand Up @@ -156,11 +156,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}")
Expand All @@ -171,7 +168,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:
Expand Down
10 changes: 4 additions & 6 deletions src/launchpad/artifacts/android/apk.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
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.safe_directory import SafeDirectory
from ..providers.zip_provider import UnsafePathError, ZipProvider
from .manifest.axml import AxmlUtils
from .manifest.manifest import AndroidManifest
from .resources.binary import BinaryResourceTable
Expand Down Expand Up @@ -109,7 +110,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:
Expand All @@ -128,10 +129,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}")
Expand Down
12 changes: 6 additions & 6 deletions src/launchpad/artifacts/apple/zipped_xcarchive.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
from launchpad.utils.logging import get_logger

from ..artifact import AppleArtifact
from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path
from ..providers.safe_directory import SafeDirectory
from ..providers.zip_provider import ZipProvider

logger = get_logger(__name__)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 = SafeDirectory(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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discarded resolve result silently performs validation

Medium Severity

app_bundle.resolve(icon_name) is called as a bare expression with its return value silently discarded. It serves as the sole path traversal validation for untrusted plist input, but it looks identical to dead code. Every other call site in this PR assigns the result (e.g., icon_path = self._extract_dir.resolve(icon_path_str) in apk.py and aab.py), making this the only place where the critical security check is invisible. A future developer or automated cleanup could remove this "unused" expression, silently disabling the path traversal protection for icon names.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8a90e9f. Configure here.


# 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
Expand Down
10 changes: 10 additions & 0 deletions src/launchpad/artifacts/providers/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
class UnreasonableZipError(ValueError):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved from zipprovider to avoid circular imports.

"""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
90 changes: 90 additions & 0 deletions src/launchpad/artifacts/providers/safe_directory.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import os

from pathlib import Path
from typing import Generator

from .exceptions import UnsafePathError


class SafeDirectory:
"""A directory wrapper that validates untrusted paths stay within bounds.

Use .resolve(untrusted) for attacker-controlled input (manifest values,
plist entries, zip member names). Trusted Path operations (glob, rglob,
iterdir, /, etc.) are delegated directly.
"""

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))

# -- Trusted Path delegations --

def __truediv__(self, other: str | os.PathLike[str]) -> Path:
return self._base / other

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]:
return self._base.glob(pattern)

def rglob(self, pattern: str) -> Generator[Path, None, None]:
return self._base.rglob(pattern)

def iterdir(self) -> Generator[Path, None, None]:
return self._base.iterdir()

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
21 changes: 6 additions & 15 deletions src/launchpad/artifacts/providers/zip_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, UnsafePathError
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,
Expand Down Expand Up @@ -80,21 +71,21 @@ 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)

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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand Down
17 changes: 7 additions & 10 deletions src/launchpad/parsers/android/icon/icon_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@

from PIL import Image, ImageDraw

from launchpad.artifacts.providers.zip_provider import UnsafePathError, is_safe_path
from launchpad.artifacts.providers.safe_directory import SafeDirectory
from launchpad.artifacts.providers.zip_provider import UnsafePathError
from launchpad.parsers.android.binary.types import XmlNode
from launchpad.utils.logging import get_logger

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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]
Expand All @@ -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,
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/launchpad/size/analyzers/android.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading
Loading