From 40f5fa8ea24fb69498f38588cb6ac36f8c0e6b7a Mon Sep 17 00:00:00 2001 From: Ethan Arbuckle Date: Thu, 31 May 2018 18:00:40 -0700 Subject: [PATCH] Standardize the filename of Archive Entries so path traversal characters cannot be used to manipulate file output location --- ZipZap.xcodeproj/project.pbxproj | 10 +++++++++ ZipZap/ZZOldArchiveEntry.mm | 11 +++++++--- ZipZapTests/ZZPathTraversalUnzipTests.h | 12 +++++++++++ ZipZapTests/ZZPathTraversalUnzipTests.m | 27 ++++++++++++++++++++++++ ZipZapTests/assets/path-traversal.zip | Bin 0 -> 191 bytes 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 ZipZapTests/ZZPathTraversalUnzipTests.h create mode 100644 ZipZapTests/ZZPathTraversalUnzipTests.m create mode 100644 ZipZapTests/assets/path-traversal.zip diff --git a/ZipZap.xcodeproj/project.pbxproj b/ZipZap.xcodeproj/project.pbxproj index e3a1b164..515a8cc3 100644 --- a/ZipZap.xcodeproj/project.pbxproj +++ b/ZipZap.xcodeproj/project.pbxproj @@ -13,6 +13,8 @@ 5BC58CA618745940002FAE04 /* large-test-encrypted-standard.zip in Resources */ = {isa = PBXBuildFile; fileRef = 5BC58CA418745940002FAE04 /* large-test-encrypted-standard.zip */; }; 5BC58CA718745940002FAE04 /* small-test-encrypted-standard.zip in Resources */ = {isa = PBXBuildFile; fileRef = 5BC58CA518745940002FAE04 /* small-test-encrypted-standard.zip */; }; 5BC58CAC18745D22002FAE04 /* ZZStandardCryptoEngine.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5B50392E187069E4002B2B12 /* ZZStandardCryptoEngine.cpp */; }; + 5F2CE0DE20C0B2D900B558EC /* path-traversal.zip in Resources */ = {isa = PBXBuildFile; fileRef = 5F2CE0DD20C0B2D900B558EC /* path-traversal.zip */; }; + 5F2CE0E120C0B3F900B558EC /* ZZPathTraversalUnzipTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 5F2CE0E020C0B3F900B558EC /* ZZPathTraversalUnzipTests.m */; }; BF8624AD1BB7495200C12EEE /* ZZAESDecryptInputStream.mm in Sources */ = {isa = PBXBuildFile; fileRef = D8ABBF3D18883528002858BE /* ZZAESDecryptInputStream.mm */; }; BF8624AE1BB7495200C12EEE /* ZZArchive.mm in Sources */ = {isa = PBXBuildFile; fileRef = D899CFDB162C608300112F49 /* ZZArchive.mm */; }; BF8624AF1BB7495200C12EEE /* ZZArchiveEntry.m in Sources */ = {isa = PBXBuildFile; fileRef = D899CFD9162C608300112F49 /* ZZArchiveEntry.m */; }; @@ -242,6 +244,9 @@ 5B50393218707C36002B2B12 /* ZZStandardDecryptInputStream.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ZZStandardDecryptInputStream.h; sourceTree = ""; }; 5BC58CA418745940002FAE04 /* large-test-encrypted-standard.zip */ = {isa = PBXFileReference; lastKnownFileType = archive.zip; path = "large-test-encrypted-standard.zip"; sourceTree = ""; }; 5BC58CA518745940002FAE04 /* small-test-encrypted-standard.zip */ = {isa = PBXFileReference; lastKnownFileType = archive.zip; path = "small-test-encrypted-standard.zip"; sourceTree = ""; }; + 5F2CE0DD20C0B2D900B558EC /* path-traversal.zip */ = {isa = PBXFileReference; lastKnownFileType = archive.zip; path = "path-traversal.zip"; sourceTree = ""; }; + 5F2CE0DF20C0B3F900B558EC /* ZZPathTraversalUnzipTests.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ZZPathTraversalUnzipTests.h; sourceTree = ""; }; + 5F2CE0E020C0B3F900B558EC /* ZZPathTraversalUnzipTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ZZPathTraversalUnzipTests.m; sourceTree = ""; }; BF8624CB1BB7495200C12EEE /* ZipZap.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = ZipZap.framework; sourceTree = BUILT_PRODUCTS_DIR; }; BF8624EF1BB74B1F00C12EEE /* libZipZap.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libZipZap.a; sourceTree = BUILT_PRODUCTS_DIR; }; D804E4C8187645FE00289404 /* ZZDataProvider.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ZZDataProvider.h; sourceTree = ""; }; @@ -447,6 +452,7 @@ 5BC58CA318745940002FAE04 /* assets */ = { isa = PBXGroup; children = ( + 5F2CE0DD20C0B2D900B558EC /* path-traversal.zip */, D87BA1AA1B3D5B2900ED7EB3 /* dog.jpg */, D87BA1AB1B3D5B2900ED7EB3 /* dog.png */, D8E366EA188832C0009F3008 /* large-test-encrypted-aes128.zip */, @@ -543,6 +549,8 @@ D8B83A7E19B7D3A700CF72B6 /* ZZZipOldTests.h */, D8B83A7C19B7D39200CF72B6 /* ZZZipOldTests.m */, D899D01F162F6C9700112F49 /* ZZZipTests.h */, + 5F2CE0DF20C0B3F900B558EC /* ZZPathTraversalUnzipTests.h */, + 5F2CE0E020C0B3F900B558EC /* ZZPathTraversalUnzipTests.m */, D899D020162F6C9700112F49 /* ZZZipTests.m */, D899CF9E162C5EC100112F49 /* Supporting Files */, ); @@ -813,6 +821,7 @@ buildActionMask = 2147483647; files = ( D8E366F5188832C0009F3008 /* small-test-encrypted-aes256.zip in Resources */, + 5F2CE0DE20C0B2D900B558EC /* path-traversal.zip in Resources */, D899CFA2162C5EC100112F49 /* InfoPlist.strings in Resources */, D8E366F0188832C0009F3008 /* large-test-encrypted-aes128.zip in Resources */, D8E366F2188832C0009F3008 /* large-test-encrypted-aes256.zip in Resources */, @@ -952,6 +961,7 @@ D899D01A162CCF9700112F49 /* ZZUnzipTests.m in Sources */, D8B83A7719B7CBB400CF72B6 /* ZZZipNewTests.m in Sources */, D8B83A7D19B7D39200CF72B6 /* ZZZipOldTests.m in Sources */, + 5F2CE0E120C0B3F900B558EC /* ZZPathTraversalUnzipTests.m in Sources */, D899D021162F6C9700112F49 /* ZZZipTests.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/ZipZap/ZZOldArchiveEntry.mm b/ZipZap/ZZOldArchiveEntry.mm index 18ae2ff6..0a0978d4 100644 --- a/ZipZap/ZZOldArchiveEntry.mm +++ b/ZipZap/ZZOldArchiveEntry.mm @@ -250,9 +250,14 @@ - (BOOL)check:(out NSError**)error - (NSString*)fileNameWithEncoding:(NSStringEncoding)encoding { - return [[NSString alloc] initWithBytes:_centralFileHeader->fileName() - length:_centralFileHeader->fileNameLength - encoding:encoding]; + // Sanitize path traversal characters if they're present in the file name to prevent directory backtracking. Ignoring these characters mimicks the default behavior of the Unarchiving tool on macOS. + NSString *rawFilePath = [[NSString alloc] initWithBytes:_centralFileHeader->fileName() length:_centralFileHeader->fileNameLength encoding:encoding]; + + // Add URL percent encoding to the path as it may contain non-english accented characters that would break NSURL + rawFilePath = [rawFilePath stringByAddingPercentEncodingWithAllowedCharacters:[NSCharacterSet URLQueryAllowedCharacterSet]]; + + // Standardize the URL (remove any path oddities) before removing the percent encoding + return [[[[NSURL URLWithString:rawFilePath] standardizedURL] absoluteString] stringByRemovingPercentEncoding]; } - (BOOL)checkEncryptionAndCompression:(out NSError**)error diff --git a/ZipZapTests/ZZPathTraversalUnzipTests.h b/ZipZapTests/ZZPathTraversalUnzipTests.h new file mode 100644 index 00000000..40aa83cc --- /dev/null +++ b/ZipZapTests/ZZPathTraversalUnzipTests.h @@ -0,0 +1,12 @@ +// +// ZZPathTraversalUnzipTests.h +// ZipZapTests +// +// Created by Ethan Arbuckle on 5/31/18. +// + +#import + +@interface ZZPathTraversalUnzipTests : XCTestCase + +@end diff --git a/ZipZapTests/ZZPathTraversalUnzipTests.m b/ZipZapTests/ZZPathTraversalUnzipTests.m new file mode 100644 index 00000000..97913051 --- /dev/null +++ b/ZipZapTests/ZZPathTraversalUnzipTests.m @@ -0,0 +1,27 @@ +// +// ZZPathTraversalUnzipTests.m +// ZipZapTests +// +// Created by Ethan Arbuckle on 5/31/18. +// + +#import "ZZPathTraversalUnzipTests.h" +#import + +@implementation ZZPathTraversalUnzipTests + +- (void)testExtractingZipContainingPathTraversalEntries +{ + // This zip archive contains a file titled '../../../../../../../../../../..//tmp/test.txt'. ZipZap should ignore the path traversing and write the file to "tmp/test.txt" + ZZArchive* zipFile = [ZZArchive archiveWithURL:[[NSBundle bundleForClass:self.class] URLForResource:@"path-traversal" withExtension:@"zip"] error:nil]; + + for (NSUInteger index = 0, count = zipFile.entries.count; index < count; ++index) + { + ZZArchiveEntry* nextEntry = zipFile.entries[index]; + + // Assert that the entry does not begin with '..' + XCTAssertFalse([[[nextEntry fileName] substringToIndex:2] isEqualToString:@".."]); + } +} + +@end diff --git a/ZipZapTests/assets/path-traversal.zip b/ZipZapTests/assets/path-traversal.zip new file mode 100644 index 0000000000000000000000000000000000000000..af0027f56e2e98e718d9c23bb6b368bce1730461 GIT binary patch literal 191 zcmWIWW@Zs#0D;5Rn|zjC4ya-VvO!oEi1qaJ@uQO50{xQI;u5`*ijt_zf&gzuCJ|=b i<^hcagJq2%3ct}nS!B}#yjj^mnizpF8c6$rI1B(mD