More security tidbits!

This patch prevents malicious downgrades, which are still possible with DSA validation: suppose there's some (signed) version with a security hole. A malicious attacker could serve an appcast with that version's URL and DSA signature, but a higher version number, forcing the user to "upgrade" to the version with the security hole.

While I was at it, I fixed a bug that should have completely stopped .pkg installation from working since 1.5b1. Why didn't I hear anything about that? Does anyone actually use .pkgs? It still needs testing to be sure it works.

SUBasicUpdateDriver.m

@@ -227,7 +227,7 @@ - (void)installUpdate
 	if ([[NSFileManager defaultManager] copyPath:relaunchPathToCopy toPath:targetPath handler:nil])
 		relaunchPath = [targetPath retain];
 
-	[SUInstaller installFromUpdateFolder:[downloadPath stringByDeletingLastPathComponent] overHost:host delegate:self synchronously:[self shouldInstallSynchronously]];
+	[SUInstaller installFromUpdateFolder:[downloadPath stringByDeletingLastPathComponent] overHost:host delegate:self synchronously:[self shouldInstallSynchronously] versionComparator:[self _versionComparator]];
 }
 
 - (void)installerFinishedForHost:(SUHost *)aHost

SUConstants.h

@@ -51,6 +51,7 @@ extern OSStatus SUMissingUpdateError;
 extern OSStatus SUMissingInstallerToolError;
 extern OSStatus SURelaunchError;
 extern OSStatus SUInstallationError;
+extern OSStatus SUDowngradeError;
 
 // NSInteger is a type that was added to Leopard.
 // Here is some glue to ensure that NSInteger will work with pre-10.5 SDKs:

SUConstants.m

@@ -46,3 +46,4 @@
 OSStatus SUMissingInstallerToolError = 4003;
 OSStatus SURelaunchError = 4004;
 OSStatus SUInstallationError = 4005;
+OSStatus SUDowngradeError = 4006;

SUInstaller.h

@@ -10,10 +10,11 @@
 #define SUINSTALLER_H
 
 #import <Cocoa/Cocoa.h>
+#import "SUVersionComparisonProtocol.h"
 
 @class SUHost;
 @interface SUInstaller : NSObject { }
-+ (void)installFromUpdateFolder:(NSString *)updateFolder overHost:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously;
++ (void)installFromUpdateFolder:(NSString *)updateFolder overHost:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator;
 + (void)_finishInstallationWithResult:(BOOL)result host:(SUHost *)host error:(NSError *)error delegate:delegate;
 @end
 

SUInstaller.m

@@ -10,10 +10,6 @@
 #import "SUPlainInstaller.h"
 #import "SUPackageInstaller.h"
 
-NSString *SUInstallerPathKey = @"SUInstallerPath";
-NSString *SUInstallerHostKey = @"SUInstallerHost";
-NSString *SUInstallerDelegateKey = @"SUInstallerDelegate";
-
 @implementation SUInstaller
 
 + (BOOL)_isAliasFolderAtPath:(NSString *)path
@@ -36,7 +32,7 @@ + (BOOL)_isAliasFolderAtPath:(NSString *)path
 }
 
 
-+ (void)installFromUpdateFolder:(NSString *)updateFolder overHost:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously
++ (void)installFromUpdateFolder:(NSString *)updateFolder overHost:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator
 {
 	// Search subdirectories for the application
 	NSString *currentFile, *newAppDownloadPath = nil, *bundleFileName = [[host bundlePath] lastPathComponent], *alternateBundleFileName = [[host name] stringByAppendingPathExtension:[[host bundlePath] pathExtension]];
@@ -76,7 +72,7 @@ + (void)installFromUpdateFolder:(NSString *)updateFolder overHost:(SUHost *)host
 	}
 	else
 	{
-		[(isPackage ? [SUPackageInstaller class] : [SUPlainInstaller class]) performInstallationWithPath:newAppDownloadPath host:host delegate:delegate synchronously:synchronously];
+		[(isPackage ? [SUPackageInstaller class] : [SUPlainInstaller class]) performInstallationWithPath:newAppDownloadPath host:host delegate:delegate synchronously:synchronously versionComparator:comparator];
 	}
 }
 

SUPackageInstaller.h

@@ -13,7 +13,6 @@
 #import "SUPlainInstaller.h"
 
 @interface SUPackageInstaller : SUPlainInstaller { }
-+ (void)installPath:(NSString *)path overHost:(SUHost *)bundle delegate:delegate;
 @end
 
 #endif

SUPackageInstaller.m

@@ -11,7 +11,7 @@
 
 @implementation SUPackageInstaller
 
-+ (void)installPath:(NSString *)path overHost:(SUHost *)bundle delegate:delegate
++ (void)performInstallationWithPath:(NSString *)path host:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator;
 {
 	NSError *error = nil;
 	BOOL result = YES;
@@ -26,7 +26,7 @@ + (void)installPath:(NSString *)path overHost:(SUHost *)bundle delegate:delegate
 	NSTask *installer = [NSTask launchedTaskWithLaunchPath:installerPath arguments:[NSArray arrayWithObjects:path, nil]];
 	[installer waitUntilExit];
 	// Known bug: if the installation fails or is canceled, Sparkle goes ahead and restarts, thinking everything is fine.
-	[self _finishInstallationWithResult:result host:bundle error:error delegate:delegate];
+	[self _finishInstallationWithResult:result host:host error:error delegate:delegate];
 }
 
 @end

SUPlainInstaller.h

@@ -11,9 +11,10 @@
 
 #import "Sparkle.h"
 #import "SUInstaller.h"
+#import "SUVersionComparisonProtocol.h"
 
 @interface SUPlainInstaller : SUInstaller { }
-+ (void)performInstallationWithPath:(NSString *)path host:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously;
++ (void)performInstallationWithPath:(NSString *)path host:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator;
 @end
 
 #endif

SUPlainInstaller.m

@@ -9,31 +9,43 @@
 #import "SUPlainInstaller.h"
 #import "SUPlainInstallerInternals.h"
 
-extern NSString *SUInstallerPathKey;
-extern NSString *SUInstallerHostKey;
-extern NSString *SUInstallerDelegateKey;
+NSString *SUInstallerPathKey = @"SUInstallerPath";
+NSString *SUInstallerHostKey = @"SUInstallerHost";
+NSString *SUInstallerDelegateKey = @"SUInstallerDelegate";
+NSString *SUInstallerVersionComparatorKey = @"SUInstallerVersionComparator";
 
 @implementation SUPlainInstaller
 
-+ (void)installPath:(NSString *)path overHost:(SUHost *)bundle delegate:delegate
++ (void)installPath:(NSString *)path overHost:(SUHost *)bundle delegate:delegate versionComparator:(id <SUVersionComparison>)comparator
 {
 	NSError *error;
-	BOOL result = [self copyPathWithAuthentication:path overPath:[bundle bundlePath] error:&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];
 }
 
 + (void)_performInstallationWithInfo:(NSDictionary *)info
 {
 	NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
 
-	[self installPath:[info objectForKey:SUInstallerPathKey] overHost:[info objectForKey:SUInstallerHostKey] delegate:[info objectForKey:SUInstallerDelegateKey]];
+	[self installPath:[info objectForKey:SUInstallerPathKey] overHost:[info objectForKey:SUInstallerHostKey] delegate:[info objectForKey:SUInstallerDelegateKey] versionComparator:[info objectForKey:SUInstallerVersionComparatorKey]];
 
 	[pool drain];
 }
 
-+ (void)performInstallationWithPath:(NSString *)path host:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously;
++ (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, nil];
+	NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:path, SUInstallerPathKey, host, SUInstallerHostKey, delegate, SUInstallerDelegateKey, comparator, SUInstallerVersionComparatorKey, nil];
 	if (synchronously)
 		[self _performInstallationWithInfo:info];
 	else