From 6d73e0186134106b68260de9f3da8dda3ced4657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20M=C3=BCcke?= <317346+fabianmuecke@users.noreply.github.com> Date: Mon, 14 Jul 2025 13:02:03 +0200 Subject: [PATCH 1/2] Fix a rare crash in TSKSPKIHashCache --- TrustKit/Pinning/TSKSPKIHashCache.m | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/TrustKit/Pinning/TSKSPKIHashCache.m b/TrustKit/Pinning/TSKSPKIHashCache.m index fac360e..37840e0 100644 --- a/TrustKit/Pinning/TSKSPKIHashCache.m +++ b/TrustKit/Pinning/TSKSPKIHashCache.m @@ -266,8 +266,10 @@ - (NSData *)hashSubjectPublicKeyInfoFromCertificate:(SecCertificateRef)certifica void (^updateCacheBlock)(void) = ^{ if (isProtectedDataAvailable()) { - NSData *serializedSpkiCache = [NSKeyedArchiver archivedDataWithRootObject:weakSelf.spkiCache requiringSecureCoding:YES error:nil]; - if ([serializedSpkiCache writeToURL:[weakSelf SPKICachePath] atomically:YES] == NO) { + __strong typeof(weakSelf) strongSelf = weakSelf; + if (!strongSelf) return; + NSData *serializedSpkiCache = [NSKeyedArchiver archivedDataWithRootObject:strongSelf.spkiCache requiringSecureCoding:YES error:nil]; + if ([serializedSpkiCache writeToURL:[strongSelf SPKICachePath] atomically:YES] == NO) { NSAssert(false, @"Failed to write cache"); TSKLog(@"Could not persist SPKI cache to the filesystem"); } From 580ce02d76877d7cf0643d01eb6588fcd67d3456 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20M=C3=BCcke?= <317346+fabianmuecke@users.noreply.github.com> Date: Tue, 2 Sep 2025 11:15:48 +0200 Subject: [PATCH 2/2] Fix a crash in TSKSPKIHashCache caused by concurrent read write operations. --- TrustKit/Pinning/TSKSPKIHashCache.m | 21 +++++++---- TrustKitTests/TSKPublicKeyAlgorithmTests.m | 42 ++++++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/TrustKit/Pinning/TSKSPKIHashCache.m b/TrustKit/Pinning/TSKSPKIHashCache.m index 37840e0..dbaa82f 100644 --- a/TrustKit/Pinning/TSKSPKIHashCache.m +++ b/TrustKit/Pinning/TSKSPKIHashCache.m @@ -266,13 +266,20 @@ - (NSData *)hashSubjectPublicKeyInfoFromCertificate:(SecCertificateRef)certifica void (^updateCacheBlock)(void) = ^{ if (isProtectedDataAvailable()) { - __strong typeof(weakSelf) strongSelf = weakSelf; - if (!strongSelf) return; - NSData *serializedSpkiCache = [NSKeyedArchiver archivedDataWithRootObject:strongSelf.spkiCache requiringSecureCoding:YES error:nil]; - if ([serializedSpkiCache writeToURL:[strongSelf SPKICachePath] atomically:YES] == NO) { - NSAssert(false, @"Failed to write cache"); - TSKLog(@"Could not persist SPKI cache to the filesystem"); - } + dispatch_queue_t lockQueue = weakSelf.lockQueue; + if (!lockQueue) return; + + dispatch_sync(lockQueue, ^{ + NSDictionary *spkiCache = weakSelf.spkiCache; + if (!spkiCache) return; + NSData *serializedSpkiCache = [NSKeyedArchiver archivedDataWithRootObject:spkiCache requiringSecureCoding:YES error:nil]; + NSURL *cacheURL = [weakSelf SPKICachePath]; + if (!cacheURL) return; + if ([serializedSpkiCache writeToURL:cacheURL atomically:YES] == NO) { + NSAssert(false, @"Failed to write cache"); + TSKLog(@"Could not persist SPKI cache to the filesystem"); + } + }); } else { TSKLog(@"Protected data not available, skipping SPKI cache persistence"); diff --git a/TrustKitTests/TSKPublicKeyAlgorithmTests.m b/TrustKitTests/TSKPublicKeyAlgorithmTests.m index 5410f84..cd944ad 100644 --- a/TrustKitTests/TSKPublicKeyAlgorithmTests.m +++ b/TrustKitTests/TSKPublicKeyAlgorithmTests.m @@ -156,4 +156,46 @@ - (void)testSPKICacheThreadSafetyAndProtectedData [self waitForExpectationsWithTimeout:5.0 handler:nil]; } +// This test hardly manages to reproduce the crash, but it does reproduce it sometimes. +- (void)testSPKICacheThreadSafetyAndProtectedDataDoesntCrash +{ + XCTestExpectation *expectation = [self expectationWithDescription:@"Cache operations completed"]; + + id mockApplication = OCMClassMock([UIApplication class]); + OCMStub([mockApplication sharedApplication]).andReturn(mockApplication); + OCMStub([mockApplication isProtectedDataAvailable]).andReturn(YES); + + // Perform multiple cache operations in parallel on a background queue + SecCertificateRef certificate = [TSKCertificateUtils createCertificateFromDer:@"www.globalsign.com"]; + dispatch_group_t group = dispatch_group_create(); + for (int i = 0; i < 100; i++) { + dispatch_group_async(group, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + [self->spkiCache hashSubjectPublicKeyInfoFromCertificate:certificate]; + }); + } + + dispatch_group_notify(group, dispatch_get_main_queue(), ^{ + + NSDictionary *cache = [self->spkiCache getSubjectPublicKeyInfoHashesCache]; + XCTAssertEqual(cache.count, 1, @"Cache should contain one entry"); + + BOOL yes = YES; + OCMStub([mockApplication isProtectedDataAvailable]).andReturn(YES).andDo(^(NSInvocation *invocation) { + self->spkiCache = nil; + [invocation setReturnValue:(void *)&yes]; + }); + + // Perform one more cache operation to trigger filesystem write + [self->spkiCache hashSubjectPublicKeyInfoFromCertificate:certificate]; + + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{ + CFRelease(certificate); + [mockApplication stopMocking]; + [expectation fulfill]; + }); + }); + + [self waitForExpectationsWithTimeout:5.0 handler:nil]; +} + @end