From 5cde6eac24ff7f903eb2bef9d9a0ae9ac9b102e9 Mon Sep 17 00:00:00 2001 From: Maksim Romanov Date: Sun, 9 Aug 2026 12:33:57 +0300 Subject: [PATCH] ArchiveReader: Preserve special permission bits during extraction ArchiveReader.setFileAttributes was losing the sticky/set-user-ID/set-group-ID bits on every extracted regular file and directory, for two independent reasons: 1. #816 masked the applied fchmod mode to 0o777 to strip file-type bits picked up from the archived mode, but 0o777 also strips the sticky/set-user-ID/set-group-ID bits (0o7000) that extraction should keep. 2. setFileAttributes called fchmod before fchown. On Darwin, fchown clears set-user-ID/set-group-ID as a security measure, even when the new owner/group match the file's existing owner/group, so applying ownership after the mode silently drops those bits regardless of (1). Fix: apply ownership first, then fchmod with a 0o7777 mask (the full POSIX permission field, still excluding unrelated file-type bits). Added preserveSpecialPermissionBits() in ArchiveReaderTests.swift: extracts a 0o1777 directory and a 0o6755 regular file (owned by the current uid/gid so fchown succeeds without root) and asserts both survive extraction. Verified the test fails without the fix (reproducing the truncated values reported in the issues) and passes with it. swift test --filter ContainerizationArchiveTests (51 tests) and swift format lint --strict both clean. Closes apple/containerization#818 Closes apple/containerization#819 --- .../ArchiveReader.swift | 8 ++- .../ArchiveReaderTests.swift | 53 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/Sources/ContainerizationArchive/ArchiveReader.swift b/Sources/ContainerizationArchive/ArchiveReader.swift index 3c291d1a5..2ae415f89 100644 --- a/Sources/ContainerizationArchive/ArchiveReader.swift +++ b/Sources/ContainerizationArchive/ArchiveReader.swift @@ -398,10 +398,16 @@ extension ArchiveReader { } private func setFileAttributes(fd: Int32, entry: WriteEntry) { - fchmod(fd, entry.permissions & 0o777) + // Ownership must be set before the permission mode: Darwin's fchown clears + // the set-user-ID and set-group-ID bits, even when chowning to the file's + // existing owner/group, so applying it after fchmod would silently drop + // those bits. if let owner = entry.owner, let group = entry.group { fchown(fd, owner, group) } + // Mask to the POSIX permission field (0o7777: owner/group/other rwx plus + // set-user-ID, set-group-ID, and sticky), excluding unrelated file-type bits. + fchmod(fd, entry.permissions & 0o7777) } private static func copyDataReaderToFd(dataReader: ArchiveEntryReader, fileFd: Int32, memberPath: FilePath) throws { diff --git a/Tests/ContainerizationArchiveTests/ArchiveReaderTests.swift b/Tests/ContainerizationArchiveTests/ArchiveReaderTests.swift index c7c0e563a..466b8200b 100644 --- a/Tests/ContainerizationArchiveTests/ArchiveReaderTests.swift +++ b/Tests/ContainerizationArchiveTests/ArchiveReaderTests.swift @@ -438,6 +438,59 @@ struct ArchiveReaderTests { #expect((perms & permMask) == 0o755, "Permissions should be preserved") } + @Test func preserveSpecialPermissionBits() throws { + // Extraction must preserve the sticky/set-user-ID/set-group-ID bits, and + // ownership must be applied before the permission mode: Darwin's fchown + // clears set-user-ID/set-group-ID even when chowning a file to its own + // existing owner/group. Use the current process's real uid/gid so the + // fchown below is permitted without requiring root. + let uid = getuid() + let gid = getgid() + + let testDirectory = createTemporaryDirectory(baseName: "ArchiveReaderTests")! + let archiveURL = testDirectory.appendingPathComponent("special-perms.tar") + let archiver = try ArchiveWriter(format: .paxRestricted, filter: .none, file: archiveURL) + + let dirEntry = WriteEntry() + dirEntry.path = "sticky-dir" + dirEntry.fileType = .directory + dirEntry.permissions = 0o1777 + dirEntry.owner = uid + dirEntry.group = gid + dirEntry.size = 0 + try archiver.writeEntry(entry: dirEntry, data: nil) + + let fileEntry = WriteEntry() + fileEntry.path = "setid-file" + fileEntry.fileType = .regular + fileEntry.permissions = 0o6755 + fileEntry.owner = uid + fileEntry.group = gid + let data = Data("setuid/setgid content".utf8) + fileEntry.size = numericCast(data.count) + try archiver.writeEntry(entry: fileEntry, data: data) + + try archiver.finishEncoding() + defer { try? FileManager.default.removeItem(at: testDirectory) } + + let extractDir = try createExtractionDirectory(name: "special-perms") + 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 permMask: UInt16 = 0o7777 + + let dirAttrs = try FileManager.default.attributesOfItem(atPath: extractDir.appendingPathComponent("sticky-dir").path) + let dirPerms = (dirAttrs[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 + #expect((dirPerms & permMask) == 0o1777, "Sticky bit should be preserved on directories") + + let fileAttrs = try FileManager.default.attributesOfItem(atPath: extractDir.appendingPathComponent("setid-file").path) + let filePerms = (fileAttrs[.posixPermissions] as? NSNumber)?.uint16Value ?? 0 + #expect((filePerms & permMask) == 0o6755, "Set-ID bits should be preserved after ownership is applied") + } + // MARK: - Duplicate Entry Tests @Test func duplicateRegularFiles() throws {