From 5302629e71b1aaa6b1d3eb7f05b837724d14a370 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 15 May 2026 13:53:35 +0200 Subject: [PATCH 1/3] fix(security): Add path traversal guards to icon file reading Zip extraction already validates paths via is_safe_path(), but after extraction, icon reading code constructed file paths from manifest data without checking for traversal. A malicious manifest could reference "../../../etc/passwd" as the icon path and escape the extract directory. Add is_safe_path() checks to: - APK.get_app_icon() (manifest icon_path) - AAB.get_app_icon() (manifest icon_path) - IconParser._find_file() (drawable references from parsed XML) - ZippedXCArchive.get_app_icon() (plist icon names) Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/android/aab.py | 8 ++++++-- src/launchpad/artifacts/android/apk.py | 5 ++++- src/launchpad/artifacts/apple/zipped_xcarchive.py | 6 +++++- src/launchpad/parsers/android/icon/icon_parser.py | 5 +++++ tests/unit/artifacts/android/test_aab.py | 10 ++++++++++ tests/unit/artifacts/android/test_apk.py | 10 ++++++++++ tests/unit/parsers/android/icon/__init__.py | 0 .../unit/parsers/android/icon/test_icon_parser.py | 15 +++++++++++++++ 8 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 tests/unit/parsers/android/icon/__init__.py create mode 100644 tests/unit/parsers/android/icon/test_icon_parser.py diff --git a/src/launchpad/artifacts/android/aab.py b/src/launchpad/artifacts/android/aab.py index d2d69fbd..f01653bf 100644 --- a/src/launchpad/artifacts/android/aab.py +++ b/src/launchpad/artifacts/android/aab.py @@ -19,7 +19,7 @@ from launchpad.utils.logging import get_logger from ..artifact import AndroidArtifact -from ..providers.zip_provider import ZipProvider +from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path from .apk import APK from .manifest.manifest import AndroidManifest from .manifest.proto_xml import ProtoXmlUtils @@ -156,7 +156,11 @@ def get_app_icon(self) -> bytes | None: logger.info("No icon path found in manifest") return None - icon_path = self._extract_dir / "base" / icon_path_str + 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 if not icon_path.exists(): logger.info(f"Icon not found in AAB: {icon_path_str}") diff --git a/src/launchpad/artifacts/android/apk.py b/src/launchpad/artifacts/android/apk.py index aace6779..082ee033 100644 --- a/src/launchpad/artifacts/android/apk.py +++ b/src/launchpad/artifacts/android/apk.py @@ -18,7 +18,7 @@ from ...parsers.android.dex.types import ClassDefinition from ...utils.logging import get_logger from ..artifact import AndroidArtifact -from ..providers.zip_provider import ZipProvider +from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path from .manifest.axml import AxmlUtils from .manifest.manifest import AndroidManifest from .resources.binary import BinaryResourceTable @@ -128,6 +128,9 @@ 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 if not icon_path.exists(): diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index f1ee7cdf..b4cf8fb5 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -17,7 +17,7 @@ from launchpad.utils.logging import get_logger from ..artifact import AppleArtifact -from ..providers.zip_provider import ZipProvider +from ..providers.zip_provider import ZipProvider, is_safe_path logger = get_logger(__name__) @@ -130,6 +130,10 @@ def get_app_icon(self) -> bytes | None: app_bundle_path = self.get_app_bundle_path() for icon_name in icon_info.primary_icon_files: + if not is_safe_path(app_bundle_path, icon_name): + logger.warning(f"Unsafe icon name in plist: {icon_name}") + continue + # 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" diff --git a/src/launchpad/parsers/android/icon/icon_parser.py b/src/launchpad/parsers/android/icon/icon_parser.py index 0b019f7a..4e253bf0 100644 --- a/src/launchpad/parsers/android/icon/icon_parser.py +++ b/src/launchpad/parsers/android/icon/icon_parser.py @@ -10,6 +10,7 @@ from PIL import Image, ImageDraw +from launchpad.artifacts.providers.zip_provider import is_safe_path from launchpad.parsers.android.binary.types import XmlNode from launchpad.utils.logging import get_logger @@ -458,6 +459,10 @@ def _interpolate_gradient_color( return None def _find_file(self, filename: str) -> Path | None: + if not is_safe_path(self.extract_dir, filename): + logger.warning("Unsafe file path rejected", extra={"file_name": filename}) + return None + # Try exact match first exact_path = self.extract_dir / filename if exact_path.exists(): diff --git a/tests/unit/artifacts/android/test_aab.py b/tests/unit/artifacts/android/test_aab.py index 7d1f8f21..48ac0834 100644 --- a/tests/unit/artifacts/android/test_aab.py +++ b/tests/unit/artifacts/android/test_aab.py @@ -1,8 +1,11 @@ from pathlib import Path +from unittest.mock import patch import pytest from launchpad.artifacts.android.aab import AAB +from launchpad.artifacts.android.manifest.manifest import AndroidApplication, AndroidManifest +from launchpad.artifacts.providers.zip_provider import UnsafePathError @pytest.fixture @@ -32,3 +35,10 @@ def test_get_app_icon(self, test_aab: AAB) -> None: assert len(icon) > 0 assert icon.startswith(b"\x89PNG") assert icon.endswith(b"IEND\xae\x42\x60\x82") + + def test_get_app_icon_rejects_path_traversal(self, test_aab: AAB) -> None: + malicious_app = AndroidApplication.model_construct(icon_path="../../../etc/passwd") + malicious_manifest = AndroidManifest.model_construct(application=malicious_app) + with patch.object(test_aab, "get_manifest", return_value=malicious_manifest): + with pytest.raises(UnsafePathError): + test_aab.get_app_icon() diff --git a/tests/unit/artifacts/android/test_apk.py b/tests/unit/artifacts/android/test_apk.py index 8d7b7002..feea0895 100644 --- a/tests/unit/artifacts/android/test_apk.py +++ b/tests/unit/artifacts/android/test_apk.py @@ -1,8 +1,11 @@ from pathlib import Path +from unittest.mock import patch import pytest from launchpad.artifacts.android.apk import APK +from launchpad.artifacts.android.manifest.manifest import AndroidApplication, AndroidManifest +from launchpad.artifacts.providers.zip_provider import UnsafePathError @pytest.fixture @@ -43,3 +46,10 @@ def test_get_app_icon(self, test_apk: APK) -> None: assert len(icon) > 0 assert icon.startswith(b"\x89PNG") assert icon.endswith(b"IEND\xae\x42\x60\x82") + + def test_get_app_icon_rejects_path_traversal(self, test_apk: APK) -> None: + malicious_app = AndroidApplication.model_construct(icon_path="../../../etc/passwd") + malicious_manifest = AndroidManifest.model_construct(application=malicious_app) + with patch.object(test_apk, "get_manifest", return_value=malicious_manifest): + with pytest.raises(UnsafePathError): + test_apk.get_app_icon() diff --git a/tests/unit/parsers/android/icon/__init__.py b/tests/unit/parsers/android/icon/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/parsers/android/icon/test_icon_parser.py b/tests/unit/parsers/android/icon/test_icon_parser.py new file mode 100644 index 00000000..0a504e14 --- /dev/null +++ b/tests/unit/parsers/android/icon/test_icon_parser.py @@ -0,0 +1,15 @@ +import tempfile + +from pathlib import Path + +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) + + assert parser._find_file("../../etc/passwd") is None + assert parser._find_file("/etc/passwd") is None From 7a95f24800e6d6f7e4f1e8ff3975a2d2943a9a7f Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 15 May 2026 13:56:24 +0200 Subject: [PATCH 2/3] fix(security): Raise UnsafePathError consistently for all path traversal All is_safe_path() callsites now raise UnsafePathError instead of some logging warnings and returning None. Matches the behavior in zip_provider._safe_extract(). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/apple/zipped_xcarchive.py | 5 ++--- src/launchpad/parsers/android/icon/icon_parser.py | 5 ++--- tests/unit/parsers/android/icon/test_icon_parser.py | 10 ++++++++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/launchpad/artifacts/apple/zipped_xcarchive.py b/src/launchpad/artifacts/apple/zipped_xcarchive.py index b4cf8fb5..608daa31 100644 --- a/src/launchpad/artifacts/apple/zipped_xcarchive.py +++ b/src/launchpad/artifacts/apple/zipped_xcarchive.py @@ -17,7 +17,7 @@ from launchpad.utils.logging import get_logger from ..artifact import AppleArtifact -from ..providers.zip_provider import ZipProvider, is_safe_path +from ..providers.zip_provider import UnsafePathError, ZipProvider, is_safe_path logger = get_logger(__name__) @@ -131,8 +131,7 @@ def get_app_icon(self) -> bytes | None: for icon_name in icon_info.primary_icon_files: if not is_safe_path(app_bundle_path, icon_name): - logger.warning(f"Unsafe icon name in plist: {icon_name}") - continue + raise UnsafePathError(f"Unsafe icon name in plist: {icon_name}") # iOS lists base names without extensions or resolution modifiers (@2x, @3x, ~ipad) # Search for files matching the base name with any suffix diff --git a/src/launchpad/parsers/android/icon/icon_parser.py b/src/launchpad/parsers/android/icon/icon_parser.py index 4e253bf0..6e7451c8 100644 --- a/src/launchpad/parsers/android/icon/icon_parser.py +++ b/src/launchpad/parsers/android/icon/icon_parser.py @@ -10,7 +10,7 @@ from PIL import Image, ImageDraw -from launchpad.artifacts.providers.zip_provider import is_safe_path +from launchpad.artifacts.providers.zip_provider import UnsafePathError, is_safe_path from launchpad.parsers.android.binary.types import XmlNode from launchpad.utils.logging import get_logger @@ -460,8 +460,7 @@ def _interpolate_gradient_color( def _find_file(self, filename: str) -> Path | None: if not is_safe_path(self.extract_dir, filename): - logger.warning("Unsafe file path rejected", extra={"file_name": filename}) - return None + raise UnsafePathError(f"Unsafe file path in drawable: {filename}") # Try exact match first exact_path = self.extract_dir / filename diff --git a/tests/unit/parsers/android/icon/test_icon_parser.py b/tests/unit/parsers/android/icon/test_icon_parser.py index 0a504e14..6d50ca96 100644 --- a/tests/unit/parsers/android/icon/test_icon_parser.py +++ b/tests/unit/parsers/android/icon/test_icon_parser.py @@ -2,6 +2,9 @@ from pathlib import Path +import pytest + +from launchpad.artifacts.providers.zip_provider import UnsafePathError from launchpad.parsers.android.icon.icon_parser import IconParser @@ -11,5 +14,8 @@ def test_find_file_rejects_path_traversal(self) -> None: extract_dir = Path(tmpdir) parser = IconParser(extract_dir) - assert parser._find_file("../../etc/passwd") is None - assert parser._find_file("/etc/passwd") is None + with pytest.raises(UnsafePathError): + parser._find_file("../../etc/passwd") + + with pytest.raises(UnsafePathError): + parser._find_file("/etc/passwd") From fad39ac9d68c942662c999294beec96b103f898a Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Fri, 15 May 2026 14:21:06 +0200 Subject: [PATCH 3/3] fix(security): Re-raise UnsafePathError through except Exception blocks The broad except Exception blocks in render_from_path, APK.get_app_icon, and AAB.get_app_icon were silently catching and swallowing UnsafePathError, undermining the path traversal guards. Add explicit re-raise clauses so the security error propagates to callers. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/launchpad/artifacts/android/aab.py | 2 ++ src/launchpad/artifacts/android/apk.py | 2 ++ src/launchpad/parsers/android/icon/icon_parser.py | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/launchpad/artifacts/android/aab.py b/src/launchpad/artifacts/android/aab.py index f01653bf..332c191b 100644 --- a/src/launchpad/artifacts/android/aab.py +++ b/src/launchpad/artifacts/android/aab.py @@ -179,6 +179,8 @@ def get_app_icon(self) -> bytes | None: logger.info(f"Could not process XML drawable for icon: {icon_path_str}") return None + except UnsafePathError: + raise except Exception: logger.exception(f"Error processing XML drawable for icon: {icon_path_str}") return None diff --git a/src/launchpad/artifacts/android/apk.py b/src/launchpad/artifacts/android/apk.py index 082ee033..436180f8 100644 --- a/src/launchpad/artifacts/android/apk.py +++ b/src/launchpad/artifacts/android/apk.py @@ -150,6 +150,8 @@ def get_app_icon(self) -> bytes | None: logger.info(f"Could not process XML drawable for icon: {icon_path_str}") return None + except UnsafePathError: + raise except Exception: logger.exception(f"Error processing XML drawable for icon: {icon_path_str}") return None diff --git a/src/launchpad/parsers/android/icon/icon_parser.py b/src/launchpad/parsers/android/icon/icon_parser.py index 6e7451c8..153b6e1d 100644 --- a/src/launchpad/parsers/android/icon/icon_parser.py +++ b/src/launchpad/parsers/android/icon/icon_parser.py @@ -80,6 +80,8 @@ def render_from_path(self, xml_file_path: Path) -> bytes | None: return self._render_adaptive_icon(root_node) return self._render_vector_drawable(root_node) + except UnsafePathError: + raise except Exception: logger.exception("Error rendering icon from path") return None