Judge zip entry names, not standardized paths

Sources/yap/ZipArchive.swift

@@ -98,15 +98,28 @@ enum ZipArchive {
     }
 
     /// Entry names are attacker-controlled the moment a feed is: an archive
-    /// saying `../../bin/launchd` must not write there. Resolving and then
-    /// checking containment covers `..` and absolute names together.
+    /// saying `../../bin/launchd` must not write there.
+    ///
+    /// The name is judged, not the joined path. The previous version compared
+    /// two `standardizedFileURL` paths, which is a security check that depends
+    /// on the filesystem and on Foundation's willingness to resolve the
+    /// `/tmp` → `/private/tmp` class of symlink — and it does so
+    /// asymmetrically: hand it a *relative* destination and an ordinary
+    /// `yap.app/` entry is rejected as an escape. The updater only ever passes
+    /// an absolute path, so nothing shipped was broken; a rule this important
+    /// should not rest on that being true forever.
+    ///
+    /// Judging the components instead is symmetric, touches no filesystem, and
+    /// says exactly what is meant: every written path is the destination plus
+    /// a sequence of plain names. With symlink entries refused as well, that
+    /// is the whole containment argument — nothing inside the destination can
+    /// point outside it.
     private static func resolve(_ name: String, under destination: URL) throws -> URL {
-        let root = destination.standardizedFileURL.path
-        let target = destination.appendingPathComponent(name).standardizedFileURL
-        guard target.path == root || target.path.hasPrefix(root + "/") else {
-            throw ZipError.escapingPath(name)
-        }
-        return target
+        let components = name.split(separator: "/", omittingEmptySubsequences: true).map(String.init)
+        guard !name.hasPrefix("/"), !components.isEmpty,
+            !components.contains(".."), !components.contains(".")
+        else { throw ZipError.escapingPath(name) }
+        return components.reduce(destination) { $0.appendingPathComponent($1) }
     }
 
     private static func centralDirectory(in data: Data) throws -> [Entry] {

Tests/yapTests/ZipArchiveTests.swift

@@ -30,6 +30,30 @@ final class ZipArchiveTests: XCTestCase {
         return destination
     }
 
+    /// The containment rule must not depend on how the caller spelled the
+    /// destination. The first version compared two standardized paths, which
+    /// resolves symlinks off the filesystem and does it asymmetrically: with a
+    /// relative destination it rejected an ordinary `yap.app/` entry. The
+    /// updater always passes an absolute path, so nothing shipped was broken —
+    /// this pins it so nothing has to keep being true by luck.
+    func testDestinationSpellingDoesNotAffectExtraction() throws {
+        let previous = FileManager.default.currentDirectoryPath
+        defer { FileManager.default.changeCurrentDirectoryPath(previous) }
+        XCTAssertTrue(FileManager.default.changeCurrentDirectoryPath(directory.path))
+
+        let archive = directory.appendingPathComponent("fixture.zip")
+        try XCTUnwrap(Data(base64Encoded: Self.fixture, options: .ignoreUnknownCharacters))
+            .write(to: archive)
+
+        // Relative, resolved against the working directory set above.
+        try ZipArchive.extract(URL(fileURLWithPath: "fixture.zip"), to: URL(fileURLWithPath: "out"))
+        XCTAssertEqual(
+            try String(
+                contentsOf: directory.appendingPathComponent("out/bundle/Info.plist"),
+                encoding: .utf8),
+            "<plist/>")
+    }
+
     /// Deflated, and 0755: a Mach-O written 0644 is a bundle macOS refuses to
     /// launch, and nothing else in the update path would notice.
     func testDeflatedEntryKeepsItsContentsAndExecutableBit() throws {