Skip to content

Commit

Permalink
Standardize the filename of Archive Entries so path traversal charact…
Browse files Browse the repository at this point in the history
…ers cannot be used to manipulate file output location
  • Loading branch information
EthanArbuckle committed Jun 1, 2018
1 parent 88cf773 commit 40f5fa8
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 3 deletions.
10 changes: 10 additions & 0 deletions ZipZap.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -242,6 +244,9 @@
5B50393218707C36002B2B12 /* ZZStandardDecryptInputStream.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ZZStandardDecryptInputStream.h; sourceTree = "<group>"; };
5BC58CA418745940002FAE04 /* large-test-encrypted-standard.zip */ = {isa = PBXFileReference; lastKnownFileType = archive.zip; path = "large-test-encrypted-standard.zip"; sourceTree = "<group>"; };
5BC58CA518745940002FAE04 /* small-test-encrypted-standard.zip */ = {isa = PBXFileReference; lastKnownFileType = archive.zip; path = "small-test-encrypted-standard.zip"; sourceTree = "<group>"; };
5F2CE0DD20C0B2D900B558EC /* path-traversal.zip */ = {isa = PBXFileReference; lastKnownFileType = archive.zip; path = "path-traversal.zip"; sourceTree = "<group>"; };
5F2CE0DF20C0B3F900B558EC /* ZZPathTraversalUnzipTests.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ZZPathTraversalUnzipTests.h; sourceTree = "<group>"; };
5F2CE0E020C0B3F900B558EC /* ZZPathTraversalUnzipTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ZZPathTraversalUnzipTests.m; sourceTree = "<group>"; };
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 = "<group>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
);
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 8 additions & 3 deletions ZipZap/ZZOldArchiveEntry.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions ZipZapTests/ZZPathTraversalUnzipTests.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//
// ZZPathTraversalUnzipTests.h
// ZipZapTests
//
// Created by Ethan Arbuckle on 5/31/18.
//

#import <XCTest/XCTest.h>

@interface ZZPathTraversalUnzipTests : XCTestCase

@end
27 changes: 27 additions & 0 deletions ZipZapTests/ZZPathTraversalUnzipTests.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//
// ZZPathTraversalUnzipTests.m
// ZipZapTests
//
// Created by Ethan Arbuckle on 5/31/18.
//

#import "ZZPathTraversalUnzipTests.h"
#import <ZipZap/ZipZap.h>

@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
Binary file added ZipZapTests/assets/path-traversal.zip
Binary file not shown.

0 comments on commit 40f5fa8

Please sign in to comment.