search

datatheorem / TrustKit

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

No validation of result when calling SecTrustEvaluate #239

Closed yonicsurny closed 3 years ago

yonicsurny commented 3 years ago

Hello, shouldn't the result of the call to SecTrustEvaluate be validated?

https://github.com/datatheorem/TrustKit/blob/3c953558d61fdd9b136d981764e3242bd92b2648/TrustKit/Pinning/TSKSPKIHashCache.m#L264

My team uses a code analyser tool which is reporting that this piece of code "fails to check the result of the evaluation".

Suggested remediation is as follow:

SecTrustResultType result = kSecTrustResultInvalid;
OSStatus status = SecTrustEvaluate(trust, &result);
if (status == noErr && (kSecTrustResultProceed == result || kSecTrustResultUnspecified == result)) {
    // Proceed with trust
}
else {
    // Cancel process or return error
}

What do you think?

nabla-c0d3 commented 3 years ago

Hello and thanks for the report.

The code you pointed out does not use the result of the validation at all: SecTrustEvaluate() is only called to initialize the trust object so we can call SecTrustCopyPublicKey() on it.

We do use SecTrustEvaluate() to "check the result of the evaluation" in https://github.com/datatheorem/TrustKit/blob/3c953558d61fdd9b136d981764e3242bd92b2648/TrustKit/Pinning/ssl_pin_verifier.m#L50 , which follows a similar flow as the suggested remediation.