Add path traversal guards to plist-derived paths (#624)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: trevor-e

src/launchpad/artifacts/apple/zipped_xcarchive.py

@@ -287,6 +287,9 @@ 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
 
     @sentry_sdk.trace
@@ -394,6 +397,8 @@ def get_asset_catalog_details(self, relative_path: Path) -> List[AssetCatalogEle
                 data = json.load(f)
 
             return [self._parse_asset_element(item, parent_path) for item in data]
+        except UnsafePathError:
+            raise
         except Exception:
             logger.exception(f"Failed to get asset catalog details for {relative_path}")
             return []
@@ -421,11 +426,17 @@ 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
                             if extension_binary_path.exists():
                                 extension_binaries.append(extension_binary_path)
                             else:
                                 logger.warning("Extension binary not found", extra={"path": extension_binary_path})
+                    except UnsafePathError:
+                        raise
                     except Exception:
                         logger.exception(f"Failed to read extension Info.plist at {extension_path}")
         return extension_binaries
@@ -441,11 +452,15 @@ 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
                             if watch_binary_path.exists():
                                 watch_binaries.append(watch_binary_path)
                             else:
                                 logger.warning("Watch binary not found", extra={"path": watch_binary_path})
+                    except UnsafePathError:
+                        raise
                     except Exception:
                         logger.exception(f"Failed to read Watch app Info.plist at {watch_path}")
         return watch_binaries
@@ -469,6 +484,8 @@ 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))
 
     def _parse_asset_element(self, item: dict[str, Any], parent_path: Path) -> AssetCatalogElement:
@@ -486,7 +503,10 @@ 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"}:
-            potential_path = parent_path / f"{image_id}{file_extension}"
+            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
             if potential_path.exists():
                 full_path = potential_path
             else:

tests/unit/artifacts/apple/test_zipped_xcarchive.py

@@ -1,10 +1,14 @@
 import json
+import plistlib
 import tempfile
 
 from pathlib import Path
 from unittest.mock import patch
 
+import pytest
+
 from launchpad.artifacts.apple.zipped_xcarchive import ZippedXCArchive
+from launchpad.artifacts.providers.zip_provider import UnsafePathError
 
 
 class TestZippedXCArchive:
@@ -145,3 +149,92 @@ def test_framework_bundle_asset_catalog_parsing(self) -> None:
                     assert not wrong_path.exists(), "Image should NOT exist at top-level"
 
                     assert "MyFramework.bundle" in str(element.full_path)
+
+    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()
+
+            with patch.object(ZippedXCArchive, "__init__", lambda self, path: None):
+                archive = ZippedXCArchive(Path("dummy"))
+
+                with (
+                    patch.object(archive, "get_app_bundle_path", return_value=app_bundle_path),
+                    patch.object(archive, "get_plist", return_value={"CFBundleExecutable": "../../../etc/passwd"}),
+                ):
+                    with pytest.raises(UnsafePathError):
+                        archive.get_binary_path()
+
+    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()
+
+            with patch.object(ZippedXCArchive, "__init__", lambda self, path: None):
+                archive = ZippedXCArchive(Path("dummy"))
+
+                with (
+                    patch.object(archive, "get_app_bundle_path", return_value=app_bundle_path),
+                    patch.object(archive, "get_plist", return_value={"CFBundleExecutable": "../../../etc/passwd"}),
+                ):
+                    with pytest.raises(UnsafePathError):
+                        archive._get_main_binary_path()
+
+    def test_discover_extension_binaries_rejects_path_traversal(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            app_bundle_path = Path(tmpdir) / "Test.app"
+            extension_path = app_bundle_path / "PlugIns" / "Malicious.appex"
+            extension_path.mkdir(parents=True)
+            with open(extension_path / "Info.plist", "wb") as f:
+                plistlib.dump({"CFBundleExecutable": "../../../etc/passwd"}, f)
+
+            with patch.object(ZippedXCArchive, "__init__", lambda self, path: None):
+                archive = ZippedXCArchive(Path("dummy"))
+                with pytest.raises(UnsafePathError):
+                    archive._discover_extension_binaries(app_bundle_path)
+
+    def test_discover_watch_binaries_rejects_path_traversal(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            app_bundle_path = Path(tmpdir) / "Test.app"
+            watch_path = app_bundle_path / "Watch" / "Malicious.app"
+            watch_path.mkdir(parents=True)
+            with open(watch_path / "Info.plist", "wb") as f:
+                plistlib.dump({"CFBundleExecutable": "../../../etc/passwd"}, f)
+
+            with patch.object(ZippedXCArchive, "__init__", lambda self, path: None):
+                archive = ZippedXCArchive(Path("dummy"))
+                with pytest.raises(UnsafePathError):
+                    archive._discover_watch_binaries(app_bundle_path)
+
+    def test_asset_catalog_rejects_path_traversal_in_image_id(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            tmpdir_path = Path(tmpdir)
+
+            xcarchive_dir = tmpdir_path / "Test.xcarchive"
+            parsed_assets_dir = xcarchive_dir / "ParsedAssets" / "Products" / "Applications" / "Test.app"
+            parsed_assets_dir.mkdir(parents=True)
+
+            assets_json = parsed_assets_dir / "Assets.json"
+            assets_data = [
+                {
+                    "name": "icon.png",
+                    "imageId": "../../../etc/passwd",
+                    "size": 1024,
+                    "type": 0,
+                    "vector": False,
+                    "filename": "icon.png",
+                }
+            ]
+            assets_json.write_text(json.dumps(assets_data))
+
+            with patch.object(ZippedXCArchive, "__init__", lambda self, path: None):
+                archive = ZippedXCArchive(Path("dummy"))
+                archive._extract_dir = tmpdir_path
+
+                with patch.object(
+                    archive,
+                    "get_app_bundle_path",
+                    return_value=xcarchive_dir / "Products" / "Applications" / "Test.app",
+                ):
+                    with pytest.raises(UnsafePathError):
+                        archive.get_asset_catalog_details(Path("Assets.car"))