datatheorem / TrustKit

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

Veracode flags completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, nil); as a security violation #264

Closed cesar-cmt closed 2 years ago

cesar-cmt commented 3 years ago

The readme has an example implementation showing

- (void)URLSession:(NSURLSession *)session 
               task:(NSURLSessionTask *)task 
 didReceiveChallenge:(NSURLAuthenticationChallenge *)challenge 
  completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential *credential))completionHandler {
 {
     TSKPinningValidator *pinningValidator = [[TrustKit sharedInstance] pinningValidator];
     // Pass the authentication challenge to the validator; if the validation fails, the connection will be blocked
     if (![pinningValidator handleChallenge:challenge completionHandler:completionHandler])
     {
               // TrustKit did not handle this challenge: perhaps it was not for server trust
               // or the domain was not pinned. Fall back to the default behavior
               completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, nil);
     }
 }

When performing a Veracode static scan the line completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, nil); if flagged as a security flaw. http://cwe.mitre.org/data/definitions/297.html

nabla-c0d3 commented 2 years ago

This looks like a false positive from Veracode.

Using completionHandler(NSURLSessionAuthChallengePerformDefaultHandling, nil); triggers iOS' standard TLS validation, which is secure. In addition to that we have unit tests to ensure that the validation code works as expected.