Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixing:

Bug #389869: "Sparkle runs thread-unsafe code on secondary threads"
Bug #312995: "Canceling authentication request causes crash on next update"
Bug #388793: "Need to notify SUUpdateDriverFinishedNotification on main thread"

The unfortunate side-effect of this fix is that all the file-handling code is now CoreServices-based, since NSFileManager is not thread-safe. This is disgusting and will be stricken from all records when installation is performed by relaunch in Next Major, as it should have been in the first place.
  • Loading branch information...
commit 2fc1b66a799c6830eff7f461dc21fde2ad5da5a4 1 parent bd99d04
@andymatuschak andymatuschak authored
View
67 SUDiskImageUnarchiver.m
@@ -9,6 +9,7 @@
#import "SUDiskImageUnarchiver.h"
#import "SUUnarchiver_Private.h"
#import "NTSynchronousTask.h"
+#import <CoreServices/CoreServices.h>
@implementation SUDiskImageUnarchiver
@@ -28,18 +29,23 @@ - (void)_extractDMG
BOOL mountedSuccessfully = NO;
// get a unique mount point path
- NSString *mountPoint = [@"/Volumes" stringByAppendingPathComponent:[[NSProcessInfo processInfo] globallyUniqueString]];
-
- if ([[NSFileManager defaultManager] fileExistsAtPath:mountPoint]) goto reportError;
-
- // create mount point folder
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
- [[NSFileManager defaultManager] createDirectoryAtPath:mountPoint attributes:nil];
+ NSString *mountPointName = nil;
+ NSString *mountPoint = nil;
+ FSRef tmpRef;
+ do
+ {
+ CFUUIDRef uuid = CFUUIDCreate(NULL);
+ mountPointName = (NSString *)CFUUIDCreateString(NULL, uuid);
+ CFRelease(uuid);
+#if MAC_OS_X_VERSION_MAX_ALLOWED > MAC_OS_X_VERSION_10_4
+ [NSMakeCollectable((CFStringRef)mountPointName) autorelease];
#else
- [[NSFileManager defaultManager] createDirectoryAtPath:mountPoint withIntermediateDirectories:YES attributes:nil error:NULL];
+ [mountPointName autorelease];
#endif
- if (![[NSFileManager defaultManager] fileExistsAtPath:mountPoint]) goto reportError;
-
+ mountPoint = [@"/Volumes" stringByAppendingPathComponent:mountPointName];
+ }
+ while (noErr == FSPathMakeRefWithOptions((UInt8 *)[mountPoint fileSystemRepresentation], kFSPathMakeRefDoNotFollowLeafSymlink, &tmpRef, NULL));
+
NSArray* arguments = [NSArray arrayWithObjects:@"attach", archivePath, @"-mountpoint", mountPoint, @"-noverify", @"-nobrowse", @"-noautoopen", nil];
// set up a pipe and push "yes" (y works too), this will accept any license agreement crap
// not every .dmg needs this, but this will make sure it works with everyone
@@ -50,33 +56,16 @@ - (void)_extractDMG
mountedSuccessfully = YES;
// Now that we've mounted it, we need to copy out its contents.
- NSString *targetPath = [[archivePath stringByDeletingLastPathComponent] stringByAppendingPathComponent:[mountPoint lastPathComponent]];
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
- if (![[NSFileManager defaultManager] createDirectoryAtPath:targetPath attributes:nil]) goto reportError;
-#else
- if (![[NSFileManager defaultManager] createDirectoryAtPath:targetPath withIntermediateDirectories:YES attributes:nil error:NULL]) goto reportError;
-#endif
+ FSRef srcRef, dstRef;
+ OSErr err;
+ err = FSPathMakeRef((UInt8 *)[mountPoint fileSystemRepresentation], &srcRef, NULL);
+ if (err != noErr) goto reportError;
+ err = FSPathMakeRef((UInt8 *)[[archivePath stringByDeletingLastPathComponent] fileSystemRepresentation], &dstRef, NULL);
+ if (err != noErr) goto reportError;
+
+ err = FSCopyObjectSync(&srcRef, &dstRef, (CFStringRef)mountPointName, NULL, kFSFileOperationSkipSourcePermissionErrors);
+ if (err != noErr) goto reportError;
- // We can't just copyPath: from the volume root because that always fails. Seems to be a bug.
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
- id subpathEnumerator = [[[NSFileManager defaultManager] directoryContentsAtPath:mountPoint] objectEnumerator];
-#else
- id subpathEnumerator = [[[NSFileManager defaultManager] contentsOfDirectoryAtPath:mountPoint error:NULL] objectEnumerator];
-#endif
- NSString *currentSubpath;
- while ((currentSubpath = [subpathEnumerator nextObject]))
- {
- NSString *currentFullPath = [mountPoint stringByAppendingPathComponent:currentSubpath];
- // Don't bother trying (and failing) to copy out files we can't read. That's not going to be the app anyway.
- if (![[NSFileManager defaultManager] isReadableFileAtPath:currentFullPath]) continue;
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
- if (![[NSFileManager defaultManager] copyPath:currentFullPath toPath:[targetPath stringByAppendingPathComponent:currentSubpath] handler:nil])
-#else
- if (![[NSFileManager defaultManager] copyItemAtPath:currentFullPath toPath:[targetPath stringByAppendingPathComponent:currentSubpath] error:NULL])
-#endif
- goto reportError;
- }
-
[self performSelectorOnMainThread:@selector(_notifyDelegateOfSuccess) withObject:nil waitUntilDone:NO];
goto finally;
@@ -86,12 +75,6 @@ - (void)_extractDMG
finally:
if (mountedSuccessfully)
[NSTask launchedTaskWithLaunchPath:@"/usr/bin/hdiutil" arguments:[NSArray arrayWithObjects:@"detach", mountPoint, @"-force", nil]];
- else
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
- [[NSFileManager defaultManager] removeFileAtPath:mountPoint handler:nil];
-#else
- [[NSFileManager defaultManager] removeItemAtPath:mountPoint error:NULL];
-#endif
[pool drain];
}
View
46 SUPlainInstaller.m
@@ -8,45 +8,53 @@
#import "SUPlainInstaller.h"
#import "SUPlainInstallerInternals.h"
-#import "SUHost.h"
NSString *SUInstallerPathKey = @"SUInstallerPath";
+NSString *SUInstallerTargetPathKey = @"SUInstallerTargetPath";
+NSString *SUInstallerTempNameKey = @"SUInstallerTempName";
NSString *SUInstallerHostKey = @"SUInstallerHost";
NSString *SUInstallerDelegateKey = @"SUInstallerDelegate";
-NSString *SUInstallerVersionComparatorKey = @"SUInstallerVersionComparator";
+NSString *SUInstallerResultKey = @"SUInstallerResult";
+NSString *SUInstallerErrorKey = @"SUInstallerError";
@implementation SUPlainInstaller
-+ (void)installPath:(NSString *)path overHost:(SUHost *)bundle delegate:delegate versionComparator:(id <SUVersionComparison>)comparator
++ (void)_finishInstallationWithInfo:(NSDictionary *)info
{
- NSError *error;
- BOOL result = YES;
-
- // Prevent malicious downgrades:
- if ([comparator compareVersion:[bundle version] toVersion:[[NSBundle bundleWithPath:path] objectForInfoDictionaryKey:@"CFBundleVersion"]] == NSOrderedDescending)
- {
- NSString * errorMessage = [NSString stringWithFormat:@"Sparkle Updater: Possible attack in progress! Attempting to \"upgrade\" from %@ to %@. Aborting update.", [bundle version], [[NSBundle bundleWithPath:path] objectForInfoDictionaryKey:@"CFBundleVersion"]];
- result = NO;
- error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUDowngradeError userInfo:[NSDictionary dictionaryWithObject:errorMessage forKey:NSLocalizedDescriptionKey]];
- }
-
- if (result)
- result = [self copyPathWithAuthentication:path overPath:[bundle bundlePath] error:&error];
- [self _finishInstallationWithResult:result host:bundle error:error delegate:delegate];
+ [self _finishInstallationWithResult:[[info objectForKey:SUInstallerResultKey] boolValue] host:[info objectForKey:SUInstallerHostKey] error:[info objectForKey:SUInstallerErrorKey] delegate:[info objectForKey:SUInstallerDelegateKey]];
}
+ (void)_performInstallationWithInfo:(NSDictionary *)info
{
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
- [self installPath:[info objectForKey:SUInstallerPathKey] overHost:[info objectForKey:SUInstallerHostKey] delegate:[info objectForKey:SUInstallerDelegateKey] versionComparator:[info objectForKey:SUInstallerVersionComparatorKey]];
+ NSError *error = nil;
+ BOOL result = [self copyPathWithAuthentication:[info objectForKey:SUInstallerPathKey] overPath:[info objectForKey:SUInstallerTargetPathKey] temporaryName:[info objectForKey:SUInstallerTempNameKey] error:&error];
+
+ NSMutableDictionary *mutableInfo = [[info mutableCopy] autorelease];
+ [mutableInfo setObject:[NSNumber numberWithBool:result] forKey:SUInstallerResultKey];
+ if (!result && error)
+ [mutableInfo setObject:error forKey:SUInstallerErrorKey];
+ [self performSelectorOnMainThread:@selector(_finishInstallationWithInfo:) withObject:mutableInfo waitUntilDone:NO];
+
[pool drain];
}
+ (void)performInstallationWithPath:(NSString *)path host:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator
{
- NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:path, SUInstallerPathKey, host, SUInstallerHostKey, delegate, SUInstallerDelegateKey, comparator, SUInstallerVersionComparatorKey, nil];
+ // Prevent malicious downgrades:
+ if ([comparator compareVersion:[host version] toVersion:[[NSBundle bundleWithPath:path] objectForInfoDictionaryKey:@"CFBundleVersion"]] == NSOrderedDescending)
+ {
+ NSString * errorMessage = [NSString stringWithFormat:@"Sparkle Updater: Possible attack in progress! Attempting to \"upgrade\" from %@ to %@. Aborting update.", [host version], [[NSBundle bundleWithPath:path] objectForInfoDictionaryKey:@"CFBundleVersion"]];
+ NSError *error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUDowngradeError userInfo:[NSDictionary dictionaryWithObject:errorMessage forKey:NSLocalizedDescriptionKey]];
+ [self _finishInstallationWithResult:NO host:host error:error delegate:delegate];
+ return;
+ }
+
+ NSString *targetPath = [host bundlePath];
+ NSString *tempName = [self temporaryNameForPath:targetPath];
+ NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:path, SUInstallerPathKey, targetPath, SUInstallerTargetPathKey, tempName, SUInstallerTempNameKey, host, SUInstallerHostKey, delegate, SUInstallerDelegateKey, nil];
if (synchronously)
[self _performInstallationWithInfo:info];
else
View
3  SUPlainInstallerInternals.h
@@ -12,7 +12,8 @@
#import "SUPlainInstaller.h"
@interface SUPlainInstaller (Internals)
-+ (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst error:(NSError **)error;
++ (NSString *)temporaryNameForPath:(NSString *)path;
++ (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst temporaryName:(NSString *)tmp error:(NSError **)error;
@end
#endif
View
93 SUPlainInstallerInternals.m
@@ -9,6 +9,7 @@
#import "Sparkle.h"
#import "SUPlainInstallerInternals.h"
+#import <CoreServices/CoreServices.h>
#import <Security/Security.h>
#import <sys/stat.h>
#import <sys/wait.h>
@@ -60,42 +61,7 @@ static BOOL AuthorizationExecuteWithPrivilegesAndWait(AuthorizationRef authoriza
@implementation SUPlainInstaller (Internals)
-+ (BOOL)currentUserOwnsPath:(NSString *)oPath
-{
- const char *path = [oPath fileSystemRepresentation];
- uid_t uid = getuid();
- bool res = false;
- struct stat sb;
- if(stat(path, &sb) == 0)
- {
- if(sb.st_uid == uid)
- {
- res = true;
- if(sb.st_mode & S_IFDIR)
- {
- DIR* dir = opendir(path);
- struct dirent* entry = NULL;
- while(res && (entry = readdir(dir)))
- {
- if(strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
- continue;
-
- size_t len = strlen(path) + 1 + entry->d_namlen + 1;
- char descend[len];
- strlcpy(descend, path, len);
- strlcat(descend, "/", len);
- strlcat(descend, entry->d_name, len);
- NSString* newPath = [[NSFileManager defaultManager] stringWithFileSystemRepresentation:descend length:strlen(descend)];
- res = [self currentUserOwnsPath:newPath];
- }
- closedir(dir);
- }
- }
- }
- return res;
-}
-
-+ (NSString *)_temporaryInstallationPathForPath:(NSString *)path
++ (NSString *)temporaryNameForPath:(NSString *)path
{
// Let's try to read the version number so the filename will be more meaningful.
NSString *postFix;
@@ -116,12 +82,11 @@ + (NSString *)_temporaryInstallationPathForPath:(NSString *)path
int cnt=2;
while ([[NSFileManager defaultManager] fileExistsAtPath:tempDir] && cnt <= 999)
tempDir = [NSString stringWithFormat:@"%@ %d.%@", prefix, cnt++, [path pathExtension]];
- return tempDir;
+ return [tempDir lastPathComponent];
}
-+ (BOOL)_copyPathWithForcedAuthentication:(NSString *)src toPath:(NSString *)dst error:(NSError **)error
++ (BOOL)_copyPathWithForcedAuthentication:(NSString *)src toPath:(NSString *)dst temporaryPath:(NSString *)tmp error:(NSError **)error
{
- NSString *tmp = [self _temporaryInstallationPathForPath:dst];
const char* srcPath = [src fileSystemRepresentation];
const char* tmpPath = [tmp fileSystemRepresentation];
const char* dstPath = [dst fileSystemRepresentation];
@@ -190,7 +155,7 @@ + (BOOL)_copyPathWithForcedAuthentication:(NSString *)src toPath:(NSString *)dst
// change will almost certainly make it impossible to change
// attributes to release the files from the quarantine.
if (res) {
- [self releaseFromQuarantine:dst];
+ [self performSelectorOnMainThread:@selector(releaseFromQuarantine:) withObject:dst waitUntilDone:YES];
}
// Now move past the NULL we found and continue executing
@@ -220,35 +185,38 @@ + (BOOL)_copyPathWithForcedAuthentication:(NSString *)src toPath:(NSString *)dst
return res;
}
-+ (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst error:(NSError **)error
++ (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst temporaryName:(NSString *)tmp error:(NSError **)error
{
- if (![[NSFileManager defaultManager] fileExistsAtPath:dst])
+ FSRef srcRef, dstRef, targetRef, movedRef;
+ OSErr err;
+
+ err = FSPathMakeRefWithOptions((UInt8 *)[dst fileSystemRepresentation], kFSPathMakeRefDoNotFollowLeafSymlink, &dstRef, NULL);
+ if (err != noErr)
{
NSString *errorMessage = [NSString stringWithFormat:@"Couldn't copy %@ over %@ because there is no file at %@.", src, dst, dst];
if (error != NULL)
*error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUFileCopyFailure userInfo:[NSDictionary dictionaryWithObject:errorMessage forKey:NSLocalizedDescriptionKey]];
return NO;
}
-
- if (![[NSFileManager defaultManager] isWritableFileAtPath:dst] || ![[NSFileManager defaultManager] isWritableFileAtPath:[dst stringByDeletingLastPathComponent]])
- return [self _copyPathWithForcedAuthentication:src toPath:dst error:error];
-
- NSString *tmpPath = [self _temporaryInstallationPathForPath:dst];
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
- if (![[NSFileManager defaultManager] movePath:dst toPath:tmpPath handler:nil])
-#else
- if (![[NSFileManager defaultManager] moveItemAtPath:dst toPath:tmpPath error:NULL])
-#endif
+
+ NSString *tmpPath = [[dst stringByDeletingLastPathComponent] stringByAppendingPathComponent:tmp];
+
+ if (0 != access([dst fileSystemRepresentation], W_OK) || 0 != access([[dst stringByDeletingLastPathComponent] fileSystemRepresentation], W_OK))
+ return [self _copyPathWithForcedAuthentication:src toPath:dst temporaryPath:tmpPath error:error];
+
+ err = FSPathMakeRef((UInt8 *)[[dst stringByDeletingLastPathComponent] fileSystemRepresentation], &targetRef, NULL);
+ if (err == noErr)
+ err = FSMoveObjectSync(&dstRef, &targetRef, (CFStringRef)tmp, &movedRef, 0);
+ if (err != noErr)
{
if (error != NULL)
*error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUFileCopyFailure userInfo:[NSDictionary dictionaryWithObject:[NSString stringWithFormat:@"Couldn't move %@ to %@.", dst, tmpPath] forKey:NSLocalizedDescriptionKey]];
return NO;
}
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
- if (![[NSFileManager defaultManager] copyPath:src toPath:dst handler:nil])
-#else
- if (![[NSFileManager defaultManager] copyItemAtPath:src toPath:dst error:NULL])
-#endif
+ err = FSPathMakeRef((UInt8 *)[src fileSystemRepresentation], &srcRef, NULL);
+ if (err == noErr)
+ err = FSCopyObjectSync(&srcRef, &targetRef, (CFStringRef)[dst lastPathComponent], NULL, 0);
+ if (err != noErr)
{
if (error != NULL)
*error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUFileCopyFailure userInfo:[NSDictionary dictionaryWithObject:[NSString stringWithFormat:@"Couldn't copy %@ to %@.", src, dst] forKey:NSLocalizedDescriptionKey]];
@@ -256,8 +224,7 @@ + (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst erro
}
// Trash the old copy of the app.
- NSInteger tag = 0;
- if (![[NSWorkspace sharedWorkspace] performFileOperation:NSWorkspaceRecycleOperation source:[tmpPath stringByDeletingLastPathComponent] destination:@"" files:[NSArray arrayWithObject:[tmpPath lastPathComponent]] tag:&tag])
+ if (noErr != FSMoveObjectToTrashSync(&movedRef, NULL, 0))
NSLog(@"Sparkle error: couldn't move %@ to the trash. This is often a sign of a permissions error.", tmpPath);
// If the currently-running application is trusted, the new
@@ -269,7 +236,7 @@ + (BOOL)copyPathWithAuthentication:(NSString *)src overPath:(NSString *)dst erro
// new home in case it's moved across filesystems: if that
// happens, the move is actually a copy, and it may result
// in the application being quarantined.
- [self releaseFromQuarantine:dst];
+ [self performSelectorOnMainThread:@selector(releaseFromQuarantine:) withObject:dst waitUntilDone:YES];
return YES;
}
@@ -327,11 +294,7 @@ + (void)releaseFromQuarantine:(NSString*)root
// Only recurse if it's actually a directory. Don't recurse into a
// root-level symbolic link.
NSDictionary* rootAttributes =
-#if MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5
- [[NSFileManager defaultManager] fileAttributesAtPath:root traverseLink:NO];
-#else
- [[NSFileManager defaultManager] attributesOfItemAtPath:root error:NULL];
-#endif
+ [[NSFileManager defaultManager] fileAttributesAtPath:root traverseLink:NO];
NSString* rootType = [rootAttributes objectForKey:NSFileType];
if (rootType == NSFileTypeDirectory) {
View
2  SUUIBasedUpdateDriver.m
@@ -157,6 +157,7 @@ - (void)installUpdate
{
[statusController close];
[statusController autorelease];
+ statusController = nil;
}
}
@@ -173,6 +174,7 @@ - (void)abortUpdate
{
[statusController close];
[statusController autorelease];
+ statusController = nil;
}
[super abortUpdate];
}
Please sign in to comment.
Something went wrong with that request. Please try again.