Implemented a way for users to opt into ignoring mismatched CRCs in a…

…n archive header (Issue abbeycode#82), and consistently replaced all references to 'self' inside blocks with 'welf'
xiaogdgenuine · Nov 3, 2020 · 79a32cc · 79a32cc
1 parent 85a79a8
commit 79a32cc
Show file tree

Hide file tree

Showing 15 changed files with 978 additions and 101 deletions.
diff --git a/Classes/URKArchive.h b/Classes/URKArchive.h
@@ -477,11 +477,30 @@ extern NSString *URKErrorDomain;
 /**
  Extract each file in the archive, checking whether the data matches the CRC checksum
  stored at the time it was written
-
+ 
  @return YES if the data is all correct, false if any check failed
  */
 - (BOOL)checkDataIntegrity;
 
+/**
+ Extract each file in the archive, checking whether the data matches the CRC checksum
+ stored at the time it was written. If any file doesn't match, run the given block
+ to allow the API consumer to decide whether to ignore mismatches. NOTE: This may be a
+ security risk. The block is intended to prompt the user, which is why it's forced onto
+ the main thread, rather than making a design-time decision
+ 
+ @param ignoreCRCMismatches This block, called on the main thread, allows a consuming API to
+                            prompt the user whether or not he'd like to ignore CRC mismatches.
+                            This block is called the first time a CRC mismatch is detected, if
+                            at all. It won't be called if all CRCs match. If this returns YES,
+                            then all further CRC mismatches will be ignored for the
+                            archive instance
+ 
+ @return YES if the data is all correct and/or the block returns YES; returns false if
+         any check failed and the given block also returns NO
+ */
+- (BOOL)checkDataIntegrityIgnoringCRCMismatches:(BOOL(^)(void))ignoreCRCMismatches;
+
 /**
  Extract a particular file, to determine if its data matches the CRC
  checksum stored at the time it written

diff --git a/Classes/URKArchive.mm b/Classes/URKArchive.mm
diff --git a/Classes/URKFileInfo.m b/Classes/URKFileInfo.m
@@ -40,7 +40,7 @@ - (instancetype)initWithFileHeader:(struct RARHeaderDataEx *)fileHeader
         _isEncryptedWithPassword = fileHeader->Flags & (1 << 2);
         //_fileHasComment = fileHeader->Flags  & (1 << 3)
 
-        _isDirectory = fileHeader->Flags & RHDF_DIRECTORY;
+        _isDirectory = (fileHeader->Flags & RHDF_DIRECTORY) ? YES : NO;
     }
 
     return self;

diff --git a/Tests/CheckDataTests.m b/Tests/CheckDataTests.m
@@ -91,4 +91,70 @@ - (void)testCheckDataIntegrityForFile_ModifiedCRC {
     XCTAssertFalse(success, @"Data integrity check passed for archive with modified CRC");
 }
 
+- (void)testCheckDataIntegrityIgnoringCRCMismatches {
+    NSArray *testArchives = @[@"Test Archive.rar",
+                              @"Test Archive (Password).rar",
+                              @"Test Archive (Header Password).rar"];
+
+    for (NSString *testArchiveName in testArchives) {
+        NSLog(@"Testing data integrity of file in archive %@", testArchiveName);
+        NSURL *testArchiveURL = self.testFileURLs[testArchiveName];
+        NSString *password = ([testArchiveName rangeOfString:@"Password"].location != NSNotFound
+                              ? @"password"
+                              : nil);
+        URKArchive *archive = [[URKArchive alloc] initWithURL:testArchiveURL password:password error:nil];
+
+        __block BOOL blockInvoked = NO;
+        BOOL success = [archive checkDataIntegrityIgnoringCRCMismatches:^BOOL{
+            blockInvoked = YES;
+            return NO;
+        }];
+
+        XCTAssertTrue(success, @"Data integrity check failed for %@", testArchiveName);
+        XCTAssertFalse(blockInvoked, @"Block prompting whether to ignore CRC mismatches should not have been called");
+    }
+}
+
+- (void)testCheckDataIntegrityIgnoringCRCMismatches_NotAnArchive {
+    NSURL *testArchiveURL = self.testFileURLs[@"Test File B.jpg"];
+    URKArchive *archive = [[URKArchive alloc] initWithURL:testArchiveURL error:nil];
+
+    __block BOOL blockInvoked = NO;
+    BOOL success = [archive checkDataIntegrityIgnoringCRCMismatches:^BOOL{
+        blockInvoked = YES;
+        return NO;
+    }];
+
+    XCTAssertFalse(success, @"Data integrity check passed for non-archive");
+    XCTAssertFalse(blockInvoked, @"Block prompting whether to ignore CRC mismatches should not have been called");
+}
+
+- (void)testCheckDataIntegrityIgnoringCRCMismatches_ModifiedCRC_Ignore {
+    NSURL *testArchiveURL = self.testFileURLs[@"Modified CRC Archive.rar"];
+    URKArchive *archive = [[URKArchive alloc] initWithURL:testArchiveURL error:nil];
+
+    __block BOOL blockInvoked = NO;
+    BOOL success = [archive checkDataIntegrityIgnoringCRCMismatches:^BOOL{
+        blockInvoked = YES;
+        return YES;
+    }];
+
+    XCTAssertTrue(success, @"Data integrity check failed for archive with modified CRC, when instructed to ignore");
+    XCTAssertTrue(blockInvoked, @"Block prompting whether to ignore CRC mismatches should have been called");
+}
+
+- (void)testCheckDataIntegrityIgnoringCRCMismatches_ModifiedCRC_DontIgnore {
+    NSURL *testArchiveURL = self.testFileURLs[@"Modified CRC Archive.rar"];
+    URKArchive *archive = [[URKArchive alloc] initWithURL:testArchiveURL error:nil];
+
+    __block BOOL blockInvoked = NO;
+    BOOL success = [archive checkDataIntegrityIgnoringCRCMismatches:^BOOL{
+        blockInvoked = YES;
+        return NO;
+    }];
+
+    XCTAssertFalse(success, @"Data integrity check passed for archive with modified CRC");
+    XCTAssertTrue(blockInvoked, @"Block prompting whether to ignore CRC mismatches should have been called");
+}
+
 @end
diff --git a/Tests/ExtractBufferedDataTests.m b/Tests/ExtractBufferedDataTests.m
@@ -52,6 +52,63 @@ - (void)testExtractBufferedData
                   @"File extracted in buffer not returned correctly");
 }
 
+- (void)testExtractBufferedData_ModifiedCRC
+{
+    NSURL *archiveURL = self.testFileURLs[@"Modified CRC Archive.rar"];
+    NSString *extractedFile = @"README.md";
+    URKArchive *archive = [[URKArchive alloc] initWithURL:archiveURL error:nil];
+
+    NSError *error = nil;
+    NSMutableData *reconstructedFile = [NSMutableData data];
+    BOOL success = [archive extractBufferedDataFromFile:extractedFile
+                                                  error:&error
+                                                 action:
+                    ^(NSData *dataChunk, CGFloat percentDecompressed) {
+                        NSLog(@"Decompressed: %f%%", percentDecompressed);
+                        [reconstructedFile appendBytes:dataChunk.bytes
+                                                length:dataChunk.length];
+                    }];
+
+    XCTAssertFalse(success, @"Failed to read buffered data");
+    XCTAssertNotNil(error, @"Error reading buffered data");
+
+    NSData *originalFile = [NSData dataWithContentsOfURL:self.testFileURLs[extractedFile]];
+    XCTAssertTrue([originalFile isEqualToData:reconstructedFile],
+                  @"File extracted in buffer not returned correctly");
+}
+
+- (void)testExtractBufferedData_ModifiedCRC_IgnoringMismatches
+{
+    NSURL *archiveURL = self.testFileURLs[@"Modified CRC Archive.rar"];
+    NSString *extractedFile = @"README.md";
+    URKArchive *archive = [[URKArchive alloc] initWithURL:archiveURL error:nil];
+
+    BOOL checkIntegritySuccess = [archive checkDataIntegrityIgnoringCRCMismatches:^BOOL{
+        return YES;
+    }];
+
+    XCTAssertTrue(checkIntegritySuccess, @"Data integrity check failed for archive with modified CRC, when instructed to ignore");
+
+    NSError *error = nil;
+    NSMutableData *reconstructedFile = [NSMutableData data];
+    BOOL success = [archive extractBufferedDataFromFile:extractedFile
+                                                  error:&error
+                                                 action:
+                    ^(NSData *dataChunk, CGFloat percentDecompressed) {
+                        NSLog(@"Decompressed: %f%%", percentDecompressed);
+                        [reconstructedFile appendBytes:dataChunk.bytes
+                                                length:dataChunk.length];
+                    }];
+
+    XCTAssertTrue(success, @"Failed to read buffered data");
+    XCTAssertNil(error, @"Error reading buffered data");
+    XCTAssertGreaterThan(reconstructedFile.length, 0, @"No data returned");
+
+    NSData *originalFile = [NSData dataWithContentsOfURL:self.testFileURLs[extractedFile]];
+    XCTAssertTrue([originalFile isEqualToData:reconstructedFile],
+                  @"File extracted in buffer not returned correctly");
+}
+
 #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200
 - (void)testExtractBufferedData_VeryLarge
 {

diff --git a/Tests/ExtractDataTests.m b/Tests/ExtractDataTests.m
@@ -19,7 +19,7 @@ - (void)testExtractData
                               @"Test Archive (Header Password).rar"];
 
     NSSet *expectedFileSet = [self.testFileURLs keysOfEntriesPassingTest:^BOOL(NSString *key, id obj, BOOL *stop) {
-        return ![key hasSuffix:@"rar"];
+        return ![key hasSuffix:@"rar"] && ![key hasSuffix:@"md"];
     }];
 
     NSArray *expectedFiles = [[expectedFileSet allObjects] sortedArrayUsingSelector:@selector(compare:)];
@@ -60,7 +60,7 @@ - (void)testExtractData
 - (void)testExtractData_Unicode
 {
     NSSet *expectedFileSet = [self.unicodeFileURLs keysOfEntriesPassingTest:^BOOL(NSString *key, id obj, BOOL *stop) {
-        return ![key hasSuffix:@"rar"];
+        return ![key hasSuffix:@"rar"] && ![key hasSuffix:@"md"];
     }];
 
     NSArray *expectedFiles = [[expectedFileSet allObjects] sortedArrayUsingSelector:@selector(compare:)];
@@ -121,4 +121,35 @@ - (void)testExtractData_InvalidArchive
     XCTAssertEqual(error.code, URKErrorCodeBadArchive, @"Unexpected error code returned");
 }
 
+- (void)testExtractData_ModifiedCRC
+{
+    URKArchive *archive = [[URKArchive alloc] initWithURL:self.testFileURLs[@"Modified CRC Archive.rar"] error:nil];
+
+    NSError *error = nil;
+    NSData *data = [archive extractDataFromFile:@"README.md" error:&error];
+
+    XCTAssertNotNil(error, @"Extract data for invalid archive succeeded");
+    XCTAssertNil(data, @"Data returned for invalid archive");
+    XCTAssertEqual(error.code, URKErrorCodeBadData, @"Unexpected error code returned");
+}
+
+- (void)testExtractData_ModifiedCRC_IgnoringMismatches
+{
+    URKArchive *archive = [[URKArchive alloc] initWithURL:self.testFileURLs[@"Modified CRC Archive.rar"] error:nil];
+
+    BOOL checkIntegritySuccess = [archive checkDataIntegrityIgnoringCRCMismatches:^BOOL{
+        return YES;
+    }];
+
+    XCTAssertTrue(checkIntegritySuccess, @"Data integrity check failed for archive with modified CRC, when instructed to ignore");
+
+    NSError *error = nil;
+    NSData *data = [archive extractDataFromFile:@"README.md" error:&error];
+    NSData *expectedData = [NSData dataWithContentsOfURL:self.testFileURLs[@"README.md"]];
+
+    XCTAssertNil(error, @"Extract data for invalid archive succeeded");
+    XCTAssertNotNil(data, @"Data returned for invalid archive");
+    XCTAssertEqualObjects(data, expectedData);
+}
+
 @end
diff --git a/Tests/ExtractFilesTests.m b/Tests/ExtractFilesTests.m
@@ -21,7 +21,7 @@ - (void)testExtractFiles
                               @"Test Archive (Header Password).rar"];
 
     NSSet *expectedFileSet = [self.testFileURLs keysOfEntriesPassingTest:^BOOL(NSString *key, id obj, BOOL *stop) {
-        return ![key hasSuffix:@"rar"];
+        return ![key hasSuffix:@"rar"] && ![key hasSuffix:@"md"];
     }];
 
     NSArray *expectedFiles = [[expectedFileSet allObjects] sortedArrayUsingSelector:@selector(compare:)];
@@ -142,7 +142,7 @@ - (void)testExtractFiles_RAR5
 - (void)testExtractFiles_Unicode
 {
     NSSet *expectedFileSet = [self.unicodeFileURLs keysOfEntriesPassingTest:^BOOL(NSString *key, id obj, BOOL *stop) {
-        return ![key hasSuffix:@"rar"];
+        return ![key hasSuffix:@"rar"] && ![key hasSuffix:@"md"];
     }];
 
     NSArray *expectedFiles = [[expectedFileSet allObjects] sortedArrayUsingSelector:@selector(compare:)];
@@ -240,4 +240,81 @@ - (void)testExtractFiles_InvalidArchive
     XCTAssertFalse(dirExists, @"Directory successfully created for invalid archive");
 }
 
+- (void)testExtractFiles_ModifiedCRC
+{
+    NSFileManager *fm = [NSFileManager defaultManager];
+
+    URKArchive *archive = [[URKArchive alloc] initWithURL:self.testFileURLs[@"Modified CRC Archive.rar"] error:nil];
+
+    NSString *extractDirectory = [self randomDirectoryWithPrefix:@"ExtractInvalidArchive"];
+    NSURL *extractURL = [self.tempDirectory URLByAppendingPathComponent:extractDirectory];
+
+    NSError *error = nil;
+    BOOL success = [archive extractFilesTo:extractURL.path
+                                 overwrite:NO
+                                     error:&error];
+    BOOL dirExists = [fm fileExistsAtPath:extractURL.path];
+
+    XCTAssertFalse(success, @"Extract invalid archive succeeded");
+    XCTAssertEqual(error.code, URKErrorCodeBadData, @"Unexpected error code returned");
+    XCTAssertFalse(dirExists, @"Directory successfully created for invalid archive");
+}
+
+- (void)testExtractFiles_ModifiedCRC_IgnoreCRCMismatches
+{
+    NSFileManager *fm = [NSFileManager defaultManager];
+
+    URKArchive *archive = [[URKArchive alloc] initWithURL:self.testFileURLs[@"Modified CRC Archive.rar"] error:nil];
+
+    BOOL fileHasIntegrity = [archive checkDataIntegrityIgnoringCRCMismatches:^BOOL{
+        return YES;
+    }];
+
+    XCTAssertTrue(fileHasIntegrity);
+
+    NSString *extractDirectory = [self randomDirectoryWithPrefix:@"ExtractInvalidArchive"];
+    NSURL *extractURL = [self.tempDirectory URLByAppendingPathComponent:extractDirectory];
+
+    NSError *error = nil;
+    BOOL success = [archive extractFilesTo:extractURL.path
+                                 overwrite:NO
+                                     error:&error];
+
+    XCTAssertTrue(success, @"CRC mismatch not ignored");
+    XCTAssertNil(error, @"Unexpected error code returned");
+
+    BOOL dirExists = [fm fileExistsAtPath:extractURL.path];
+    XCTAssertTrue(dirExists, @"Directory successfully created for invalid archive");
+
+    NSURL *extractedFileURL = [extractURL URLByAppendingPathComponent:@"README.md"];
+    XCTAssertTrue([extractedFileURL checkResourceIsReachableAndReturnError:NULL]);
+}
+
+- (void)testExtractFiles_ModifiedCRC_DontIgnoreCRCMismatches
+{
+    NSFileManager *fm = [NSFileManager defaultManager];
+
+    URKArchive *archive = [[URKArchive alloc] initWithURL:self.testFileURLs[@"Modified CRC Archive.rar"] error:nil];
+
+    BOOL fileHasIntegrity = [archive checkDataIntegrityIgnoringCRCMismatches:^BOOL{
+        return NO;
+    }];
+
+    XCTAssertFalse(fileHasIntegrity);
+
+    NSString *extractDirectory = [self randomDirectoryWithPrefix:@"ExtractInvalidArchive"];
+    NSURL *extractURL = [self.tempDirectory URLByAppendingPathComponent:extractDirectory];
+
+    NSError *error = nil;
+    BOOL success = [archive extractFilesTo:extractURL.path
+                                 overwrite:NO
+                                     error:&error];
+
+    XCTAssertFalse(success, @"CRC mismatch not ignored");
+    XCTAssertNotNil(error, @"Unexpected error code returned");
+
+    BOOL dirExists = [fm fileExistsAtPath:extractURL.path];
+    XCTAssertFalse(dirExists, @"Directory successfully created for invalid archive");
+}
+
 @end