Skip to content

Fix use of deallocated file during nested file write #109

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.

Sign up for GitHub

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

Merged
abbeycode merged 3 commits into from
Mar 16, 2021
Merged

Fix use of deallocated file during nested file write #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f97a52
Fixed bug that could cause a deallocated pointer to be used (Issue #107)
abbeycode Mar 14, 2021
2b49140
Updated changelog
abbeycode Mar 14, 2021
43d1525
Fixed solution to #107 by limiting its scope
abbeycode Mar 14, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
21 changes: 13 additions & 8 deletions Source/UZKArchive.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -1817,6 +1819,7 @@ - (BOOL)performActionWithArchiveOpen:(void(^)(NSError * __autoreleasing*innerErr
*error = openError;
}

shouldCloseFile = openError.code != UZKErrorCodeFileWrite;
return NO;
}

Expand All @@ -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;
}
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/ModesTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down