Keep host paths safe from guest-controlled paths (v2 port of #469) - #501
Closed
fresh-fx59 wants to merge 3 commits into
Closed
fresh-fx59 wants to merge 3 commits into
fresh-fx59 wants to merge 3 commits into
Conversation
resolve(path:in:) now rejects empty, absolute, '.' and '..' paths and any symlinked component (broken links included), and checks the real path stays under the bundle. load() and write() validate every path field; the hand-joined disk paths go through resolve. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Guest names must be a single plain component. Downloads create folders one level at a time with O_NOFOLLOW and write through an O_CREAT|O_EXCL|O_NOFOLLOW temp file renamed into place; drag-out gets the same name check. The guest HTTP client caps in-memory bodies at 64 MiB and moves file content in 64 KiB chunks to and from a FileHandle. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Guest-volume writes run as root. A helper now walks each path from the mount root with openat(O_NOFOLLOW|O_DIRECTORY) and finishes with fchmodat(AT_SYMLINK_NOFOLLOW), O_EXCL|O_NOFOLLOW temp files and renameat. setMode, copies, moves, symlink creation, tree removal and the in-place launchd_cache_loader patch go through it. copyfile uses COPYFILE_NOFOLLOW and failures throw. Images attach with -nobrowse -owners on under a private 0700 -mountrandom directory, and statfs must show the expected device before any write. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
fresh-fx59
force-pushed
the
fix/path-integrity-v2
branch
from
September 25, 2026 10:19
d0c2de1 to
ece480e
Compare
Owner
|
You have to resolve conflict before submit it again, and a fix is alreay on the branch. |
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
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.
Follow-up to #469, per #469 (comment) ("if problem still exists, feel free to ask me open this again").
I re-checked the v2 rewrite (tag
2.0.1). Three of the four issues were still present and one was partial, so this is a fresh port onto currentmaininstead of reopening the old branch.What was still open in 2.0.1
VPhoneVirtualMachineManifest.resolve(path:in:)only appended the path;load()/write()did not checkdiskImage,nvramStorage,sepStorage,romImages.*.VPhoneBundle.swiftandVPhoneVirtualMachineLaunchCommand.swiftjoined the disk path by hand.VPhoneRemoteFile) were not checked for/,..or empty;downloadFile/downloadDirectory/drag-out wrote withData.write/createDirectory(withIntermediateDirectories:), following existing symlinks in the destination.FirmwarePatcher/CryptexFilesystem) —setMode, copy/move and the in-placelaunchd_cache_loaderpatch resolved paths through any symlink in the guest volume;copyfilehad noCOPYFILE_NOFOLLOWand failures were only printed; volumes mounted in shared/Volumeswith no device check.Data.Changes
resolvenow throws; rejects empty/absolute paths,..components and symlinked components (including dangling), and confirms the real path stays under the bundle. Checked on load and write; hand-joined callers go throughresolve.VPhoneHostSafeFile(name validation, one-level no-follow directory creation, temp file withO_CREAT|O_EXCL|O_NOFOLLOW+ rename). Used by the file browser and drag-out.withGuestParentwalks withopenat(O_NOFOLLOW|O_DIRECTORY); final ops are no-follow (fchmodat(AT_SYMLINK_NOFOLLOW),openat,renameat).copyfileusesCOPYFILE_NOFOLLOWand throws on failure.hdiutil attach -nobrowse -owners on -mountrandom <0700 dir>plus astatfscheck that the mount comes from the expected device.downloadFile(path:to:)/uploadFile(path:from:)in 64 KiB chunks, used by the file browser.Validation
buildworkflow (VPhone.bundle build + bundle validation) passed on the fork: https://github.com/fresh-fx59/vphone-cli/actions/runs/36123354867 (after rebase onto 2290c7b)xcodebuild test -scheme VPhoneCoreKitTestson macos-26:Test run with 170 tests in 29 suites passed, including newManifestPathTestsandHostSafeFileTests.Not yet verified: a full restore/CFW install on a real VM (mount + guest-volume writes) and live file transfers against a running guest. Still path-based and out of scope here:
VPhoneSigner.sign(fileAt:),deleteAppleDoubleFiles, the mobileactivationd patch.🤖 Generated with Claude Code