Archiver: use secure archive extraction

Author: mazdak

Package.swift

@@ -194,6 +194,7 @@ let package = Package(
             name: "ContainerAPIClientTests",
             dependencies: [
                 .product(name: "Containerization", package: "containerization"),
+                .product(name: "ContainerizationArchive", package: "containerization"),
                 "ContainerAPIClient",
                 "ContainerPersistence",
             ]

Sources/Services/ContainerAPIService/Client/Archiver.swift

@@ -158,86 +158,14 @@ public final class Archiver: Sendable {
         let source = source.standardizedFileURL
         let destination = destination.standardizedFileURL
 
-        // TODO: ArchiveReader needs some enhancement to support buffered uncompression
         let reader = try ArchiveReader(
             format: .paxRestricted,
             filter: .gzip,
             file: source
         )
-
-        for (entry, data) in reader {
-            guard let path = entry.path else {
-                continue
-            }
-            let uncompressPath = destination.appendingPathComponent(path)
-
-            let fileManager = FileManager.default
-            switch entry.fileType {
-            case .blockSpecial, .characterSpecial, .socket:
-                continue
-            case .directory:
-                try fileManager.createDirectory(
-                    at: uncompressPath,
-                    withIntermediateDirectories: true,
-                    attributes: [
-                        FileAttributeKey.posixPermissions: entry.permissions
-                    ]
-                )
-            case .regular:
-                try fileManager.createDirectory(
-                    at: uncompressPath.deletingLastPathComponent(),
-                    withIntermediateDirectories: true,
-                    attributes: [
-                        FileAttributeKey.posixPermissions: 0o755
-                    ]
-                )
-                let success = fileManager.createFile(
-                    atPath: uncompressPath.path,
-                    contents: data,
-                    attributes: [
-                        FileAttributeKey.posixPermissions: entry.permissions
-                    ]
-                )
-                if !success {
-                    throw POSIXError.fromErrno()
-                }
-                try data.write(to: uncompressPath)
-            case .symbolicLink:
-                guard let target = entry.symlinkTarget else {
-                    continue
-                }
-                try fileManager.createDirectory(
-                    at: uncompressPath.deletingLastPathComponent(),
-                    withIntermediateDirectories: true,
-                    attributes: [
-                        FileAttributeKey.posixPermissions: 0o755
-                    ]
-                )
-                try fileManager.createSymbolicLink(atPath: uncompressPath.path, withDestinationPath: target)
-                continue
-            default:
-                continue
-            }
-
-            // FIXME: uid/gid for compress.
-            try fileManager.setAttributes(
-                [.posixPermissions: NSNumber(value: entry.permissions)],
-                ofItemAtPath: uncompressPath.path
-            )
-
-            if let creationDate = entry.creationDate {
-                try fileManager.setAttributes(
-                    [.creationDate: creationDate],
-                    ofItemAtPath: uncompressPath.path
-                )
-            }
-
-            if let modificationDate = entry.modificationDate {
-                try fileManager.setAttributes(
-                    [.modificationDate: modificationDate],
-                    ofItemAtPath: uncompressPath.path
-                )
-            }
+        let rejectedMembers = try reader.extractContents(to: destination)
+        if !rejectedMembers.isEmpty {
+            throw Error.rejectedArchiveMembers(rejectedMembers)
         }
     }
 
@@ -248,18 +176,14 @@ public final class Archiver: Sendable {
         writerConfiguration: ArchiveWriterConfiguration,
         hasher: inout SHA256
     ) throws {
-        let archivedPathsByHostPath = entryInfo.reduce(into: [String: [URL]]()) { result, info in
-            result[info.pathOnHost.path, default: []].append(info.pathInArchive)
-        }
-
         let archiver = try ArchiveWriter(configuration: writerConfiguration)
         try archiver.open(file: destination)
 
         let encoder = JSONEncoder()
         encoder.outputFormatting = .sortedKeys
 
         for info in entryInfo {
-            guard let entry = try Self._createEntry(entryInfo: info, archivedPathsByHostPath: archivedPathsByHostPath) else {
+            guard let entry = try Self._createEntry(entryInfo: info) else {
                 throw Error.failedToCreateEntry
             }
             let hashInfo = ArchiveEntryHashInfo(
@@ -324,7 +248,6 @@ public final class Archiver: Sendable {
 
     private static func _createEntry(
         entryInfo: ArchiveEntryInfo,
-        archivedPathsByHostPath: [String: [URL]] = [:],
         pathPrefix: String = ""
     ) throws -> WriteEntry? {
         let entry = WriteEntry()
@@ -341,11 +264,8 @@ public final class Archiver: Sendable {
         case .symbolicLink:
             entry.fileType = .symbolicLink
             entry.size = 0
-            entry.symlinkTarget = Self._rewriteArchivedAbsoluteSymlinkTarget(
-                status.symlinkTarget ?? "",
-                entryInfo: entryInfo,
-                archivedPathsByHostPath: archivedPathsByHostPath
-            )
+            // Match Docker build-context semantics and preserve the original target verbatim.
+            entry.symlinkTarget = status.symlinkTarget
         }
 
         #if os(macOS)
@@ -469,51 +389,6 @@ public final class Archiver: Sendable {
         return trimmedPath
     }
 
-    private static func _rewriteArchivedAbsoluteSymlinkTarget(
-        _ symlinkTarget: String,
-        entryInfo: ArchiveEntryInfo,
-        archivedPathsByHostPath: [String: [URL]]
-    ) -> String {
-        guard symlinkTarget.hasPrefix("/") else {
-            return symlinkTarget
-        }
-
-        let targetPath = URL(fileURLWithPath: symlinkTarget)
-            .standardizedFileURL
-            .resolvingSymlinksInPath()
-            .path
-        guard let targetArchivePaths = archivedPathsByHostPath[targetPath], targetArchivePaths.count == 1, let targetArchivePath = targetArchivePaths.first else {
-            return symlinkTarget
-        }
-
-        let sourceDirectory = entryInfo.pathInArchive.deletingLastPathComponent().relativePath
-        return Self._relativeArchivePath(fromDirectory: sourceDirectory, to: targetArchivePath.relativePath)
-    }
-
-    private static func _relativeArchivePath(fromDirectory: String, to path: String) -> String {
-        let fromComponents = Self._archivePathComponents(fromDirectory)
-        let toComponents = Self._archivePathComponents(path)
-
-        var commonPrefixCount = 0
-        while commonPrefixCount < fromComponents.count,
-            commonPrefixCount < toComponents.count,
-            fromComponents[commonPrefixCount] == toComponents[commonPrefixCount]
-        {
-            commonPrefixCount += 1
-        }
-
-        let upwardTraversal = Array(repeating: "..", count: fromComponents.count - commonPrefixCount)
-        let remainder = Array(toComponents.dropFirst(commonPrefixCount))
-        let relativeComponents = upwardTraversal + remainder
-        return relativeComponents.isEmpty ? "." : relativeComponents.joined(separator: "/")
-    }
-
-    private static func _archivePathComponents(_ path: String) -> [String] {
-        NSString(string: path).pathComponents.filter { component in
-            component != "/" && component != "."
-        }
-    }
-
     private static func _isSymbolicLink(_ path: URL) throws -> Bool {
         let resourceValues = try path.resourceValues(forKeys: [.isSymbolicLinkKey])
         if let isSymbolicLink = resourceValues.isSymbolicLink {
@@ -526,16 +401,19 @@ public final class Archiver: Sendable {
 }
 
 extension Archiver {
-    public enum Error: Swift.Error, CustomStringConvertible {
+    public enum Error: Swift.Error, CustomStringConvertible, Equatable {
         case failedToCreateEntry
         case fileDoesNotExist(_ url: URL)
+        case rejectedArchiveMembers([String])
 
         public var description: String {
             switch self {
             case .failedToCreateEntry:
                 return "failed to create entry"
             case .fileDoesNotExist(let url):
                 return "file \(url.path) does not exist"
+            case .rejectedArchiveMembers(let members):
+                return "rejected archive members: \(members.joined(separator: ", "))"
             }
         }
     }

Tests/ContainerAPIClientTests/ArchiverTests.swift

@@ -17,9 +17,55 @@
 import Foundation
 import Testing
 
+import ContainerizationArchive
+
 @testable import ContainerAPIClient
 
 struct ArchiverTests {
+    private enum EntryType {
+        case regular(String)
+        case directory
+        case symlink(String)
+    }
+
+    private func createTestArchive(
+        name: String,
+        entries: [(path: String, type: EntryType)],
+        baseDirectory: URL
+    ) throws -> URL {
+        let archiveURL = baseDirectory.appendingPathComponent("\(name).tar.gz")
+        let archiver = try ArchiveWriter(format: .paxRestricted, filter: .gzip, file: archiveURL)
+
+        for entry in entries {
+            let writeEntry = WriteEntry()
+            writeEntry.path = entry.path
+            writeEntry.permissions = 0o644
+            writeEntry.owner = 1000
+            writeEntry.group = 1000
+
+            switch entry.type {
+            case .regular(let content):
+                writeEntry.fileType = .regular
+                let data = try #require(content.data(using: .utf8))
+                writeEntry.size = numericCast(data.count)
+                try archiver.writeEntry(entry: writeEntry, data: data)
+            case .directory:
+                writeEntry.fileType = .directory
+                writeEntry.permissions = 0o755
+                writeEntry.size = 0
+                try archiver.writeEntry(entry: writeEntry, data: nil)
+            case .symlink(let target):
+                writeEntry.fileType = .symbolicLink
+                writeEntry.symlinkTarget = target
+                writeEntry.size = 0
+                try archiver.writeEntry(entry: writeEntry, data: nil)
+            }
+        }
+
+        try archiver.finishEncoding()
+        return archiveURL
+    }
+
     @Test
     func testCompressAndUncompressPreservesRelativeSymbolicLink() throws {
         let fileManager = FileManager.default
@@ -100,7 +146,7 @@ struct ArchiverTests {
     }
 
     @Test
-    func testCompressAndUncompressRewritesArchivedAbsoluteSymbolicLinkTarget() throws {
+    func testCompressAndUncompressPreservesInternalAbsoluteSymbolicLinkTarget() throws {
         let fileManager = FileManager.default
         let tempURL = try fileManager.url(
             for: .itemReplacementDirectory,
@@ -135,12 +181,12 @@ struct ArchiverTests {
         let extractedLinkURL = destinationURL.appendingPathComponent("link.txt")
         let values = try extractedLinkURL.resourceValues(forKeys: [.isSymbolicLinkKey])
         #expect(values.isSymbolicLink == true)
-        #expect(try fileManager.destinationOfSymbolicLink(atPath: extractedLinkURL.path) == "target.txt")
+        #expect(try fileManager.destinationOfSymbolicLink(atPath: extractedLinkURL.path) == targetURL.path)
         #expect(try String(contentsOf: extractedLinkURL, encoding: .utf8) == "hello")
     }
 
     @Test
-    func testCompressAndUncompressRewritesArchivedAbsoluteSymbolicLinkTargetThroughSymlinkedAncestor() throws {
+    func testCompressAndUncompressPreservesAbsoluteSymbolicLinkTargetThroughSymlinkedAncestor() throws {
         let fileManager = FileManager.default
         let tempURL = try fileManager.url(
             for: .itemReplacementDirectory,
@@ -184,7 +230,10 @@ struct ArchiverTests {
         let extractedLinkURL = destinationURL.appendingPathComponent("link.txt")
         let values = try extractedLinkURL.resourceValues(forKeys: [.isSymbolicLinkKey])
         #expect(values.isSymbolicLink == true)
-        #expect(try fileManager.destinationOfSymbolicLink(atPath: extractedLinkURL.path) == "real/target.txt")
+        #expect(
+            try fileManager.destinationOfSymbolicLink(atPath: extractedLinkURL.path)
+                == sourceURL.appendingPathComponent("alias/target.txt").path
+        )
         #expect(try String(contentsOf: extractedLinkURL, encoding: .utf8) == "hello")
     }
 
@@ -259,6 +308,35 @@ struct ArchiverTests {
         #expect(!fileManager.fileExists(atPath: destinationURL.appendingPathComponent("exclude.txt").path))
     }
 
+    @Test
+    func testUncompressRejectsPathTraversalMembers() throws {
+        let fileManager = FileManager.default
+        let tempURL = try fileManager.url(
+            for: .itemReplacementDirectory,
+            in: .userDomainMask,
+            appropriateFor: .temporaryDirectory,
+            create: true
+        )
+        defer { try? fileManager.removeItem(at: tempURL) }
+
+        let archiveURL = try createTestArchive(
+            name: "traversal",
+            entries: [
+                (path: "safe.txt", type: .regular("safe")),
+                (path: "../outside.txt", type: .regular("evil")),
+            ],
+            baseDirectory: tempURL
+        )
+        let destinationURL = tempURL.appendingPathComponent("destination")
+
+        #expect(throws: Archiver.Error.rejectedArchiveMembers(["../outside.txt"])) {
+            try Archiver.uncompress(source: archiveURL, destination: destinationURL)
+        }
+
+        #expect(fileManager.fileExists(atPath: destinationURL.appendingPathComponent("safe.txt").path))
+        #expect(!fileManager.fileExists(atPath: tempURL.appendingPathComponent("outside.txt").path))
+    }
+
     private func archiveDigest(sourceURL: URL, destinationURL: URL) throws -> String {
         let digest = try Archiver.compress(source: sourceURL, destination: destinationURL) { url in
             let sourcePath = sourceURL.standardizedFileURL.path