Guard against implicitly unwrapped file handle crashes

weichsel · Nov 20, 2018 · 17d3a41 · 17d3a41
1 parent 210f251
commit 17d3a41
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 9 deletions.
diff --git a/Sources/ZIPFoundation/Archive+Reading.swift b/Sources/ZIPFoundation/Archive+Reading.swift
@@ -30,8 +30,10 @@ extension Archive {
                 throw CocoaError.error(.fileWriteFileExists, userInfo: [NSFilePathErrorKey: url.path], url: nil)
             }
             try fileManager.createParentDirectoryStructure(for: url)
-            let destinationFileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: url.path)
-            let destinationFile: UnsafeMutablePointer<FILE> = fopen(destinationFileSystemRepresentation, "wb+")
+            let destinationRepresentation = fileManager.fileSystemRepresentation(withPath: url.path)
+            guard let destinationFile: UnsafeMutablePointer<FILE> = fopen(destinationRepresentation, "wb+") else {
+                throw CocoaError.error(.fileNoSuchFile)
+            }
             defer { fclose(destinationFile) }
             let consumer = { _ = try Data.write(chunk: $0, to: destinationFile) }
             checksum = try self.extract(entry, bufferSize: bufferSize, progress: progress, consumer: consumer)

diff --git a/Sources/ZIPFoundation/Archive+Writing.swift b/Sources/ZIPFoundation/Archive+Writing.swift
@@ -44,7 +44,9 @@ extension Archive {
         switch type {
         case .file:
             let entryFileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: entryURL.path)
-            let entryFile: UnsafeMutablePointer<FILE> = fopen(entryFileSystemRepresentation, "rb")
+            guard let entryFile: UnsafeMutablePointer<FILE> = fopen(entryFileSystemRepresentation, "rb") else {
+                throw CocoaError.error(.fileNoSuchFile)
+            }
             defer { fclose(entryFile) }
             provider = { _, _ in return try Data.readChunk(of: Int(bufferSize), from: entryFile) }
             try self.addEntry(with: path, type: type, uncompressedSize: uncompressedSize,
@@ -60,7 +62,6 @@ extension Archive {
                               progress: progress, provider: provider)
         case .symlink:
             provider = { _, _ -> Data in
-                let fileManager = FileManager()
                 let linkDestination = try fileManager.destinationOfSymbolicLink(atPath: entryURL.path)
                 let linkFileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: linkDestination)
                 let linkLength = Int(strlen(linkFileSystemRepresentation))

diff --git a/Sources/ZIPFoundation/Archive.swift b/Sources/ZIPFoundation/Archive.swift
@@ -141,10 +141,11 @@ public final class Archive: Sequence {
             guard fileManager.fileExists(atPath: url.path) else { return nil }
             guard fileManager.isReadableFile(atPath: url.path) else { return nil }
             let fileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: url.path)
-            self.archiveFile = fopen(fileSystemRepresentation, "rb")
-            guard let endOfCentralDirectoryRecord = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {
+            guard let archiveFile = fopen(fileSystemRepresentation, "rb"),
+                let endOfCentralDirectoryRecord = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {
                 return nil
             }
+            self.archiveFile = archiveFile
             self.endOfCentralDirectoryRecord = endOfCentralDirectoryRecord
         case .create:
             guard !fileManager.fileExists(atPath: url.path) else { return nil }
@@ -161,10 +162,11 @@ public final class Archive: Sequence {
         case .update:
             guard fileManager.isWritableFile(atPath: url.path) else { return nil }
             let fileSystemRepresentation = fileManager.fileSystemRepresentation(withPath: url.path)
-            self.archiveFile = fopen(fileSystemRepresentation, "rb+")
-            guard let endOfCentralDirectoryRecord = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {
+            guard let archiveFile = fopen(fileSystemRepresentation, "rb+"),
+                let endOfCentralDirectoryRecord = Archive.scanForEndOfCentralDirectoryRecord(in: archiveFile) else {
                 return nil
             }
+            self.archiveFile = archiveFile
             self.endOfCentralDirectoryRecord = endOfCentralDirectoryRecord
             fseek(self.archiveFile, 0, SEEK_SET)
         }
@@ -188,7 +190,7 @@ public final class Archive: Sequence {
             let offset = Int(centralDirStruct.relativeOffsetOfLocalHeader)
             guard let localFileHeader: LocalFileHeader = Data.readStruct(from: self.archiveFile,
                                                                          at: offset) else { return nil }
-            var dataDescriptor: DataDescriptor? = nil
+            var dataDescriptor: DataDescriptor?
             if centralDirStruct.usesDataDescriptor {
                 let additionalSize = Int(localFileHeader.fileNameLength + localFileHeader.extraFieldLength)
                 let isCompressed = centralDirStruct.compressionMethod != CompressionMethod.none.rawValue