Serialize animated-image frame decodes; refuse damaged frames; fix fr…

.github/workflows/ci.yaml

@@ -11,8 +11,9 @@ on:
       - master
 
 env:
-  # Use Xcode 15.2 or newer to support VisionOS
-  DEVELOPER_DIR: /Applications/Xcode_16.4.app
+  # Must exist on the current macos-latest runner image:
+  # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md
+  DEVELOPER_DIR: /Applications/Xcode_26.5.0.app/Contents/Developer
 
 jobs:
   debuggithub:
@@ -28,19 +29,13 @@ jobs:
   analyze:
     name: Analyze
     runs-on: macos-latest
-    strategy:
-      matrix:
-        platform: ['iOS Simulator,name=iPhone 16,OS=18.5']
     steps:
       - uses: actions/checkout@v2
       - name: Analyze
         run: make analyze
   test:
     name: Test
     runs-on: macos-latest
-    strategy:
-      matrix:
-        platform: ['iOS Simulator,name=iPhone 16,OS=18.5']
     steps:
       - uses: actions/checkout@v2
       - name: Test

.github/workflows/publish_release.yml

@@ -7,8 +7,9 @@ on:
         required: true
 
 env:
-  # Use Xcode 15.2 or newer to support VisionOS
-  DEVELOPER_DIR: /Applications/Xcode_16.4.app
+  # Must exist on the current macos-latest runner image:
+  # https://github.com/actions/runner-images/blob/main/images/macos/macos-26-arm64-Readme.md
+  DEVELOPER_DIR: /Applications/Xcode_26.5.0.app/Contents/Developer
 
 jobs:
   create_release:

Makefile

@@ -1,5 +1,5 @@
-PLATFORM="platform=iOS Simulator,name=iPhone 16,OS=18.5"
-SDK="iphonesimulator18.5"
+PLATFORM="platform=iOS Simulator,name=iPhone 17"
+SDK="iphonesimulator"
 SHELL=/bin/bash -o pipefail
 XCODE_MAJOR_VERSION=$(shell xcodebuild -version | HEAD -n 1 | sed -E 's/Xcode ([0-9]+).*/\1/')
 IOS_EXAMPLE_PROJECT="Examples/Example-Xcode-SPM/Example-Xcode-SPM.xcodeproj"
@@ -14,13 +14,13 @@ analyze:
 	xcodebuild clean analyze -destination ${PLATFORM} -sdk ${SDK} -workspace PINRemoteImage.xcworkspace -scheme PINRemoteImage \
 	CODE_SIGNING_REQUIRED=NO \
 	CLANG_ANALYZER_OUTPUT=plist-html \
-	CLANG_ANALYZER_OUTPUT_DIR="$(shell pwd)/clang" | xcpretty
+	CLANG_ANALYZER_OUTPUT_DIR="$(shell pwd)/clang" | xcbeautify
 	if [[ -n `find $(shell pwd)/clang -name "*.html"` ]] ; then rm -rf `pwd`/clang; exit 1; fi
 	rm -rf $(shell pwd)/clang
 
 test:
 	xcodebuild clean test -destination ${PLATFORM} -sdk ${SDK} -workspace PINRemoteImage.xcworkspace -scheme PINRemoteImage \
-	CODE_SIGNING_REQUIRED=NO | xcpretty
+	CODE_SIGNING_REQUIRED=NO | xcbeautify
 
 carthage:
 	carthage update --no-use-binaries --no-build
@@ -36,6 +36,6 @@ example:
 	fi
 	xcodebuild clean build -project ${IOS_EXAMPLE_PROJECT} -scheme ${EXAMPLE_SCHEME} -destination ${PLATFORM} -sdk ${SDK} \
 	ONLY_ACTIVE_ARCH=NO \
-	CODE_SIGNING_REQUIRED=NO | xcpretty
+	CODE_SIGNING_REQUIRED=NO | xcbeautify
 
 all: carthage test cocoapods analyze spm example

Source/Classes/AnimatedImages/PINAPNGAnimatedImage.m

@@ -19,6 +19,7 @@
 
 #import <PINRemoteImage/PINImage+DecodedImage.h>
 #import <PINRemoteImage/NSData+ImageDetectors.h>
+#import "PINRemoteLock.h"
 
 @interface PINAPNGAnimatedImage ()
 {
@@ -31,6 +32,7 @@ @interface PINAPNGAnimatedImage ()
     size_t _loopCount;
     CFTimeInterval *_durations;
     NSError *_error;
+    PINRemoteLock *_decodeLock; // serializes frame decodes on _imageSource
 }
 @end
 
@@ -40,6 +42,7 @@ - (instancetype)initWithAnimatedImageData:(NSData *)animatedImageData
 {
     if (self = [super init]) {
         _animatedImageData = animatedImageData;
+        _decodeLock = [[PINRemoteLock alloc] initWithName:@"PINAPNGAnimatedImage decode lock"];
         _imageSource =
             CGImageSourceCreateWithData((CFDataRef)animatedImageData,
                                         (CFDictionaryRef)@{(__bridge NSString *)kCGImageSourceTypeIdentifierHint:
@@ -150,7 +153,16 @@ - (CFTimeInterval)durationAtIndex:(NSUInteger)index
 
 - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id<PINCachedAnimatedFrameProvider>)cacheProvider
 {
-    // I believe this is threadsafe as CGImageSource *seems* immutable…
+    // same hardening as PINGIFAnimatedImage — serialize
+    // all ImageIO calls on the shared source (including the status query, which
+    // advances parser state), and refuse affirmatively damaged frames.
+    [_decodeLock lock];
+    CGImageSourceStatus frameStatus = CGImageSourceGetStatusAtIndex(_imageSource, index);
+    if (frameStatus == kCGImageStatusInvalidData || frameStatus == kCGImageStatusUnexpectedEOF) {
+        [_decodeLock unlock];
+        return NULL;
+    }
+
     CGImageRef imageRef =
         CGImageSourceCreateImageAtIndex(_imageSource,
                                         index,
@@ -161,7 +173,8 @@ - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id<PINCache
         CGImageRelease(imageRef);
         imageRef = decodedImageRef;
     }
-
+    [_decodeLock unlock];
+
     return imageRef;
 }
 

Source/Classes/AnimatedImages/PINCachedAnimatedImage.m

@@ -361,10 +361,10 @@ - (void)_cacheWithFrameIndex:(NSUInteger)frameIndex
     CGImageRef imageRef = [self->_animatedImage imageAtIndex:frameIndex cacheProvider:self];
     PINLog(@"Generating: %lu", (unsigned long)frameIndex);
 
-    if (imageRef) {
-        __block PINImage *coverImage = nil;
-        __block PINAnimatedImageInfoReady coverImageReadyCallback = nil;
-        [self->_lock lockWithBlock:^{
+    __block PINImage *coverImage = nil;
+    __block PINAnimatedImageInfoReady coverImageReadyCallback = nil;
+    [self->_lock lockWithBlock:^{
+        if (imageRef) {
             [self->_frameCache setObject:(__bridge id _Nonnull)(imageRef) forKey:@(frameIndex)];
 
             // Update the cover image
@@ -373,26 +373,31 @@ - (void)_cacheWithFrameIndex:(NSUInteger)frameIndex
                 coverImageReadyCallback = notifyCallback ? self->_coverImageReadyCallback : nil;
                 coverImage = self->_coverImage;
             }
+        } else {
+            // a frame that fails to decode must release its
+            // render slot; leaving it in _cachedOrCachingFrames with _frameRenderCount
+            // held meant playbackReady never fired and the frame was never retried.
+            [self->_cachedOrCachingFrames removeIndex:frameIndex];
+        }
 
-            self->_frameRenderCount--;
-            NSAssert(self->_frameRenderCount >= 0, @"playback ready is less than zero, something is wrong :(");
+        self->_frameRenderCount--;
+        NSAssert(self->_frameRenderCount >= 0, @"playback ready is less than zero, something is wrong :(");
 
-            PINLog(@"Frames left: %ld", (long)_frameRenderCount);
+        PINLog(@"Frames left: %ld", (long)_frameRenderCount);
 
-            dispatch_block_t notify = nil;
-            if (self->_frameRenderCount == 0 && self->_notifyOnReady) {
-                self->_notifyOnReady = NO;
-                if (self->_playbackReadyCallback) {
-                    notify = self->_playbackReadyCallback;
-                    [self->_operationQueue scheduleOperation:^{
-                        notify();
-                    }];
-                }
+        dispatch_block_t notify = nil;
+        if (self->_frameRenderCount == 0 && self->_notifyOnReady) {
+            self->_notifyOnReady = NO;
+            if (self->_playbackReadyCallback) {
+                notify = self->_playbackReadyCallback;
+                [self->_operationQueue scheduleOperation:^{
+                    notify();
+                }];
             }
-        }];
-        if (coverImageReadyCallback) {
-            coverImageReadyCallback(coverImage);
         }
+    }];
+    if (coverImageReadyCallback) {
+        coverImageReadyCallback(coverImage);
     }
 }
 

Source/Classes/AnimatedImages/PINGIFAnimatedImage.m

@@ -17,6 +17,7 @@
 
 #import <PINRemoteImage/PINImage+DecodedImage.h>
 #import <PINRemoteImage/NSData+ImageDetectors.h>
+#import "PINRemoteLock.h"
 
 @interface PINGIFAnimatedImage ()
 {
@@ -29,6 +30,7 @@ @interface PINGIFAnimatedImage ()
     size_t _loopCount;
     CFTimeInterval *_durations;
     NSError *_error;
+    PINRemoteLock *_decodeLock; // serializes frame decodes on _imageSource
 }
 @end
 
@@ -38,6 +40,7 @@ - (instancetype)initWithAnimatedImageData:(NSData *)animatedImageData
 {
     if (self = [super init]) {
         _animatedImageData = animatedImageData;
+        _decodeLock = [[PINRemoteLock alloc] initWithName:@"PINGIFAnimatedImage decode lock"];
         _imageSource =
             CGImageSourceCreateWithData((CFDataRef)animatedImageData,
                                         (CFDictionaryRef)@{(__bridge NSString *)kCGImageSourceTypeIdentifierHint:
@@ -148,7 +151,26 @@ - (CFTimeInterval)durationAtIndex:(NSUInteger)index
 
 - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id<PINCachedAnimatedFrameProvider>)cacheProvider
 {
-    // I believe this is threadsafe as CGImageSource *seems* immutable…
+    // serialize ALL ImageIO calls on the shared source.
+    // Despite the optimistic comment this replaced ("CGImageSource *seems* immutable"),
+    // CGImageSource is not safe for concurrent access, and PINCachedAnimatedImage
+    // reaches this concurrently from the caching queue, the init-time warmup block,
+    // and coverImage callers. The status query below also advances ImageIO parser
+    // state, so it must be inside the lock too.
+    [_decodeLock lock];
+
+    // Refuse frames whose data is affirmatively damaged. Frames are decoded lazily
+    // (kCGImageSourceShouldCache is false), so a damaged frame doesn't fail at
+    // creation — it crashes later inside CGContextDrawImage when CoreGraphics
+    // dereferences the failed decode. Only hard-failure statuses are rejected:
+    // kCGImageStatusIncomplete is what trailer-less-but-renderable GIFs (common in
+    // the wild) report, and those decode fine.
+    CGImageSourceStatus frameStatus = CGImageSourceGetStatusAtIndex(_imageSource, index);
+    if (frameStatus == kCGImageStatusInvalidData || frameStatus == kCGImageStatusUnexpectedEOF) {
+        [_decodeLock unlock];
+        return NULL;
+    }
+
     CGImageRef imageRef =
         CGImageSourceCreateImageAtIndex(_imageSource,
                                         index,
@@ -159,7 +181,8 @@ - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id<PINCache
         CGImageRelease(imageRef);
         imageRef = decodedImageRef;
     }
-
+    [_decodeLock unlock];
+
     return imageRef;
 }
 

Source/Classes/AnimatedImages/PINWebPAnimatedImage.m

@@ -20,6 +20,7 @@
 
 #import <PINRemoteImage/PINImage+DecodedImage.h>
 #import <PINRemoteImage/NSData+ImageDetectors.h>
+#import "PINRemoteLock.h"
 
 @interface PINWebPAnimatedImage ()
 {
@@ -32,6 +33,7 @@ @interface PINWebPAnimatedImage ()
     size_t _loopCount;
     CFTimeInterval *_durations;
     NSError *_error;
+    PINRemoteLock *_decodeLock; // serializes frame decodes on _imageSource
 }
 @end
 
@@ -41,7 +43,8 @@ - (instancetype)initWithAnimatedImageData:(NSData *)animatedImageData
 {
     if (self = [super init]) {
         _animatedImageData = animatedImageData;
-
+        _decodeLock = [[PINRemoteLock alloc] initWithName:@"PINWebPAnimatedImage decode lock"];
+
         _imageSource =
             CGImageSourceCreateWithData((CFDataRef)animatedImageData,
                                         (CFDictionaryRef)@{(__bridge NSString *)kCGImageSourceTypeIdentifierHint:
@@ -152,7 +155,18 @@ - (CFTimeInterval)durationAtIndex:(NSUInteger)index
 
 - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id<PINCachedAnimatedFrameProvider>)cacheProvider
 {
-    // I believe this is threadsafe as CGImageSource *seems* immutable…
+    // same hardening as PINGIFAnimatedImage — serialize
+    // all ImageIO calls on the shared source (including the status query, which
+    // advances parser state), and refuse affirmatively damaged frames. CGImageSource
+    // is not safe for concurrent lazy decodes of the same source, despite the
+    // optimistic comment this replaced.
+    [_decodeLock lock];
+    CGImageSourceStatus frameStatus = CGImageSourceGetStatusAtIndex(_imageSource, index);
+    if (frameStatus == kCGImageStatusInvalidData || frameStatus == kCGImageStatusUnexpectedEOF) {
+        [_decodeLock unlock];
+        return NULL;
+    }
+
     CGImageRef imageRef =
         CGImageSourceCreateImageAtIndex(_imageSource,
                                         index,
@@ -163,7 +177,8 @@ - (CGImageRef)imageAtIndex:(NSUInteger)index cacheProvider:(nullable id<PINCache
         CGImageRelease(imageRef);
         imageRef = decodedImageRef;
     }
-
+    [_decodeLock unlock];
+
     return imageRef;
 }
 

Source/Classes/Categories/PINImage+DecodedImage.m

@@ -229,9 +229,15 @@
 
 + (CGImageRef)pin_decodedImageRefWithCGImageRef:(CGImageRef)imageRef
 {
+    // Guard against NULL despite the nonnull annotation: CGImageSourceCreateImageAtIndex returns
+    // NULL for corrupt/truncated frames and callers have historically passed that straight through.
+    if (imageRef == NULL) {
+        return NULL;
+    }
+
     CGSize imageSize = CGSizeMake(CGImageGetWidth(imageRef), CGImageGetHeight(imageRef));
 
     CGBitmapInfo info = pin_CGImageRefIsOpaque(imageRef) ? (kCGImageAlphaNoneSkipFirst | kCGBitmapByteOrder32Host) : (kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host);
     CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB();
 
     //Use UIGraphicsBeginImageContext parameters from docs: https://developer.apple.com/library/ios/documentation/UIKit/Reference/UIKitFunctionReference/#//apple_ref/c/func/UIGraphicsBeginImageContextWithOptions
@@ -253,9 +259,15 @@
         }
         CGContextRelease(ctx);
         return decodedImageRef;
-        
+
     }
-
+
+    // match the success path's +0 autoreleased contract.
+    // Returning the borrowed imageRef directly made the animated-image callers release
+    // it and then return the same (now dangling) pointer whenever CGBitmapContextCreate
+    // failed under memory pressure — an over-release/use-after-free.
+    CGImageRetain(imageRef);
+    CFAutorelease(imageRef);
     return imageRef;
 }