search

datatheorem / TrustKit

Easy SSL pinning validation and reporting for iOS, macOS, tvOS and watchOS.
MIT License
2.01k stars 362 forks source link

Crash from "Detected over-release of a CFTypeRef" #251

Closed CraigSiemens closed 3 years ago

CraigSiemens commented 3 years ago

There's another crash that's been appearing in our app. So far it's only happened a handful of times but they've all been iOS >= 14.4, but the majority of our user base is on those versions so it might not be relevant.

Detected over-release of a CFTypeRef 0x10b015a00 (325 / SecCertificate) > hashSubjectPublicKeyInfoFromCertificate: > SecCertificate

I looked through the code and it appears that the returned OSStatus from calling SecTrustCreateWithCertificates is not checked. If it fails for any reasons, it still tries to release the trust variable which may not have been set. I also noticed the the following call to SecTrustEvaluate doesn't check the result or the returned OSStatus either which is probably an issue as well.

I'd be willing to try fixing it but I'm not sure what kind of affect returning nil from copyPublicKeyFromCertificate: would have on the other places calling it.

OS Version: iOS 14.4.2 (18D70)
Report Version: 104

Exception Type: EXC_BREAKPOINT (SIGTRAP)
Crashed Thread: 1

Application Specific Information:
Detected over-release of a CFTypeRef 0x129023400 (325 / SecCertificate) > hashSubjectPublicKeyInfoFromCertificate: > SecCertificate

Thread 1 Crashed:
0   CoreFoundation                  0x3128973d8         _CFRelease.cold.3
1   CoreFoundation                  0x3128973cc         _CFRelease.cold.3
2   CoreFoundation                  0x3127a356c         _CFRelease
3   CoreFoundation                  0x312706284         -[__NSSingleObjectArrayI dealloc]
4   Security                        0x3214a2420         SecTrustDestroy
5   CoreFoundation                  0x3127a3100         _CFRelease
6   TrustKit                        0x1050d0738         -[TSKSPKIHashCache copyPublicKeyFromCertificate:] (TSKSPKIHashCache.m:267)
7   TrustKit                        0x1050cfd50         -[TSKSPKIHashCache hashSubjectPublicKeyInfoFromCertificate:] (TSKSPKIHashCache.m:175)
8   TrustKit                        0x1050cf9f0         verifyPublicKeyPin (ssl_pin_verifier.m:71)
9   TrustKit                        0x1050d6d10         -[TSKPinningValidator evaluateTrust:forHostname:] (TSKPinningValidator.m:126)
10  TrustKit                        0x1050d70e0         -[TSKPinningValidator handleChallenge:completionHandler:] (TSKPinningValidator.m:202)
...
nabla-c0d3 commented 3 years ago

Hello and thanks for the bug report!

The copyPublicKeyFromCertificate function is only called by hashSubjectPublicKeyInfoFromCertificate, which can already return nil if something went wrong.

Hence, one option would be to return nil in copyPublicKeyFromCertificate and then nil again in hashSubjectPublicKeyInfoFromCertificate, which would cause TrustKit's validation to fail and reject the connection.

Hence, the app still wouldn't work, but it wouldn't crash.