From 2f97a52cf128007585fb745c51259205bd46b768 Mon Sep 17 00:00:00 2001 From: Dov Frankel Date: Sat, 13 Mar 2021 23:12:59 -0500 Subject: [PATCH 1/3] Fixed bug that could cause a deallocated pointer to be used (Issue #107) --- Source/UZKArchive.m | 21 +++++++++++++-------- Tests/ModesTests.m | 4 ++-- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Source/UZKArchive.m b/Source/UZKArchive.m index dd1a4ae..28c7ceb 100644 --- a/Source/UZKArchive.m +++ b/Source/UZKArchive.m @@ -1805,6 +1805,8 @@ - (BOOL)performActionWithArchiveOpen:(void(^)(NSError * __autoreleasing*innerErr NSError *openError = nil; NSError *actionError = nil; + BOOL shouldCloseFile = YES; + @try { if (![self openFile:self.filename inMode:mode @@ -1817,6 +1819,7 @@ - (BOOL)performActionWithArchiveOpen:(void(^)(NSError * __autoreleasing*innerErr *error = openError; } + shouldCloseFile = NO; return NO; } @@ -1826,15 +1829,17 @@ - (BOOL)performActionWithArchiveOpen:(void(^)(NSError * __autoreleasing*innerErr } } @finally { - NSError *closeError = nil; - if (![self closeFile:&closeError inMode:mode]) { - UZKLogDebug("Archive failed to close"); - - if (error && !actionError && !openError) { - *error = closeError; + if (shouldCloseFile) { + NSError *closeError = nil; + if (![self closeFile:&closeError inMode:mode]) { + UZKLogDebug("Archive failed to close"); + + if (error && !actionError && !openError) { + *error = closeError; + } + + return NO; } - - return NO; } } diff --git a/Tests/ModesTests.m b/Tests/ModesTests.m index 06287c1..098f35f 100644 --- a/Tests/ModesTests.m +++ b/Tests/ModesTests.m @@ -93,12 +93,12 @@ - (void)testModes_NestedWrites UZKArchive *archive = [[UZKArchive alloc] initWithURL:testArchiveURL error:nil]; NSError *outerWriteError = nil; - [archive writeIntoBufferAtPath:@"newFile.zip" + [archive writeIntoBufferAtPath:@"outer file.txt" error:&outerWriteError block: ^BOOL(BOOL(^writeData)(const void *bytes, unsigned int length), NSError**(actionError)) { NSError *innerWriteError = nil; - [archive writeIntoBufferAtPath:@"newFile.zip" + [archive writeIntoBufferAtPath:@"inner file.txt" error:&innerWriteError block:^BOOL(BOOL(^writeData)(const void *bytes, unsigned int length), NSError**(actionError)) {return YES;}]; XCTAssertNotNil(innerWriteError, @"Nested write operation succeeded"); From 2b491405c11aa266ddcede7426c97c6639aa88c6 Mon Sep 17 00:00:00 2001 From: Dov Frankel Date: Sat, 13 Mar 2021 23:14:55 -0500 Subject: [PATCH 2/3] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a1d139..2298231 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * Fixed a crasher for unreadable files in `+pathIsAZip:` (Issue #99) * Deprecated all overloads of `-writeData:...` and `-writeIntoBuffer:...` that take any file properties other than the path, replacing them each with a single call that takes an instance of the new `ZipFileProperties`. This allows for all the default values to be defined in one place, so you can specify only where you want to deviate from the defaults (Issue #89, PR #97) * Fixed buffer overrun vulnerability when deleting a file in an archive where not every file has a file comment (Issue #106) +* Fixed deallocated pointer use when a file write occurs inside the block of a file write operation, already an error condition (Issue #107) ## 1.9 From 43d15250b82f2169f55572a722f862958bd94c7a Mon Sep 17 00:00:00 2001 From: Dov Frankel Date: Sun, 14 Mar 2021 00:13:48 -0500 Subject: [PATCH 3/3] Fixed solution to #107 by limiting its scope --- Source/UZKArchive.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/UZKArchive.m b/Source/UZKArchive.m index 28c7ceb..9821c08 100644 --- a/Source/UZKArchive.m +++ b/Source/UZKArchive.m @@ -1819,7 +1819,7 @@ - (BOOL)performActionWithArchiveOpen:(void(^)(NSError * __autoreleasing*innerErr *error = openError; } - shouldCloseFile = NO; + shouldCloseFile = openError.code != UZKErrorCodeFileWrite; return NO; }