Close ZIP validation/extraction race with private snapshots

README.md

@@ -30,7 +30,7 @@ Tuck is a native **Swift 6 macOS Finder context-menu utility** for file compress
 - **Non-destructive by default.** Compression, PDF/image tools and recipes publish separate copies; rename and Move To are explicitly confirmed. Generated-file Undo checks content hashes and file identity before using macOS Trash. It will refuse modified/replaced outputs, unverified old history, symlinks and directories.
 - **Decimal limits:** “Under 5 MB” means *less than 5,000,000 bytes*, not 5 MiB. Strict-size output might be impossible without unacceptable visual changes, especially for lossless formats; errors should be explicit.
 - **IPC:** Signed Finder requests have a hard limit of **256 paths and 64 KB of payload**. Larger or long-path selections are rejected with a warning instead of being silently truncated. This is not unlimited batch processing.
-- **ZIP inspector:** Supports a conservative subset of ZIP/Deflate; rejects encrypted/ZIP64, symlink entries, traversal paths and filenames that the underlying extractor could interpret as glob selectors. Inspection is limited to 10,000 entries and 2 GB expanded size.
+- **ZIP inspector:** Supports a conservative subset of ZIP/Deflate; rejects encrypted/ZIP64, symlink entries, traversal paths and filenames that the underlying extractor could interpret as glob selectors. Inspection is limited to 10,000 entries and 2 GB expanded size. Extraction first streams the archive into a private snapshot (maximum 2 GB compressed archive size) on the selected destination volume and validates that snapshot before invoking the extractor; allow temporary space for both the snapshot and the extracted content.
 - **Image/document integrity:** Animated/multiframe inputs are refused where processing would drop frames. OCR creates a searchable output but recognition is not guaranteed. Tuck is **not a secure PDF redaction utility**; digital signatures, forms, annotations, unusual metadata and color profiles can require external validation.
 - **Jobs:** Cancellation is cooperative and may leave already-published outputs visible in Activity. Persisted unfinished jobs are marked interrupted after restart; Tuck does not replay a previously running executable automatically.
 - **Custom actions:** Users who explicitly configure a script or Shortcut are authorizing it to operate with their own permissions. Tuck does not sandbox those user executables or guarantee they are non-destructive.

Sources/App/ZIP/ZipInspector.swift

@@ -108,6 +108,30 @@ public enum ZipInspector {
             guard (flags & 0x40) == 0, method == 0 || method == 8 else {
                 throw Invalid.malformed("Unsupported ZIP compression or encryption method.")
             }
+            // ZIP extractors may consult local headers, not just the central
+            // directory. Refuse archives whose two filename/method records
+            // disagree, before invoking an external extraction process.
+            guard local <= start, start - local >= 30 else {
+                throw Invalid.malformed("ZIP local entry overlaps its central directory.")
+            }
+            try handle.seek(toOffset: local)
+            let localHeader = [UInt8](try handle.read(upToCount: 30) ?? Data())
+            guard localHeader.count == 30,
+                  uint(localHeader, 0, 4) == 0x04034b50,
+                  let localFlags = uint(localHeader, 6, 2),
+                  let localMethod = uint(localHeader, 8, 2),
+                  let localNameLength = uint(localHeader, 26, 2),
+                  let localExtraLength = uint(localHeader, 28, 2),
+                  localFlags == flags, localMethod == method,
+                  localNameLength == nameLen,
+                  30 + localNameLength + localExtraLength <= start - local,
+                  compressed <= start - local - 30 - localNameLength - localExtraLength else {
+                throw Invalid.malformed("ZIP local header conflicts with its central directory.")
+            }
+            let localName = try handle.read(upToCount: Int(nameLen)) ?? Data()
+            guard localName == rawName else {
+                throw Invalid.malformed("ZIP local and central filenames disagree.")
+            }
             let mode = (external >> 16) & 0xF000
             guard mode != 0xA000 else {
                 throw Invalid.malformed("ZIP contains symbolic links; extraction refused.")
@@ -147,26 +171,64 @@ public enum ZipInspector {
         }
     }
 
+    /// Copy bytes into a private regular file rather than using copyItem,
+    /// which can reproduce a symbolic link instead of snapshotting its bytes.
+    /// Restrict memory to a single chunk and bound on-disk staging growth.
+    private static func makeSnapshot(from archive: URL, at snapshot: URL,
+                                     shouldCancel: @escaping @Sendable () -> Bool) throws {
+        let fm = FileManager.default
+        guard fm.createFile(atPath: snapshot.path, contents: nil,
+                            attributes: [.posixPermissions: 0o600]) else {
+            throw Invalid.malformed("Cannot create a private ZIP snapshot.")
+        }
+        let reader = try FileHandle(forReadingFrom: archive)
+        defer { try? reader.close() }
+        let writer = try FileHandle(forWritingTo: snapshot)
+        defer { try? writer.close() }
+
+        var written: UInt64 = 0
+        while let data = try reader.read(upToCount: 1_048_576), !data.isEmpty {
+            if shouldCancel() { throw CancellationError() }
+            guard UInt64(data.count) <= maxExpandedBytes - written else {
+                throw Invalid.malformed("ZIP archive exceeds the 2 GB snapshot safety limit.")
+            }
+            try writer.write(contentsOf: data)
+            written += UInt64(data.count)
+        }
+        if shouldCancel() { throw CancellationError() }
+    }
+
     /// Extract only preflight-validated entries into a fresh private staging
     /// directory beside the destination. The destination folder is published
     /// only after validation of the extracted file tree.
     public static func extract(archive: URL, destinationFolder: URL,
                                selected: Set<String>?,
                                shouldCancel: @escaping @Sendable () -> Bool = { false }) throws -> URL {
-        let listing = try inspect(archive)
-        let picked = listing.filter { selected?.contains($0.name) ?? true }
-        guard !picked.isEmpty else { throw Invalid.malformed("No ZIP entries selected.") }
         guard !shouldCancel() else { throw CancellationError() }
         let fm = FileManager.default
         let stage = destinationFolder.appendingPathComponent(
             ".tuck-zip-stage-\(UUID().uuidString)", isDirectory: true)
         try fm.createDirectory(at: stage, withIntermediateDirectories: false,
                                attributes: [.posixPermissions: 0o700])
         defer { try? fm.removeItem(at: stage) }
+
+        // Never let unzip reread the original after validation: the user or
+        // another process may replace that archive between inspection and
+        // extraction. Only an isolated private snapshot is handed to unzip.
+        let snapshot = stage.appendingPathComponent("validated.zip")
+        try makeSnapshot(from: archive, at: snapshot, shouldCancel: shouldCancel)
+        let listing = try inspect(snapshot)
+        if let selected, !selected.isSubset(of: Set(listing.map(\.name))) {
+            throw Invalid.malformed("The requested ZIP selection is no longer valid.")
+        }
+        let picked = listing.filter { selected?.contains($0.name) ?? true }
+        guard !picked.isEmpty else { throw Invalid.malformed("No ZIP entries selected.") }
+
+        let contents = stage.appendingPathComponent("contents", isDirectory: true)
+        try fm.createDirectory(at: contents, withIntermediateDirectories: false,
+                               attributes: [.posixPermissions: 0o700])
         let tool = Tool(name: "unzip", url: URL(fileURLWithPath: "/usr/bin/unzip"))
-        // The ZIP location is absolute. Arguments are passed individually,
-        // never interpolated into a shell command.
-        let arguments = ["-qq", "-n", archive.path] + picked.map(\.name) + ["-d", stage.path]
+        let arguments = ["-qq", "-n", snapshot.path] + picked.map(\.name) + ["-d", contents.path]
         do {
             try Toolchain.run(tool, arguments, timeout: 600, shouldCancel: shouldCancel)
         } catch ToolError.cancelled {
@@ -175,7 +237,7 @@ public enum ZipInspector {
         if shouldCancel() { throw CancellationError() }
         let allowed = Set(picked.map(\.name))
         var extractedSize: UInt64 = 0
-        guard let enumerator = fm.enumerator(at: stage, includingPropertiesForKeys:
+        guard let enumerator = fm.enumerator(at: contents, includingPropertiesForKeys:
             [.isRegularFileKey, .isDirectoryKey, .isSymbolicLinkKey], options: []) else {
             throw Invalid.malformed("Cannot inspect extracted files.")
         }
@@ -185,7 +247,7 @@ public enum ZipInspector {
             guard vals.isSymbolicLink != true else {
                 throw Invalid.malformed("Unexpected symbolic link after ZIP extraction.")
             }
-            let rootComponents = stage.standardizedFileURL.pathComponents
+            let rootComponents = contents.standardizedFileURL.pathComponents
             let fileComponents = file.standardizedFileURL.pathComponents
             guard fileComponents.starts(with: rootComponents) else {
                 throw Invalid.malformed("Extraction escaped the staging folder.")
@@ -209,7 +271,7 @@ public enum ZipInspector {
         let name = archive.deletingPathExtension().lastPathComponent
         let output = destinationFolder.appendingPathComponent(
             "\(name)-extracted-\(UUID().uuidString.prefix(8))", isDirectory: true)
-        try fm.moveItem(at: stage, to: output)
+        try fm.moveItem(at: contents, to: output)
         return output
     }
 }

Tests/ZipInspectorTests.swift

@@ -28,6 +28,17 @@ struct ZipInspectorTests {
         let exported2 = try ZipInspector.extract(archive: archive, destinationFolder: tmp,
                                                 selected: nil)
         assert(exported != exported2 && fm.fileExists(atPath: exported2.appendingPathComponent("first file.txt").path))
+        // A path that names a symlink must still produce a private regular
+        // snapshot. FileManager.copyItem can preserve the symlink and let
+        // unzip reread its mutable target instead.
+        let linkedArchive = tmp.appendingPathComponent("linked.zip")
+        try fm.createSymbolicLink(at: linkedArchive, withDestinationURL: archive)
+        let viaLink = try ZipInspector.extract(archive: linkedArchive,
+                                              destinationFolder: tmp, selected: ["second.txt"])
+        let linkedData = try String(contentsOf: viaLink.appendingPathComponent("second.txt"),
+                                    encoding: .utf8)
+        assert(linkedData == "second")
+
 
         // Change central entry name into a parent-directory traversal. No
         // extraction should begin even if an external tool would normalize it.
@@ -49,6 +60,33 @@ struct ZipInspectorTests {
             catch { rejected = true }
             assert(rejected, "Traversal must be refused before extraction.")
         }
+        // A benign central name with a different local-header filename is
+        // unsafe: the extractor may interpret local headers during extraction.
+        // Reject it even though the central directory alone looks valid.
+        let validArchive = [UInt8](try Data(contentsOf: archive))
+        guard let firstLocal = (0..<(validArchive.count - 4)).first(where: {
+            validArchive[$0] == 0x50 && validArchive[$0 + 1] == 0x4b &&
+            validArchive[$0 + 2] == 0x03 && validArchive[$0 + 3] == 0x04
+        }) else { fatalError("Fixture has no local ZIP header") }
+        let localNameOffset = firstLocal + 30
+        var inconsistent = validArchive
+        inconsistent[localNameOffset] = inconsistent[localNameOffset] == 0x78 ? 0x79 : 0x78
+        let mismatched = tmp.appendingPathComponent("mismatched-local.zip")
+        try Data(inconsistent).write(to: mismatched)
+        var refusedLocalMismatch = false
+        do { _ = try ZipInspector.inspect(mismatched) }
+        catch { refusedLocalMismatch = true }
+        assert(refusedLocalMismatch, "Local and central ZIP filenames must agree.")
+
+        // Requesting an entry that isn't in the validated snapshot must
+        // fail rather than silently extracting a different subset.
+        var refusedSelection = false
+        do {
+            _ = try ZipInspector.extract(archive: archive, destinationFolder: tmp,
+                                         selected: ["not-in-this-archive.txt"])
+        } catch { refusedSelection = true }
+        assert(refusedSelection, "Missing ZIP entries must be refused.")
+
         // unzip interprets entry selections as argv options or filename globs.
         // Refuse these at inspection instead of broadening a selective extract.
         let original = [UInt8](try Data(contentsOf: archive))