Sparkle's AppInstaller post-stage-1 XPC listener accepts unvalidated connections, allowing spoofed appcast item data injection

@@ -97,6 +97,7 @@ @implementation AppInstaller
     
     // Setting _performedStage1Installation on main thread must be synchronzied with reading it from new connection handler
     BOOL _performedStage1Installation;
+    BOOL _receivedAppcastItemData;
     
     BOOL _performedStage2Installation;
     BOOL _performedStage3Installation;
@@ -142,9 +143,9 @@ - (BOOL)listener:(NSXPCListener *)__unused listener shouldAcceptNewConnection:(N
         BOOL connectionCodeSigningValidationSkipped = NO;
     #endif
         
-        // It's safe to allow any connections once stage 1 installation is complete
+        // It's safe to allow any connections once stage 1 installation is complete and appcast data has been received.
         // This is to allow general updaters to resume the installation.
-        if (!_performedStage1Installation) {
+        if (!_performedStage1Installation || !_receivedAppcastItemData) {
             BOOL passesValidation;
             NSError *validationError = nil;
             SUValidateConnectionStatus status = [SUCodeSigningVerifier validateConnection:newConnection error:&validationError];
@@ -597,6 +598,10 @@ - (void)handleMessageWithIdentifier:(int32_t)identifier data:(NSData *)data
             [self extractAndInstallUpdate];
         });
     } else if (identifier == SPUSentUpdateAppcastItemData) {
+        os_unfair_lock_lock(&_newConnectionLock);
+        _receivedAppcastItemData = YES;
+        os_unfair_lock_unlock(&_newConnectionLock);
+        
         SUAppcastItem *updateItem = (data != nil) ? (SUAppcastItem *)SPUUnarchiveRootObjectSecurely(data, [SUAppcastItem class]) : nil;
         if (updateItem != nil) {
             SPUInstallationInfo *installationInfo = [[SPUInstallationInfo alloc] initWithAppcastItem:updateItem];

@@ -599,18 +599,20 @@ - (void)handleMessageWithIdentifier:(int32_t)identifier data:(NSData *)data
         });
     } else if (identifier == SPUSentUpdateAppcastItemData) {
         os_unfair_lock_lock(&_newConnectionLock);
-        _receivedAppcastItemData = YES;
-        os_unfair_lock_unlock(&_newConnectionLock);
-        
-        SUAppcastItem *updateItem = (data != nil) ? (SUAppcastItem *)SPUUnarchiveRootObjectSecurely(data, [SUAppcastItem class]) : nil;
-        if (updateItem != nil) {
-            SPUInstallationInfo *installationInfo = [[SPUInstallationInfo alloc] initWithAppcastItem:updateItem];
+        if (!_receivedAppcastItemData) {
+            _receivedAppcastItemData = YES;
             
-            NSData *archivedData = SPUArchiveRootObjectSecurely(installationInfo);
-            if (archivedData != nil) {
-                [_agentConnection.agent registerInstallationInfoData:archivedData];
+            SUAppcastItem *updateItem = (data != nil) ? (SUAppcastItem *)SPUUnarchiveRootObjectSecurely(data, [SUAppcastItem class]) : nil;
+            if (updateItem != nil) {
+                SPUInstallationInfo *installationInfo = [[SPUInstallationInfo alloc] initWithAppcastItem:updateItem];
+                
+                NSData *archivedData = SPUArchiveRootObjectSecurely(installationInfo);
+                if (archivedData != nil) {
+                    [_agentConnection.agent registerInstallationInfoData:archivedData];
+                }
             }
         }
+        os_unfair_lock_unlock(&_newConnectionLock);
     } else if (identifier == SPUResumeInstallationToStage2 && data.length == sizeof(uint8_t) * 2) {
         // Because anyone can ask us to resume the installation, it may be wise to think about backwards compatibility here if IPC changes
         uint8_t relaunch = *((const uint8_t *)data.bytes);

@@ -4,7 +4,7 @@
 * Polish and update Spanish translations to be gender neutral (#2874, #2875) (Emilio P Egido)
 * Guard against NULL CFRelease() on failure condition in fallback path (#2867) (Zorg)
 * Guard against symlinks when applying delta update files (fe7b718) (Zorg, fg0x0)
-* Enforce connection to installer to be validated before receiving appcast item data (#2876) (Zorg, fg0x0)
+* Enforce connection to installer to be validated before receiving appcast item data (#2876, #2877) (Zorg, fg0x0)
 
 # 2.9.1
 

@@ -471,7 +471,9 @@ + (SUValidateConnectionStatus)validateConnection:(NSXPCConnection *)connection e
         SecCodeRef code = NULL;
         OSStatus result = SecCodeCopyGuestWithAttributes(NULL, (__bridge CFDictionaryRef _Nullable)(attributes), kSecCSDefaultFlags, &code);
         if (result != errSecSuccess) {
-            CFRelease(code);
+            if (code != NULL) {
+                CFRelease(code);
+            }
             
             if (error != NULL) {
                 *error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:@{ NSLocalizedDescriptionKey: [NSString stringWithFormat:@"The client connection could not be validated because SecCodeCopyGuestWithAttributes() failed with error %d", result] }];