Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TrustKit 1.5.1: TSKSPKIHashCache identifier can be nil, but an assert checks for a valid value. #147

Closed
marcopax opened this issue Oct 9, 2017 · 2 comments

Comments

@marcopax
Copy link

marcopax commented Oct 9, 2017

I'm working with TSKSPKIHashCache, and I found a crash using the initialization method:

/**
 Create a new cache of SPKI hashes. The identifier is required to ensure that multiple cache
 instances do not attempt to use the same file on disk for persistence. If nil, persistence
 will be disabled (not recommended).

 @param uniqueIdentifier A unique identifier that is stable across app launches/instance creation
 @return An initialized hash cache.
 */
- (instancetype _Nullable)initWithIdentifier:(NSString * _Nullable)uniqueIdentifier NS_DESIGNATED_INITIALIZER;

It says that If nil, persistence will be disabled. We also have a _Nullable key: it means that identifier can actually be nil.

However, in the implementation of the class, the following line crashes:

_subjectPublicKeyInfoHashesCache = [self loadSPKICacheFromFileSystem]; 

It tries to get the path of the cache file, but trying to generate the URL for the cache file, an assert checks for a valid name:

- (NSURL *)SPKICachePath
{
    NSAssert(self.spkiCacheFilename, @"SPKI filename should not be nil");
    NSURL *cachesDirUrl = [NSFileManager.defaultManager URLsForDirectory:NSCachesDirectory
                                                               inDomains:NSUserDomainMask].firstObject;
    return [cachesDirUrl URLByAppendingPathComponent:self.spkiCacheFilename];
}

If the identifier is nil, the NSAssert will let application crashes. Shouldn't it just disable the persistence?

@nabla-c0d3
Copy link
Member

Hello,
Thanks for the report! This seems like a bug indeed; we'll take a look.

nabla-c0d3 added a commit that referenced this issue Nov 27, 2017
@nabla-c0d3
Copy link
Member

Fixed in 1.5.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants