fix(security): Add path traversal guards to icon file reading#622
Merged
Conversation
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) <noreply@anthropic.com>
Contributor
📲 Install BuildsiOS
Android
|
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) <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
is_safe_path(), but after extraction, icon reading code constructed file paths from attacker-controlled manifest/plist data without checking for directory traversalis_safe_path()checks to APK, AAB, and iOS icon reading, plus the icon parser's_find_file()methodFixes EME-1152
🤖 Generated with Claude Code