From ddfafff5d6307111f39f9d0d6a5c41e9028fed66 Mon Sep 17 00:00:00 2001 From: dbeard Date: Tue, 30 Aug 2016 13:25:04 -0700 Subject: [PATCH] Guard against NSError null pointer dereferences --- ios/CodePush/CodePush.h | 4 +- ios/CodePush/CodePushPackage.m | 73 +++++++++++++----------- ios/CodePush/CodePushUpdateUtils.m | 89 ++++++++++++++++-------------- 3 files changed, 90 insertions(+), 76 deletions(-) diff --git a/ios/CodePush/CodePush.h b/ios/CodePush/CodePush.h index 4cd21a7..5174f1a 100644 --- a/ios/CodePush/CodePush.h +++ b/ios/CodePush/CodePush.h @@ -128,7 +128,7 @@ failCallback:(void (^)(NSError *err))failCallback; + (NSString *)getPackageFolderPath:(NSString *)packageHash; -+ (void)installPackage:(NSDictionary *)updatePackage ++ (BOOL)installPackage:(NSDictionary *)updatePackage removePendingUpdate:(BOOL)removePendingUpdate error:(NSError **)error; @@ -153,7 +153,7 @@ failCallback:(void (^)(NSError *err))failCallback; @interface CodePushUpdateUtils : NSObject -+ (void)copyEntriesInFolder:(NSString *)sourceFolder ++ (BOOL)copyEntriesInFolder:(NSString *)sourceFolder destFolder:(NSString *)destFolder error:(NSError **)error; diff --git a/ios/CodePush/CodePushPackage.m b/ios/CodePush/CodePushPackage.m index 6db7d2e..3349270 100644 --- a/ios/CodePush/CodePushPackage.m +++ b/ios/CodePush/CodePushPackage.m @@ -293,10 +293,10 @@ static NSString *const UnzippedFolderName = @"unzipped"; + (NSDictionary *)getCurrentPackage:(NSError **)error { NSString *packageHash = [CodePushPackage getCurrentPackageHash:error]; - if (*error || !packageHash) { + if (!packageHash) { return nil; } - + return [CodePushPackage getPackage:packageHash error:error]; } @@ -304,14 +304,14 @@ static NSString *const UnzippedFolderName = @"unzipped"; { NSString *packageFolder = [self getCurrentPackageFolderPath:error]; - if(*error) { - return NULL; + if (!packageFolder) { + return nil; } NSDictionary *currentPackage = [self getCurrentPackage:error]; - if(*error) { - return NULL; + if (!currentPackage) { + return nil; } NSString *relativeBundlePath = [currentPackage objectForKey:RelativeBundlePathKey]; @@ -325,7 +325,7 @@ static NSString *const UnzippedFolderName = @"unzipped"; + (NSString *)getCurrentPackageHash:(NSError **)error { NSDictionary *info = [self getCurrentPackageInfo:error]; - if (*error || !info) { + if (!info) { return nil; } @@ -336,14 +336,14 @@ static NSString *const UnzippedFolderName = @"unzipped"; { NSDictionary *info = [self getCurrentPackageInfo:error]; - if (*error) { - return NULL; + if (!info) { + return nil; } NSString *packageHash = info[@"currentPackage"]; if (!packageHash) { - return NULL; + return nil; } return [self getPackageFolderPath:packageHash]; @@ -359,7 +359,7 @@ static NSString *const UnzippedFolderName = @"unzipped"; NSString *content = [NSString stringWithContentsOfFile:statusFilePath encoding:NSUTF8StringEncoding error:error]; - if (*error) { + if (!content) { return nil; } @@ -367,7 +367,7 @@ static NSString *const UnzippedFolderName = @"unzipped"; NSDictionary* json = [NSJSONSerialization JSONObjectWithData:data options:kNilOptions error:error]; - if (*error) { + if (!json) { return nil; } @@ -392,8 +392,7 @@ static NSString *const UnzippedFolderName = @"unzipped"; NSString *updateMetadataString = [NSString stringWithContentsOfFile:updateMetadataFilePath encoding:NSUTF8StringEncoding error:error]; - - if (*error) { + if (!updateMetadataString) { return nil; } @@ -411,7 +410,7 @@ static NSString *const UnzippedFolderName = @"unzipped"; + (NSDictionary *)getPreviousPackage:(NSError **)error { NSString *packageHash = [self getPreviousPackageHash:error]; - if (*error || !packageHash) { + if (!packageHash) { return nil; } @@ -421,7 +420,7 @@ static NSString *const UnzippedFolderName = @"unzipped"; + (NSString *)getPreviousPackageHash:(NSError **)error { NSDictionary *info = [self getCurrentPackageInfo:error]; - if (*error) { + if (!info) { return nil; } @@ -438,25 +437,25 @@ static NSString *const UnzippedFolderName = @"unzipped"; return [[self getCodePushPath] stringByAppendingPathComponent:UnzippedFolderName]; } -+ (void)installPackage:(NSDictionary *)updatePackage ++ (BOOL)installPackage:(NSDictionary *)updatePackage removePendingUpdate:(BOOL)removePendingUpdate error:(NSError **)error { NSString *packageHash = updatePackage[@"packageHash"]; NSMutableDictionary *info = [self getCurrentPackageInfo:error]; - if (*error) { - return; + if (!info) { + return NO; } if (packageHash && [packageHash isEqualToString:info[@"currentPackage"]]) { // The current package is already the one being installed, so we should no-op. - return; + return YES; } if (removePendingUpdate) { NSString *currentPackageFolderPath = [self getCurrentPackageFolderPath:error]; - if (!*error && currentPackageFolderPath) { + if (currentPackageFolderPath) { // Error in deleting pending package will not cause the entire operation to fail. NSError *deleteError; [[NSFileManager defaultManager] removeItemAtPath:currentPackageFolderPath @@ -467,7 +466,7 @@ static NSString *const UnzippedFolderName = @"unzipped"; } } else { NSString *previousPackageHash = [self getPreviousPackageHash:error]; - if (!*error && previousPackageHash && ![previousPackageHash isEqualToString:packageHash]) { + if (previousPackageHash && ![previousPackageHash isEqualToString:packageHash]) { NSString *previousPackageFolderPath = [self getPackageFolderPath:previousPackageHash]; // Error in deleting old package will not cause the entire operation to fail. NSError *deleteError; @@ -481,31 +480,30 @@ static NSString *const UnzippedFolderName = @"unzipped"; } [info setValue:packageHash forKey:@"currentPackage"]; - - [self updateCurrentPackageInfo:info - error:error]; + return [self updateCurrentPackageInfo:info + error:error]; } + (void)rollbackPackage { NSError *error; NSMutableDictionary *info = [self getCurrentPackageInfo:&error]; - if (error) { + if (!info) { CPLog(@"Error getting current package info: %@", error); return; } - NSString *currentPackageFolderPath = [self getCurrentPackageFolderPath:&error]; - if (error) { + NSString *currentPackageFolderPath = [self getCurrentPackageFolderPath:&error]; + if (!currentPackageFolderPath) { CPLog(@"Error getting current package folder path: %@", error); return; } NSError *deleteError; - [[NSFileManager defaultManager] removeItemAtPath:currentPackageFolderPath + BOOL result = [[NSFileManager defaultManager] removeItemAtPath:currentPackageFolderPath error:&deleteError]; - if (deleteError) { - CPLog(@"Error deleting current package contents at %@", currentPackageFolderPath); + if (!result) { + CPLog(@"Error deleting current package contents at %@ error %@", currentPackageFolderPath, deleteError); } [info setValue:info[@"previousPackage"] forKey:@"currentPackage"]; @@ -514,18 +512,27 @@ static NSString *const UnzippedFolderName = @"unzipped"; [self updateCurrentPackageInfo:info error:&error]; } -+ (void)updateCurrentPackageInfo:(NSDictionary *)packageInfo ++ (BOOL)updateCurrentPackageInfo:(NSDictionary *)packageInfo error:(NSError **)error { NSData *packageInfoData = [NSJSONSerialization dataWithJSONObject:packageInfo options:0 error:error]; + if (!packageInfoData) { + return NO; + } + NSString *packageInfoString = [[NSString alloc] initWithData:packageInfoData encoding:NSUTF8StringEncoding]; - [packageInfoString writeToFile:[self getStatusFilePath] + BOOL result = [packageInfoString writeToFile:[self getStatusFilePath] atomically:YES encoding:NSUTF8StringEncoding error:error]; + + if (!result) { + return NO; + } + return YES; } @end \ No newline at end of file diff --git a/ios/CodePush/CodePushUpdateUtils.m b/ios/CodePush/CodePushUpdateUtils.m index df19dac..0db2bde 100644 --- a/ios/CodePush/CodePushUpdateUtils.m +++ b/ios/CodePush/CodePushUpdateUtils.m @@ -7,16 +7,16 @@ NSString * const AssetsFolderName = @"assets"; NSString * const BinaryHashKey = @"CodePushBinaryHash"; NSString * const ManifestFolderPrefix = @"CodePush"; -+ (void)addContentsOfFolderToManifest:(NSString *)folderPath ++ (BOOL)addContentsOfFolderToManifest:(NSString *)folderPath pathPrefix:(NSString *)pathPrefix manifest:(NSMutableArray *)manifest error:(NSError **)error { - NSArray* folderFiles = [[NSFileManager defaultManager] + NSArray *folderFiles = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:folderPath error:error]; - if (*error) { - return; + if (!folderFiles) { + return NO; } for (NSString *fileName in folderFiles) { @@ -30,12 +30,12 @@ NSString * const ManifestFolderPrefix = @"CodePush"; BOOL isDir = NO; if ([[NSFileManager defaultManager] fileExistsAtPath:fullFilePath isDirectory:&isDir] && isDir) { - [self addContentsOfFolderToManifest:fullFilePath - pathPrefix:relativePath - manifest:manifest - error:error]; - if (*error) { - return; + BOOL result = [self addContentsOfFolderToManifest:fullFilePath + pathPrefix:relativePath + manifest:manifest + error:error]; + if (!result) { + return NO; } } else { NSData *fileContents = [NSData dataWithContentsOfFile:fullFilePath]; @@ -43,6 +43,7 @@ NSString * const ManifestFolderPrefix = @"CodePush"; [manifest addObject:[[relativePath stringByAppendingString:@":"] stringByAppendingString:fileContentsHash]]; } } + return YES; } + (void)addFileToManifest:(NSURL *)fileURL @@ -62,7 +63,7 @@ NSString * const ManifestFolderPrefix = @"CodePush"; NSData *manifestData = [NSJSONSerialization dataWithJSONObject:sortedManifest options:kNilOptions error:error]; - if (*error) { + if (!manifestData) { return nil; } @@ -86,15 +87,15 @@ NSString * const ManifestFolderPrefix = @"CodePush"; return inputHash; } -+ (void)copyEntriesInFolder:(NSString *)sourceFolder ++ (BOOL)copyEntriesInFolder:(NSString *)sourceFolder destFolder:(NSString *)destFolder error:(NSError **)error { - NSArray* files = [[NSFileManager defaultManager] + NSArray *files = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:sourceFolder error:error]; - if (*error) { - return; + if (!files) { + return NO; } for (NSString *fileName in files) { @@ -103,33 +104,39 @@ NSString * const ManifestFolderPrefix = @"CodePush"; if ([[NSFileManager defaultManager] fileExistsAtPath:fullFilePath isDirectory:&isDir] && isDir) { NSString *nestedDestFolder = [destFolder stringByAppendingPathComponent:fileName]; - [self copyEntriesInFolder:fullFilePath - destFolder:nestedDestFolder - error:error]; + BOOL result = [self copyEntriesInFolder:fullFilePath + destFolder:nestedDestFolder + error:error]; + + if (!result) { + return NO; + } + } else { NSString *destFileName = [destFolder stringByAppendingPathComponent:fileName]; if ([[NSFileManager defaultManager] fileExistsAtPath:destFileName]) { - [[NSFileManager defaultManager] removeItemAtPath:destFileName error:error]; - if (*error) { - return; + BOOL result = [[NSFileManager defaultManager] removeItemAtPath:destFileName error:error]; + if (!result) { + return NO; } } if (![[NSFileManager defaultManager] fileExistsAtPath:destFolder]) { - [[NSFileManager defaultManager] createDirectoryAtPath:destFolder + BOOL result = [[NSFileManager defaultManager] createDirectoryAtPath:destFolder withIntermediateDirectories:YES attributes:nil error:error]; - if (*error) { - return; + if (!result) { + return NO; } } - [[NSFileManager defaultManager] copyItemAtPath:fullFilePath toPath:destFileName error:error]; - if (*error) { - return; + BOOL result = [[NSFileManager defaultManager] copyItemAtPath:fullFilePath toPath:destFileName error:error]; + if (!result) { + return NO; } } } + return YES; } + (NSString *)findMainBundleInFolder:(NSString *)folderPath @@ -139,7 +146,7 @@ NSString * const ManifestFolderPrefix = @"CodePush"; NSArray* folderFiles = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:folderPath error:error]; - if (*error) { + if (!folderFiles) { return nil; } @@ -151,7 +158,7 @@ NSString * const ManifestFolderPrefix = @"CodePush"; NSString *mainBundlePathInFolder = [self findMainBundleInFolder:fullFilePath expectedFileName:expectedFileName error:error]; - if (*error) { + if (!mainBundlePathInFolder) { return nil; } @@ -196,11 +203,11 @@ NSString * const ManifestFolderPrefix = @"CodePush"; // them to the generated content manifest. NSString *assetsPath = [CodePush bundleAssetsPath]; if ([[NSFileManager defaultManager] fileExistsAtPath:assetsPath]) { - [self addContentsOfFolderToManifest:assetsPath - pathPrefix:[NSString stringWithFormat:@"%@/%@", [self manifestFolderPrefix], @"assets"] - manifest:manifest - error:error]; - if (*error) { + BOOL result = [self addContentsOfFolderToManifest:assetsPath + pathPrefix:[NSString stringWithFormat:@"%@/%@", [self manifestFolderPrefix], @"assets"] + manifest:manifest + error:error]; + if (!result) { return nil; } } @@ -239,17 +246,17 @@ NSString * const ManifestFolderPrefix = @"CodePush"; error:(NSError **)error { NSMutableArray *updateContentsManifest = [NSMutableArray array]; - [self addContentsOfFolderToManifest:finalUpdateFolder - pathPrefix:@"" - manifest:updateContentsManifest - error:error]; - if (*error) { + BOOL result = [self addContentsOfFolderToManifest:finalUpdateFolder + pathPrefix:@"" + manifest:updateContentsManifest + error:error]; + if (!result) { return NO; } - + NSString *updateContentsManifestHash = [self computeFinalHashFromManifest:updateContentsManifest error:error]; - if (*error || updateContentsManifestHash == nil) { + if (!updateContentsManifestHash) { return NO; }