Check X509Certificate::CreateFromHandle result.

net/cert/cert_verify_proc_ios.cc

 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/cert/cert_verify_proc_ios.h"
 
 #include <CommonCrypto/CommonDigest.h>
 
 #include "base/logging.h"
 #include "base/mac/scoped_cftyperef.h"
@@ skipping 98 matching lines @@
   for (CFIndex i = 0, count = CFArrayGetCount(cert_chain); i < count; ++i) {
     SecCertificateRef chain_cert = reinterpret_cast<SecCertificateRef>(
         const_cast<void*>(CFArrayGetValueAtIndex(cert_chain, i)));
     if (i == 0) {
       verified_cert = chain_cert;
     } else {
       verified_chain.push_back(chain_cert);
     }
 
     std::string der_bytes;
-    if (!X509Certificate::GetDEREncoded(chain_cert, &der_bytes))
+    if (!X509Certificate::GetDEREncoded(chain_cert, &der_bytes)) {
+      verify_result->cert_status |= CERT_STATUS_INVALID;
       return;
+    }
 
     base::StringPiece spki_bytes;
-    if (!asn1::ExtractSPKIFromDERCert(der_bytes, &spki_bytes))
-      continue;
+    if (!asn1::ExtractSPKIFromDERCert(der_bytes, &spki_bytes)) {
+      verify_result->cert_status |= CERT_STATUS_INVALID;
+      return;
+    }
 
     HashValue sha1(HASH_VALUE_SHA1);
     CC_SHA1(spki_bytes.data(), spki_bytes.size(), sha1.data());
     verify_result->public_key_hashes.push_back(sha1);
 
     HashValue sha256(HASH_VALUE_SHA256);
     CC_SHA256(spki_bytes.data(), spki_bytes.size(), sha256.data());
     verify_result->public_key_hashes.push_back(sha256);
 
     // Ignore the signature algorithm for the trust anchor.
     if ((verify_result->cert_status & CERT_STATUS_AUTHORITY_INVALID) == 0 &&
         i == count - 1) {
       continue;
     }
   }
   if (!verified_cert) {
     NOTREACHED();
+    verify_result->cert_status |= CERT_STATUS_INVALID;

[eroman, 2017/03/24 22:07:23]

I don't know about this one, as reaching it means the chain was empty. That
said, according to the DCHECK() at the start on cert_chain's length, this
shouldn't be reachable, so meh.

[mattm, 2017/03/27 23:24:37]

Yeah, on this and the mac one, due to the presence of the NOTREACHED, I felt
more confident that it shouldn't really happen and adding the error code just to
be safe.

But yeah, probably doesn't matter either way.

     return;
   }
 
-  verify_result->verified_cert =
+  scoped_refptr<X509Certificate> verified_cert_with_chain =
       X509Certificate::CreateFromHandle(verified_cert, verified_chain);
+  if (verified_cert_with_chain)
+    verify_result->verified_cert = std::move(verified_cert_with_chain);
+  else
+    verify_result->cert_status |= CERT_STATUS_INVALID;
 }
 
 }  // namespace
 
 CertVerifyProcIOS::CertVerifyProcIOS() {}
 
 // The iOS APIs don't expose an API-stable set of reasons for certificate
 // validation failures. However, internally, the reason is tracked, and it's
 // converted to user-facing localized strings.
 //
@@ skipping 114 matching lines @@
   // roots.
   verify_result->is_issued_by_known_root = false;
 
   if (IsCertStatusError(verify_result->cert_status))
     return MapCertStatusToNetError(verify_result->cert_status);
 
   return OK;
 }
 
 }  // namespace net