diff --git a/Sources/ContainerizationArchive/ArchiveReader.swift b/Sources/ContainerizationArchive/ArchiveReader.swift index 3c291d1a5..dcc18bab4 100644 --- a/Sources/ContainerizationArchive/ArchiveReader.swift +++ b/Sources/ContainerizationArchive/ArchiveReader.swift @@ -249,6 +249,29 @@ extension ArchiveReader: Sequence { } extension ArchiveReader { + private struct DirectoryIdentity: Equatable { + let device: UInt64 + let inode: UInt64 + } + + private struct DeferredDirectoryAttributes { + let entry: WriteEntry + let identity: DirectoryIdentity + } + + private struct ExtractionResult { + let extracted: Bool + let deferredDirectoryAttributes: DeferredDirectoryAttributes? + + static var rejected: ExtractionResult { + ExtractionResult(extracted: false, deferredDirectoryAttributes: nil) + } + + static var extracted: ExtractionResult { + ExtractionResult(extracted: true, deferredDirectoryAttributes: nil) + } + } + public convenience init(name: String, bundle: Data, tempDirectoryBaseName: String? = nil) throws { let baseName = tempDirectoryBaseName ?? "Unarchiver" guard let tempDir = createTemporaryDirectory(baseName: baseName) else { @@ -284,6 +307,7 @@ extension ArchiveReader { // Iterate and extract archive entries, collecting rejected paths. var foundEntry = false var rejectedPaths = [String]() + var deferredDirAttrs: [FilePath: DeferredDirectoryAttributes] = [:] for (entry, dataReader) in self.makeStreamingIterator() { guard let memberPath = (entry.path.map { FilePath($0) }) else { continue @@ -291,14 +315,19 @@ extension ArchiveReader { foundEntry = true // Try to extract the entry, catching path validation errors - let extracted = try extractEntry( + let result = try extractEntry( entry: entry, dataReader: dataReader, memberPath: memberPath, rootFileDescriptor: rootFileDescriptor ) - if !extracted { + if let deferredDirectoryAttributes = result.deferredDirectoryAttributes { + let deferredPath = FilePath().appending(memberPath.components).lexicallyNormalized() + deferredDirAttrs[deferredPath] = deferredDirectoryAttributes + } + + if !result.extracted { rejectedPaths.append(memberPath.string) } } @@ -306,6 +335,27 @@ extension ArchiveReader { throw ArchiveError.failedToExtractArchive("no entries found in archive") } + // Apply directory permissions after all children are extracted, deepest first, + // so a restrictive parent cannot block access to its children. + for deferred in deferredDirAttrs.sorted(by: { $0.key.components.count > $1.key.components.count }) { + do { + try FileDescriptorOps.withOpenDirectory(rootFileDescriptor, deferred.key) { fd in + guard try Self.directoryIdentity(fd) == deferred.value.identity else { + return + } + setFileAttributes(fd: fd.rawValue, entry: deferred.value.entry) + } + } catch let error as FileDescriptorOps.Error { + switch error { + case .invalidPathComponent, .cannotFollowSymlink: + // A later archive entry replaced this directory. Last entry wins. + continue + case .invalidRelativePath, .systemError: + throw error + } + } + } + return rejectedPaths } @@ -336,9 +386,9 @@ extension ArchiveReader { dataReader: ArchiveEntryReader, memberPath: FilePath, rootFileDescriptor: FileDescriptor - ) throws -> Bool { + ) throws -> ExtractionResult { guard let lastComponent = memberPath.lastComponent else { - return false + return .rejected } let relativePath = memberPath.removingLastComponent() let type = entry.fileType @@ -361,13 +411,22 @@ extension ArchiveReader { try Self.copyDataReaderToFd(dataReader: dataReader, fileFd: fileFd, memberPath: memberPath) setFileAttributes(fd: fileFd, entry: entry) } + return .extracted case .directory: + var deferredDirectoryAttributes: DeferredDirectoryAttributes? try FileDescriptorOps.mkdir(rootFileDescriptor, memberPath, makeIntermediates: true) { fd in - setFileAttributes(fd: fd.rawValue, entry: entry) + deferredDirectoryAttributes = DeferredDirectoryAttributes( + entry: entry, + identity: try Self.directoryIdentity(fd) + ) } + return ExtractionResult( + extracted: true, + deferredDirectoryAttributes: deferredDirectoryAttributes + ) case .symbolicLink: guard let targetPath = (entry.symlinkTarget.map { FilePath($0) }) else { - return false + return .rejected } var symlinkCreated = false try FileDescriptorOps.mkdir(rootFileDescriptor, relativePath, makeIntermediates: true) { fd in @@ -379,12 +438,10 @@ extension ArchiveReader { } symlinkCreated = true } - return symlinkCreated + return symlinkCreated ? .extracted : .rejected default: - return false + return .rejected } - - return true } catch let error as FileDescriptorOps.Error { // Just reject path validation errors, don't fail the extraction switch error { @@ -392,11 +449,22 @@ extension ArchiveReader { // Fail for system errors throw error case .invalidRelativePath, .invalidPathComponent, .cannotFollowSymlink: - return false + return .rejected } } } + private static func directoryIdentity(_ fd: FileDescriptor) throws -> DirectoryIdentity { + var attributes = stat() + guard fstat(fd.rawValue, &attributes) == 0 else { + throw FileDescriptorOps.Error.systemError("fstat during archive directory extraction", errno) + } + return DirectoryIdentity( + device: UInt64(attributes.st_dev), + inode: UInt64(attributes.st_ino) + ) + } + private func setFileAttributes(fd: Int32, entry: WriteEntry) { fchmod(fd, entry.permissions & 0o777) if let owner = entry.owner, let group = entry.group { diff --git a/Sources/ContainerizationOS/FileDescriptorOps.swift b/Sources/ContainerizationOS/FileDescriptorOps.swift index 4ade9a53f..465bdf26d 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps.swift @@ -111,6 +111,19 @@ public enum FileDescriptorOps { ) } + /// Opens an existing directory relative to `fd` without following symbolic links. + /// + /// Each path component must already exist and be a directory. Unlike ``mkdir``, + /// this operation never creates or replaces filesystem entries. + public static func withOpenDirectory( + _ fd: FileDescriptor, + _ relativePath: FilePath, + completion: (FileDescriptor) throws -> Void + ) throws { + try validateRelativePath(relativePath) + try withOpenDirectory(fd, relativePath.components, completion: completion) + } + /// Recursively removes a direct child of the directory at `fd`. /// /// - Parameters: @@ -269,6 +282,37 @@ public enum FileDescriptorOps { permissions: permissions, makeIntermediates: makeIntermediates, completion: completion) } + private static func withOpenDirectory( + _ fd: FileDescriptor, + _ relativeComponents: FilePath.ComponentView, + completion: (FileDescriptor) throws -> Void + ) throws { + guard let currentComponent = relativeComponents.first else { + try completion(fd) + return + } + + let componentFd = openat(fd.rawValue, currentComponent.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY) + guard componentFd >= 0 else { + switch errno { + case ELOOP: + throw Error.cannotFollowSymlink + case ENOENT, ENOTDIR: + throw Error.invalidPathComponent + default: + throw Error.systemError("directory open during file descriptor traversal", errno) + } + } + + let componentFileDescriptor = FileDescriptor(rawValue: componentFd) + defer { try? componentFileDescriptor.close() } + try withOpenDirectory( + componentFileDescriptor, + FilePath.ComponentView(relativeComponents.dropFirst()), + completion: completion + ) + } + private static func enumerateHelper( _ fd: FileDescriptor, relativePath: FilePath, diff --git a/Tests/ContainerizationArchiveTests/ArchiveReaderTests.swift b/Tests/ContainerizationArchiveTests/ArchiveReaderTests.swift index c7c0e563a..df18acf13 100644 --- a/Tests/ContainerizationArchiveTests/ArchiveReaderTests.swift +++ b/Tests/ContainerizationArchiveTests/ArchiveReaderTests.swift @@ -534,6 +534,113 @@ struct ArchiveReaderTests { #expect(content == "content", "Should have file content") } + @Test func deferredDirectoryAttributesDoNotFollowReplacedParentSymlink() throws { + let externalRoot = createTemporaryDirectory(baseName: "ArchiveReaderTests.external")! + defer { try? FileManager.default.removeItem(at: externalRoot) } + let externalChild = externalRoot.appendingPathComponent("child") + try FileManager.default.createDirectory(at: externalChild, withIntermediateDirectories: true) + try FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: externalChild.path) + + let archiveURL = try createTestArchive( + name: "deferred-attrs-replaced-parent", + entries: [ + ("parent/child/", .directory, nil), + ("parent", .symlink, externalRoot.path), + ]) + defer { try? FileManager.default.removeItem(at: archiveURL.deletingLastPathComponent()) } + + let extractDir = try createExtractionDirectory(name: "deferred-attrs-replaced-parent") + defer { try? FileManager.default.removeItem(at: extractDir.deletingLastPathComponent()) } + + let reader = try ArchiveReader(format: .paxRestricted, filter: .none, file: archiveURL) + let rejectedPaths = try reader.extractContents(to: extractDir) + + #expect(rejectedPaths.isEmpty) + let attributes = try FileManager.default.attributesOfItem(atPath: externalChild.path) + let permissions = (attributes[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 + #expect((permissions & 0o777) == 0o700, "deferred attributes escaped the extraction root") + } + + @Test func duplicateDirectoryUsesLastDeferredAttributes() throws { + let testDirectory = createTemporaryDirectory(baseName: "ArchiveReaderTests.duplicateDirectory")! + defer { try? FileManager.default.removeItem(at: testDirectory) } + let archiveURL = testDirectory.appendingPathComponent("duplicate-directory.tar") + let writer = try ArchiveWriter(format: .paxRestricted, filter: .none, file: archiveURL) + + let entries: [(path: String, permissions: mode_t)] = [ + ("directory/", 0o000), + ("./directory/", 0o755), + ] + for (path, permissions) in entries { + let entry = WriteEntry() + entry.path = path + entry.fileType = .directory + entry.permissions = permissions + entry.size = 0 + try writer.writeEntry(entry: entry, data: nil) + } + try writer.finishEncoding() + + let extractDir = testDirectory.appendingPathComponent("extract") + let reader = try ArchiveReader(format: .paxRestricted, filter: .none, file: archiveURL) + let rejectedPaths = try reader.extractContents(to: extractDir) + + #expect(rejectedPaths.isEmpty) + let attributes = try FileManager.default.attributesOfItem( + atPath: extractDir.appendingPathComponent("directory").path) + let permissions = (attributes[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 + #expect((permissions & 0o777) == 0o755, "the last directory entry should define final permissions") + } + + @Test func replacedDirectoryDoesNotReceiveStaleDeferredAttributes() throws { + let testDirectory = createTemporaryDirectory(baseName: "ArchiveReaderTests.replacedDirectory")! + defer { try? FileManager.default.removeItem(at: testDirectory) } + let archiveURL = testDirectory.appendingPathComponent("replaced-directory.tar") + let writer = try ArchiveWriter(format: .paxRestricted, filter: .none, file: archiveURL) + + let directory = WriteEntry() + directory.path = "parent/child/" + directory.fileType = .directory + directory.permissions = 0o123 + directory.size = 0 + try writer.writeEntry(entry: directory, data: nil) + + let symlink = WriteEntry() + symlink.path = "parent" + symlink.fileType = .symbolicLink + symlink.symlinkTarget = "elsewhere" + symlink.size = 0 + try writer.writeEntry(entry: symlink, data: nil) + + let file = WriteEntry() + file.path = "parent/child/file" + file.fileType = .regular + file.permissions = 0o644 + let data = Data("content".utf8) + file.size = numericCast(data.count) + try writer.writeEntry(entry: file, data: data) + try writer.finishEncoding() + + let extractDir = testDirectory.appendingPathComponent("extract") + let reader = try ArchiveReader(format: .paxRestricted, filter: .none, file: archiveURL) + let rejectedPaths = try reader.extractContents(to: extractDir) + + let parent = extractDir.appendingPathComponent("parent") + let child = parent.appendingPathComponent("child") + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: child.path) } + + #expect(rejectedPaths.isEmpty) + let parentAttributes = try FileManager.default.attributesOfItem(atPath: parent.path) + let childAttributes = try FileManager.default.attributesOfItem(atPath: child.path) + let parentPermissions = (parentAttributes[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 + let childPermissions = (childAttributes[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 + #expect( + (childPermissions & 0o777) == (parentPermissions & 0o777), + "a replacement directory received stale attributes from an unlinked inode" + ) + #expect(try String(contentsOf: child.appendingPathComponent("file"), encoding: .utf8) == "content") + } + @Test func regularFileToSymlink() throws { let archiveURL = try createTestArchive( name: "file-to-symlink", diff --git a/Tests/ContainerizationArchiveTests/ArchiveTests.swift b/Tests/ContainerizationArchiveTests/ArchiveTests.swift index 079261b6c..f0d4e8cb4 100644 --- a/Tests/ContainerizationArchiveTests/ArchiveTests.swift +++ b/Tests/ContainerizationArchiveTests/ArchiveTests.swift @@ -242,6 +242,37 @@ struct ArchiveTests { #expect(try String(contentsOf: extractDir.appendingPathComponent("subdir/file2.txt"), encoding: .utf8) == "world") } + @Test func archiveDirectoryPreservesReadonlySubdirectory() throws { + let testDir = createTemporaryDirectory(baseName: "ArchiveTests.archiveDirReadonlySubdir")! + defer { try? FileManager.default.removeItem(at: testDir) } + + let sourceDir = testDir.appendingPathComponent("source") + let readonlyDir = sourceDir.appendingPathComponent("readonly") + try FileManager.default.createDirectory(at: readonlyDir, withIntermediateDirectories: true) + try "content".write(to: readonlyDir.appendingPathComponent("file.txt"), atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: readonlyDir.path) + defer { + try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: readonlyDir.path) + } + + let archiveURL = testDir.appendingPathComponent("test.tar.gz") + let writer = try ArchiveWriter(format: .pax, filter: .gzip, file: archiveURL) + try writer.archiveDirectory(sourceDir) + try writer.finishEncoding() + + let extractDir = testDir.appendingPathComponent("extract") + let reader = try ArchiveReader(file: archiveURL) + let rejected = try reader.extractContents(to: extractDir) + + #expect(rejected.isEmpty) + #expect( + try String(contentsOf: extractDir.appendingPathComponent("readonly/file.txt"), encoding: .utf8) + == "content") + let attrs = try FileManager.default.attributesOfItem(atPath: extractDir.appendingPathComponent("readonly").path) + let perms = (attrs[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 + #expect((perms & 0o777) == 0o555, "Read-only directory permissions should be preserved") + } + @Test func archiveDirectoryEmpty() throws { let testDir = createTemporaryDirectory(baseName: "ArchiveTests.archiveDirEmpty")! defer { try? FileManager.default.removeItem(at: testDir) } @@ -722,7 +753,7 @@ struct ArchiveTests { let readonlyDir = sourceDir.appendingPathComponent("readonly") try FileManager.default.createDirectory(at: readonlyDir, withIntermediateDirectories: true) try "content".write(to: readonlyDir.appendingPathComponent("file.txt"), atomically: true, encoding: .utf8) - try FileManager.default.setAttributes([.posixPermissions: 0o777], ofItemAtPath: readonlyDir.path) + try FileManager.default.setAttributes([.posixPermissions: 0o555], ofItemAtPath: readonlyDir.path) defer { try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: readonlyDir.path) } @@ -742,7 +773,7 @@ struct ArchiveTests { == "content") let attrs = try FileManager.default.attributesOfItem(atPath: extractDir.appendingPathComponent("readonly").path) let perms = (attrs[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 - #expect((perms & 0o777) == 0o777, "Read-only directory permissions should be preserved") + #expect((perms & 0o777) == 0o555, "Read-only directory permissions should be preserved") } @Test func archiveURLsSymlinks() throws { diff --git a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift index 6ac47410c..531e13736 100644 --- a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift +++ b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift @@ -186,6 +186,40 @@ struct FileDescriptorPathSecureTests { } } + @Test + func withOpenDirectoryDoesNotReplaceRegularFile() async throws { + let rootPath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: rootPath.string) } + try createEntries(rootPath: rootPath, entries: [.regular(path: "entry")], permissions: nil) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + #expect(throws: FileDescriptorOps.Error.invalidPathComponent) { + try FileDescriptorOps.withOpenDirectory(rootFd, FilePath("entry")) { _ in } + } + + var isDirectory: ObjCBool = false + #expect(FileManager.default.fileExists(atPath: rootPath.appending("entry").string, isDirectory: &isDirectory)) + #expect(!isDirectory.boolValue) + } + + @Test + func withOpenDirectoryTraversesExistingDirectories() async throws { + let rootPath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: rootPath.string) } + try createEntries(rootPath: rootPath, entries: [.directory(path: "parent/child")], permissions: nil) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + var opened = false + try FileDescriptorOps.withOpenDirectory(rootFd, FilePath("parent/child")) { _ in + opened = true + } + #expect(opened) + } + @Test( "Test paths with .. that normalize to valid paths", arguments: [