diff --git a/README.md b/README.md index 7f42dc2..aa1f817 100644 --- a/README.md +++ b/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. diff --git a/Sources/App/ZIP/ZipInspector.swift b/Sources/App/ZIP/ZipInspector.swift index b5919ef..83f1f95 100644 --- a/Sources/App/ZIP/ZipInspector.swift +++ b/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,15 +171,39 @@ 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?, 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( @@ -163,10 +211,24 @@ public enum ZipInspector { 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 } } diff --git a/Tests/ZipInspectorTests.swift b/Tests/ZipInspectorTests.swift index 367f3d2..a9d6e95 100644 --- a/Tests/ZipInspectorTests.swift +++ b/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))