diff --git a/Platform/UTMDownloadVMTask.swift b/Platform/UTMDownloadVMTask.swift index 9f9edfaf9e..958e796308 100644 --- a/Platform/UTMDownloadVMTask.swift +++ b/Platform/UTMDownloadVMTask.swift @@ -85,12 +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.contains(utmDirectoryEnding) && !$0.path.hasSuffix(utmDirectoryEnding) && !$0.path.contains("__MACOSX") }) - for file in containedFiles { - let relativePath = file.path.replacingOccurrences(of: utmFolderInZip.path, with: "") - let isDirectory = file.path.hasSuffix("/") - _ = try archive.extract(file, to: destinationURL.appendingPathComponent(relativePath, isDirectory: isDirectory), 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 { @@ -98,13 +110,39 @@ class UTMDownloadVMTask: UTMDownloadTask { } } - private class UnzipNoUTMFileError: Error { + /// 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: 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: 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") } 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" } } ],