Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Path Escape Vulerability #304

Merged
merged 7 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion Sources/ZIPFoundation/FileManager+ZIP.swift
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,25 @@ extension CocoaError {
}

public extension URL {

func isContained(in parentDirectoryURL: URL) -> Bool {
// Ensure this URL is contained in the passed in URL
let parentDirectoryURL = URL(fileURLWithPath: parentDirectoryURL.path, isDirectory: true).standardized
return self.standardized.absoluteString.hasPrefix(parentDirectoryURL.absoluteString)
// Maliciously crafted ZIP files can contain entries using a prepended path delimiter `/` in combination
// with the parent directory shorthand `..` to bypass our containment check.
// When a malicious entry path like e.g. `/../secret.txt` gets appended to the destination
// directory URL (e.g. `file:///tmp/`), the resulting URL `file:///tmp//../secret.txt` gets expanded
// to `file:///tmp/secret` when using `URL.standardized`. This URL would pass the check performed
// in `isContained(in:)`.
// Lower level API like POSIX `fopen` - which is used at a later point during extraction - expands
// `/tmp//../secret.txt` to `/secret.txt` though. This would lead to an escape to the parent directory.
// To avoid that, we replicate the behavior of `fopen`s path expansion and replace all double delimiters
// with single delimiters.
// More details: https://github.com/weichsel/ZIPFoundation/issues/281
let sanitizedEntryPathURL: URL = {
let sanitizedPath = self.path.replacingOccurrences(of: "//", with: "/")
return URL(fileURLWithPath: sanitizedPath)
}()
return sanitizedEntryPathURL.standardized.absoluteString.hasPrefix(parentDirectoryURL.absoluteString)
}
}
Binary file not shown.
10 changes: 9 additions & 1 deletion Tests/ZIPFoundationTests/ZIPFoundationReadingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,15 @@ extension ZIPFoundationTests {
throws: Archive.ArchiveError.invalidCRC32)
}

func testTraversalAttack() {
func testSimpleTraversalAttack() {
let fileManager = FileManager()
let archive = self.archive(for: #function, mode: .read)
let destinationURL = self.createDirectory(for: #function)
XCTAssertCocoaError(try fileManager.unzipItem(at: archive.url, to: destinationURL),
throwsErrorWithCode: .fileReadInvalidFileName)
}

func testPathDelimiterTraversalAttack() {
let fileManager = FileManager()
let archive = self.archive(for: #function, mode: .read)
let destinationURL = self.createDirectory(for: #function)
Expand Down
11 changes: 6 additions & 5 deletions Tests/ZIPFoundationTests/ZIPFoundationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,16 @@ class ZIPFoundationTests: XCTestCase {

func createDirectory(for testFunction: String) -> URL {
let fileManager = FileManager()
var URL = ZIPFoundationTests.tempZipDirectoryURL
URL = URL.appendingPathComponent(self.pathComponent(for: testFunction))
var url = ZIPFoundationTests.tempZipDirectoryURL
url = url.appendingPathComponent(self.pathComponent(for: testFunction), isDirectory: true)
do {
try fileManager.createDirectory(at: URL, withIntermediateDirectories: true, attributes: nil)
try fileManager.createDirectory(at: url, withIntermediateDirectories: true, attributes: nil)
} catch {
XCTFail("Failed to get create directory for test function:\(testFunction)")
type(of: self).tearDown()
preconditionFailure()
}
return URL
return url
}

func runWithUnprivilegedGroup(handler: () throws -> Void) {
Expand Down Expand Up @@ -253,7 +253,8 @@ extension ZIPFoundationTests {
("testRemoveEntryErrorConditions", testRemoveEntryErrorConditions),
("testRemoveUncompressedEntry", testRemoveUncompressedEntry),
("testTemporaryReplacementDirectoryURL", testTemporaryReplacementDirectoryURL),
("testTraversalAttack", testTraversalAttack),
("testSimpleTraversalAttack", testSimpleTraversalAttack),
("testPathDelimiterTraversalAttack", testPathDelimiterTraversalAttack),
("testUnzipItem", testUnzipItem),
("testUnzipItemWithPreferredEncoding", testUnzipItemWithPreferredEncoding),
("testUnzipItemErrorConditions", testUnzipItemErrorConditions),
Expand Down
Loading