From ed73d11da0d72195731ddb55398cf02e2e24d57e Mon Sep 17 00:00:00 2001 From: osy <50960678+osy@users.noreply.github.com> Date: Tue, 1 Sep 2026 12:12:56 -0700 Subject: [PATCH 1/3] download: reject ZIP entries escaping the .utm folder A downloaded VM archive is attacker-controlled, so an entry named `some.utm/../../elsewhere` must not be able to write outside the package it claims to belong to. The extraction path was built by substituting the archive's .utm prefix out of the entry path and appending the remainder, and `appendingPathComponent()` does not resolve `..` -- the traversal was only resolved by the filesystem at write time, landing anywhere in the app container the depth reached. Derive the relative path by dropping the prefix, and resolve every destination through a containment check before extracting. Separators are collapsed in a loop because `fopen()` and `URL.standardized` disagree on a path such as `/../elsewhere`, and a single collapse pass still lets `///../elsewhere` through. Also bump ZIPFoundation to 0.9.20 for its accumulated fixes. This file is the only call site in the tree and the deprecated failable initializer it uses is still present, so the bump is source compatible. Reported-by: Vo Duc Thang (ugvxb) Assisted-by: Claude:claude-fable-5-1 --- Platform/UTMDownloadVMTask.swift | 33 +++++++++++++++++-- UTM.xcodeproj/project.pbxproj | 4 +-- .../xcshareddata/swiftpm/Package.resolved | 4 +-- 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/Platform/UTMDownloadVMTask.swift b/Platform/UTMDownloadVMTask.swift index 9f9edfaf9e..e509832c27 100644 --- a/Platform/UTMDownloadVMTask.swift +++ b/Platform/UTMDownloadVMTask.swift @@ -86,11 +86,12 @@ class UTMDownloadVMTask: UTMDownloadTask { /// create the .utm directory try fileManager.createDirectory(at: destinationURL, withIntermediateDirectories: false) /// get and extract all files contained in the UTM directory, except the `__MACOSX` folder - let containedFiles = archive.filter({ $0.path.contains(utmDirectoryEnding) && !$0.path.hasSuffix(utmDirectoryEnding) && !$0.path.contains("__MACOSX") }) + let containedFiles = archive.filter({ $0.path.hasPrefix(utmFolderInZip.path) && !$0.path.hasSuffix(utmDirectoryEnding) && !$0.path.contains("__MACOSX") }) for file in containedFiles { - let relativePath = file.path.replacingOccurrences(of: utmFolderInZip.path, with: "") + let relativePath = String(file.path.dropFirst(utmFolderInZip.path.count)) let isDirectory = file.path.hasSuffix("/") - _ = try archive.extract(file, to: destinationURL.appendingPathComponent(relativePath, isDirectory: isDirectory), skipCRC32: true) + let fileURL = try containedDestination(for: relativePath, in: destinationURL, isDirectory: isDirectory) + _ = try archive.extract(file, to: fileURL, skipCRC32: true) } return destinationURL } else { @@ -98,6 +99,32 @@ class UTMDownloadVMTask: UTMDownloadTask { } } + /// Resolve an archive entry's relative path inside `destinationFolder` and reject any escape. + /// + /// A crafted archive can name an entry `some.utm/../../elsewhere`. `appendingPathComponent()` does not + /// resolve `..`, so the traversal would only be resolved by the filesystem at write time. + private func containedDestination(for relativePath: String, in destinationFolder: URL, isDirectory: Bool) throws -> URL { + let candidate = destinationFolder.appendingPathComponent(relativePath, isDirectory: isDirectory) + /// POSIX `fopen()` collapses repeated separators before resolving `..`, so an entry named `/../elsewhere` + /// would otherwise standardize to a contained path here but escape once written. Collapse them first. + var path = candidate.path + while path.contains("//") { + path = path.replacingOccurrences(of: "//", with: "/") + } + let resolved = URL(fileURLWithPath: path, isDirectory: isDirectory).standardized + let root = URL(fileURLWithPath: destinationFolder.path, isDirectory: true).standardized + guard resolved.path.hasPrefix(root.path + "/") else { + throw UnzipUnsafePathError() + } + return resolved + } + + private class UnzipUnsafePathError: Error { + var errorDescription: String? { + NSLocalizedString("The downloaded ZIP archive contains an invalid path.", comment: "Error shown when importing a ZIP file from web that contains an entry pointing outside of the virtual machine directory.") + } + } + private class UnzipNoUTMFileError: Error { var errorDescription: String? { NSLocalizedString("There is no UTM file in the downloaded ZIP archive.", comment: "Error shown when importing a ZIP file from web that doesn't contain a UTM Virtual Machine.") diff --git a/UTM.xcodeproj/project.pbxproj b/UTM.xcodeproj/project.pbxproj index 9b93eff2bb..399aec735c 100644 --- a/UTM.xcodeproj/project.pbxproj +++ b/UTM.xcodeproj/project.pbxproj @@ -5509,7 +5509,7 @@ repositoryURL = "https://github.com/weichsel/ZIPFoundation.git"; requirement = { kind = upToNextMajorVersion; - minimumVersion = 0.9.17; + minimumVersion = 0.9.20; }; }; 84018693288B66370050AC51 /* XCRemoteSwiftPackageReference "swiftui-visual-effects" */ = { @@ -5653,7 +5653,7 @@ repositoryURL = "https://github.com/weichsel/ZIPFoundation.git"; requirement = { kind = upToNextMajorVersion; - minimumVersion = 0.9.17; + minimumVersion = 0.9.20; }; }; CEF7F58F2AEEDCC400E34952 /* XCRemoteSwiftPackageReference "SwiftTerm" */ = { diff --git a/UTM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/UTM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index b2a7ce9747..b819e38a42 100644 --- a/UTM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/UTM.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -132,8 +132,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/weichsel/ZIPFoundation.git", "state" : { - "revision" : "a3f5c2bae0f04b0bce9ef3c4ba6bd1031a0564c4", - "version" : "0.9.17" + "revision" : "22787ffb59de99e5dc1fbfe80b19c97a904ad48d", + "version" : "0.9.20" } } ], From a22bac70efea6492f007de126539df65d5d22440 Mon Sep 17 00:00:00 2001 From: osy <50960678+osy@users.noreply.github.com> Date: Tue, 1 Sep 2026 12:19:35 -0700 Subject: [PATCH 2/3] download: show the real reason a VM download failed The three error types in this file declare `errorDescription` but conform only to `Error`, so the description is never consulted. `downloadUTMZip()` alerts with `error.localizedDescription`, which for a plain `Error` falls back to "The operation couldn't be completed. (UTM.UTMDownloadVMTask... error 1.)" -- the localized strings here have never reached a user. Conform them to `LocalizedError` instead, matching every other error type in the codebase. No other type has this defect. `CreateUTMFailed` is currently unthrown; it is included so the three stay consistent rather than leaving one behind. Assisted-by: Claude:claude-fable-5-1 --- Platform/UTMDownloadVMTask.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Platform/UTMDownloadVMTask.swift b/Platform/UTMDownloadVMTask.swift index e509832c27..bc34fd331f 100644 --- a/Platform/UTMDownloadVMTask.swift +++ b/Platform/UTMDownloadVMTask.swift @@ -119,19 +119,19 @@ class UTMDownloadVMTask: UTMDownloadTask { return resolved } - private class UnzipUnsafePathError: Error { + private class UnzipUnsafePathError: LocalizedError { var errorDescription: String? { NSLocalizedString("The downloaded ZIP archive contains an invalid path.", comment: "Error shown when importing a ZIP file from web that contains an entry pointing outside of the virtual machine directory.") } } - private class UnzipNoUTMFileError: Error { + private class UnzipNoUTMFileError: LocalizedError { var errorDescription: String? { NSLocalizedString("There is no UTM file in the downloaded ZIP archive.", comment: "Error shown when importing a ZIP file from web that doesn't contain a UTM Virtual Machine.") } } - private class CreateUTMFailed: Error { + private class CreateUTMFailed: LocalizedError { var errorDescription: String? { NSLocalizedString("Failed to parse the downloaded VM.", comment: "UTMDownloadVMTask") } From 34d48fec66c4c296152a3f720bb8c027e123b503 Mon Sep 17 00:00:00 2001 From: osy <50960678+osy@users.noreply.github.com> Date: Tue, 1 Sep 2026 12:25:01 -0700 Subject: [PATCH 3/3] download: never extract a symlink from a downloaded VM Bumping ZIPFoundation to 0.9.20 brought in its symlink containment guard, but that guard is not sufficient here. `URL.isContained(in:)` collapses repeated separators in a single pass, so a link target ending in `///../elsewhere` is accepted while the filesystem resolves it to `/../elsewhere`. A crafted archive could pair such a link with a later regular file underneath it, whose own path passes the containment check, and write outside the package again. Nothing in UTM ever stores a symlink inside a package, so reject them outright instead of trying to validate the target. Also remove the destination on failure. Extraction created the .utm directory up front and a mid-loop error left the partial contents in place, where the library would list them as a real virtual machine that survives a relaunch. Assisted-by: Claude:claude-fable-5-1 --- Platform/UTMDownloadVMTask.swift | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/Platform/UTMDownloadVMTask.swift b/Platform/UTMDownloadVMTask.swift index bc34fd331f..958e796308 100644 --- a/Platform/UTMDownloadVMTask.swift +++ b/Platform/UTMDownloadVMTask.swift @@ -85,13 +85,24 @@ class UTMDownloadVMTask: UTMDownloadTask { let destinationURL = destinationFolder.appendingPathComponent(destinationUtmDirectory, isDirectory: true) /// create the .utm directory try fileManager.createDirectory(at: destinationURL, withIntermediateDirectories: false) - /// get and extract all files contained in the UTM directory, except the `__MACOSX` folder - let containedFiles = archive.filter({ $0.path.hasPrefix(utmFolderInZip.path) && !$0.path.hasSuffix(utmDirectoryEnding) && !$0.path.contains("__MACOSX") }) - for file in containedFiles { - let relativePath = String(file.path.dropFirst(utmFolderInZip.path.count)) - let isDirectory = file.path.hasSuffix("/") - let fileURL = try containedDestination(for: relativePath, in: destinationURL, isDirectory: isDirectory) - _ = try archive.extract(file, to: fileURL, skipCRC32: true) + do { + /// get and extract all files contained in the UTM directory, except the `__MACOSX` folder + let containedFiles = archive.filter({ $0.path.hasPrefix(utmFolderInZip.path) && !$0.path.hasSuffix(utmDirectoryEnding) && !$0.path.contains("__MACOSX") }) + for file in containedFiles { + /// we never store a symlink in a package, and a link target that survives a containment + /// check can still be resolved outside of the package when it is written through later + guard file.type != .symlink else { + throw UnzipUnsafePathError() + } + let relativePath = String(file.path.dropFirst(utmFolderInZip.path.count)) + let isDirectory = file.path.hasSuffix("/") + let fileURL = try containedDestination(for: relativePath, in: destinationURL, isDirectory: isDirectory) + _ = try archive.extract(file, to: fileURL, skipCRC32: true) + } + } catch { + /// a partially extracted package would still be picked up as a VM by the library + try? fileManager.removeItem(at: destinationURL) + throw error } return destinationURL } else {